catamphetamine / react-phone-number-input

React component for international phone number input
http://catamphetamine.gitlab.io/react-phone-number-input/
MIT License
929 stars 195 forks source link

Typescript support #195

Closed dacevedo12 closed 5 years ago

dacevedo12 commented 6 years ago

Any plans on adding types definitions (index.d.ts)?

I find this pretty useful and I'd love to use it with our React + TS implementation

catamphetamine commented 6 years ago

I myself don't use TypeScript. You can submit a pull request with index.d.ts if you want.

subvertallchris commented 6 years ago

Here's one I just slapped together to get started. I filled in all the props where I could using the documentation but have only tested the basics, as that's all my implementation requires. I'm not going to offer this as a PR, I think that should come from someone who has tested more thoroughly.

/// <reference types="react"/>

declare module 'react-phone-number-input' {
  type FlagsMap = { [countryCode: string]: () => HTMLImageElement };

  interface ComponentProps {
    autoComplete?: string;
    className?: string;
    country?: string;
    countries?: string[];
    countryOptions?: string[];
    countrySelectComponent?: (props: any) => React.Component<object>;
    countrySelectTabIndex?: number;
    disabled?: boolean;
    displayInitialValueAsLocalNumber?: boolean;
    error?: string;
    ext?: HTMLInputElement;
    flagComponent?: (props: { country: string, flagsPath: string, flags: FlagsMap }) => React.Component<object, object>;
    flags?: FlagsMap;
    flagsPath?: string;
    getInputClassName?: (props?: { disable?: boolean, invalid?: boolean }) => string;
    indicateInvalid?: boolean;
    inputComponent?: any;
    international?: boolean;
    internationalIcon?: (val: any) => void;
    inputClassName?: string;
    labels?: { [countryCode: string]: string }[];
    limitMaxLength?: boolean;
    locale?: any;
    metadata?: any;
    placeholder?: string;
    onChange: (phone: string) => void;
    onCountryChange?: (props?: any) => void;
    showCountrySelect?: boolean;
    style?: React.HTMLProps<HTMLDivElement>;
    value?: string;
  }

  export default class PhoneInput extends React.Component<ComponentProps, object> {};
}

It's possible (likely?) that I'll update this more as my implementation moves closer to production. If I get to a point that it seems decent, I'll consider submitting it as a PR. I just don't want anyone blaming me if my incomplete or crappy type defs screw them up. 😎

catamphetamine commented 6 years ago

@subvertallchris Thanks ) Yeah, stability is good. It's funny how I periodically accidentally break TypeScript for libphonenumber-js and then the contributors are there to cover my back. https://github.com/catamphetamine/libphonenumber-js/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aclosed+typescript

subvertallchris commented 6 years ago

This is why I usually submit types to https://github.com/DefinitelyTyped/DefinitelyTyped instead of the project directly unless the whole thing is written in TypeScript. As happy as I am when I see a project has types, I don't think it should be your job to maintain or care about them when you're already trying to maintain the library itself.

sl45sms commented 5 years ago

based on @subvertallchris add types for isValidPhoneNumber formatPhoneNumber


/// <reference types="react"/>

declare module 'react-phone-number-input' {
    type FlagsMap = { [countryCode: string]: () => HTMLImageElement };

    interface ComponentProps {
      autoComplete?: string;
      className?: string;
      country?: string;
      countries?: string[];
      countryOptions?: string[];
      countrySelectComponent?: (props: any) => React.Component<object>;
      countrySelectTabIndex?: number;
      disabled?: boolean;
      displayInitialValueAsLocalNumber?: boolean;
      error?: string;
      ext?: HTMLInputElement;
      flagComponent?: (props: { country: string, flagsPath: string, flags: FlagsMap }) => React.Component<object, object>;
      flags?: FlagsMap;
      flagsPath?: string;
      getInputClassName?: (props?: { disable?: boolean, invalid?: boolean }) => string;
      indicateInvalid?: boolean;
      inputComponent?: any;
      international?: boolean;
      internationalIcon?: (val: any) => void;
      inputClassName?: string;
      labels?: { [countryCode: string]: string }[];
      limitMaxLength?: boolean;
      locale?: any;
      metadata?: any;
      placeholder?: string;
      onChange: (phone: string) => void;
      onCountryChange?: (props?: any) => void;
      showCountrySelect?: boolean;
      style?: React.HTMLProps<HTMLDivElement>;
      value?: string;
    }  

    export function isValidPhoneNumber(phone?:any):any;
    export function formatPhoneNumber(phone?:any,format?:string): any;

    export default class PhoneInput extends React.Component<ComponentProps, object> {};

  }
catamphetamine commented 5 years ago

@sl45sms Though I can see your type definitions lacking refinement: in many places any could be substituted with a more specific type. See the existing index.d.ts for an example. Anyway, I guess the reason that you didn't submit a pull request for index.d.ts is because you'd like someone to continue where you left off and then a pull request could be submitted.

sl45sms commented 5 years ago

@catamphetamine correct, i am on the same path with @subvertallchris

I just don't want anyone blaming me if my incomplete or crappy type defs screw them up

catamphetamine commented 5 years ago

@sl45sms I see. Ok, the types draft with my edits then:

export type NumberFormat = 'NATIONAL' | 'National' | 'INTERNATIONAL' | 'International';

export function formatPhoneNumber(value?: string): string;
export function formatPhoneNumber(value: string, format?: NumberFormat): string;
export function formatPhoneNumberIntl(value?: string): string;
export function isValidPhoneNumber(value?: string): boolean;

// Somehow import React types here.

type FlagsMap = { [countryCode: string]: React.Component<object, object> };

interface ComponentProps {
  autoComplete?: string;
  className?: string;
  country?: string;
  countries?: string[];
  countryOptions?: string[];
  countrySelectComponent?: React.Component<object, object>;
  countrySelectTabIndex?: number;
  disabled?: boolean;
  displayInitialValueAsLocalNumber?: boolean;
  error?: string;
  ext?: React.Element,
  flagComponent?: React.Component<{ country: string, flagsPath: string, flags: FlagsMap }, object>;
  flags?: FlagsMap;
  flagsPath?: string;
  getInputClassName?: (params: { disable?: boolean, invalid?: boolean }) => string;
  // @deprecated
  // indicateInvalid?: boolean;
  inputComponent?: React.Component<object, object>;
  international?: boolean;
  internationalIcon?: React.Component<object, object>;
  inputClassName?: string;
  labels?: { [key: string]: string };
  limitMaxLength?: boolean;
  metadata?: object;
  placeholder?: string;
  onChange: (phone?: string) => void;
  onCountryChange?: (countryCode?: string) => void;
  showCountrySelect?: boolean;
  style?: object,
  value?: string;
}

export default class PhoneInput extends React.Component<ComponentProps, object> {};

I don't know how TypeScript imports other types (React in this case).

rexpan commented 5 years ago

Maybe like this?

declare module "react-phone-number-input" {
  import {ReactElement, Component, CSSProperties} from "react";

  export type NumberFormat = 'NATIONAL' | 'National' | 'INTERNATIONAL' | 'International';

  export function formatPhoneNumber(value?: string): string;
  export function formatPhoneNumber(value: string, format?: NumberFormat): string;
  export function formatPhoneNumberIntl(value?: string): string;
  export function isValidPhoneNumber(value?: string): boolean;

  // Somehow import React types here.

  export type FlagsMap = { [countryCode: string]: Component<object, object> };

  export interface CountrySelectComponentProps {
    name     ?: string; // HTML name attribute
    value    ?: string; //The currently selected country code.
    onChange ?: (value : string) => void; //Updates the value.
    onFocus  ?: () => void; //Is used to toggle the --focus CSS class.
    onBlur   ?: () => void; //Is used to toggle the --focus CSS class.
    options  ?: Array<{ value?: string, label: string, icon: Component }>; //The list of all selectable countries (including "International").
    disabled ?: boolean; //HTML disabled attribute.
    tabIndex ?: (number|string); //HTML tabIndex attribute.
    className?: string; //CSS class name.
  }

  export interface InputComponentProps {
    value         : string; // The parsed phone number. E.g.: "", "+", "+123", "123".
    onChange      : (value : string) => void; // Updates the value.
    onFocus      ?: () => void; // Is used to toggle the --focus CSS class.
    onBlur       ?: () => void; // Is used to toggle the --focus CSS class.
    country      ?: string; // The currently selected country. undefined means "International" (no country selected).
    metadata     ?: object; // libphonenumber-js metadata.
  }

  export interface PhoneInputProps {
    value                            : string;
    onChange                         : (value: string) => void;
    //
    autoComplete                    ?: string;
    className                       ?: string;
    country                         ?: string;
    countries                       ?: string[];
    countryOptions                  ?: string[];
    countrySelectComponent          ?: Component<CountrySelectComponentProps, object>;
    countrySelectTabIndex           ?: number;
    disabled                        ?: boolean;
    displayInitialValueAsLocalNumber?: boolean;
    error                           ?: string;
    ext                             ?: ReactElement,
    flagComponent                   ?: Component<{ country: string, flagsPath: string, flags: FlagsMap }, object>;
    flags                           ?: FlagsMap;
    flagsPath                       ?: string;
    getInputClassName               ?: (params: { disable ?: boolean, invalid ?: boolean }) => string;
    inputComponent                  ?: Component<InputComponentProps, object>;
    international                   ?: boolean;
    internationalIcon               ?: Component<object, object>;
    inputClassName                  ?: string;
    labels                          ?: { [key: string]: string };
    limitMaxLength                  ?: boolean;
    metadata                        ?: object;
    placeholder                     ?: string;
    onCountryChange                 ?: (countryCode?: string) => void;
    showCountrySelect               ?: boolean;
    style                           ?: CSSProperties;
  }

  export default class PhoneInput extends Component<PhoneInputProps, object> {}
}
catamphetamine commented 5 years ago

@rexpan Does it work for you? If two other men in this thread approve it then I'll add it to the library.

catamphetamine commented 5 years ago

@rexpan Also, what's the summary of your changes? And I don't think that the "align space" formatting you've used is a standardized one.

PeterYangIO commented 5 years ago

I believe you can import React types by including a Triple Slash Directive

So from @rexpan it would look something like the below with the following changes:

catamphetamine commented 5 years ago

@PeterYangIO Good work. So does this typescript definition file work in your project? If it does then I guess we should release it.

PeterYangIO commented 5 years ago

@catamphetamine Yes I use this version in my current project but I do not have full code coverage of it - I only use a few of the features. If we add this to the release I believe we may need to add @types/react as a devDependency. I can provide a pull request to demonstrate the changes necessary to index.d.ts and package.json if you would like.

catamphetamine commented 5 years ago

@PeterYangIO Yes please, do it when you have time. If something potentially breaks for TypeScript users then they'll create an issue in this repo and we'll fix it. Otherwise it would never get released.

PeterYangIO commented 5 years ago

I went ahead and made the changes I think that are necessary to be included in the library. Can anyone else take a look at #249?

catamphetamine commented 5 years ago

Everyone: I'll wait until tomorrow and if there'll be no objections i'll merge and release.

catamphetamine commented 5 years ago

Published react-phone-number-input@2.3.10. Closing this issue but anyone can still report issues here.

charisra commented 5 years ago

Hi to everyone. On index.d.ts I'm getting an error on line 55:

Generic type 'ReactElement<P, T>' requires between 1 and 2 type arguments.

Anyone else getting this? And perhaps a solution? @catamphetamine perhaps update ?

It most probably should be: React.ReactElement<string | number> (pick the type you intend 'ext' to be)

catamphetamine commented 5 years ago

@charisra Re-opening the issue. If you happen to find a fix then create a pull request and I'll merge it.

catamphetamine commented 5 years ago

@charisra Published your fix.

catamphetamine commented 5 years ago

cc @rexpan @PeterYangIO @sl45sms Does anyone know why is there declare module "react-phone-number-input" block? Is it needed there or can it be removed? Recently @hazratgs sent a pull request (which I merged) with copy-pasted definitions for /max, /min and /mobile packages (actually, /mobile is still missing type definitions so that will have to be fixed). If the original definitions file didn't contain the declare module "react-phone-number-input" block then all sub-packages could simply use that one definitions file: "types": "../index.d.ts". If there's no consensus that the declare module line is required there by Tuesday we'll be removing it (and also removing the typings for /min and /max and setting types in package.json of all sub-packages to the main typings file).

PeterYangIO commented 5 years ago

I believe the declare module block is necessary for TypeScript to know the types are associated to to the specific library / namespace, however I am not familiar with the behavior regarding sub-packages but I do agree we should look towards reducing the unnecessary code reuse if anyone else has an idea.

catamphetamine commented 5 years ago

@PeterYangIO If you look at the typings previously committed to libphonenumber-js they seem to work without declare module: https://github.com/catamphetamine/libphonenumber-js/blob/master/index.d.ts That's why it might not be necessary. We'll see what other opinions are there.

catamphetamine commented 5 years ago

Released a new version with declare module removed. @hazratgs Your pull request has been reverted.

catamphetamine commented 5 years ago

Seems that someone's having issues with the new typings: https://github.com/catamphetamine/react-phone-number-input/issues/263 If no one submits a fix today I'll be removing .d.ts files from the repo in order for the library to be usable.

subvertallchris commented 5 years ago

I second that and recommend the typings be moved to DefinitelyTyped. It’s an unreasonable burden for a library maintainer to manage them when they aren’t using TypeScript.

On Tue, Jun 11, 2019 at 11:29 AM Nikolay notifications@github.com wrote:

Seems that someone's having issues with the new typings:

263

https://github.com/catamphetamine/react-phone-number-input/issues/263 If no one submits a fix today I'll be removing .d.ts files from the repo in order for the library to be usable.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/catamphetamine/react-phone-number-input/issues/195?email_source=notifications&email_token=AA7IJ53DBUWCLL6LQ3F2VKDPZ7AFVA5CNFSM4FZUWUKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXNQWVA#issuecomment-500894548, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7IJ52FKDHUI3WO6ROTZ3TPZ7AFVANCNFSM4FZUWUKA .

catamphetamine commented 5 years ago

Removed all React-related typings so far. @Borvik See if version react-phone-number-input@2.3.16 works.

Borvik commented 5 years ago

Nope different error though:

src/index.tsx:275:16 - error TS2604: JSX element type 'PhoneInput' does not have any construct or call signatures.

275               <PhoneInput
                   ~~~~~~~~~~

We have added custom .d.ts definition files for libraries that don't have type definition files - so that is always an option - just don't want to conflict with something already in the library.

Like otherwise suggested, perhaps it's better to move it completely to DefinitelyTyped - that or fully implement it here. If it's nowhere, developers can at least create a custom file for their specific use cases.

catamphetamine commented 5 years ago

@Borvik Ok, released react-phone-number-input@2.3.17 without the TypeScript file.

MaxSvargal commented 5 years ago

Where are typings? The are no in DefinitelyTyped too...

subvertallchris commented 5 years ago

@MaxSvargal Someone needs to manually validate one of the files above and submit to DefinitelyTyped. In the meantime, you can copy/paste it into a .d.ts file and require as custom types.

AdrienEtienne commented 4 years ago

@PeterYangIO and @catamphetamine I have created a PR in the DefinitelyTyped repo (https://github.com/DefinitelyTyped/DefinitelyTyped/pull/41668)

Peter I added you in the author list since I took the code from you. Is it ok for you?

PeterYangIO commented 4 years ago

@PeterYangIO and @catamphetamine I have created a PR in the DefinitelyTyped repo (DefinitelyTyped/DefinitelyTyped#41668)

Peter I added you in the author list since I took the code from you. Is it ok for you?

sounds good

lucksp commented 4 years ago

for anyone watching, or active with this Typescript Def file:

can the international prop be added as per react-phone-number-input docs show?

international: boolean? — If country is specified and international property is true then the phone number can only be input in "international" format for that country, but without "country calling code" part. For example, if country is "US" and international property is not passed then the phone number can only be input in the "national" format for US ((213) 373-4253). But if country is "US" and international property is true then the phone number will be input in the "international" format for US (213 373 4253) without "country calling code" part (+1). This could be used for implementing phone number input components that show "country calling code" part before the input field and then the user can fill in the rest of their phone number in the input field.

catamphetamine commented 3 years ago

Added TypeScript "typings" in react-phone-number-input@3.1.32. In case of any issues, open an issue.