`react-hook-form` component typings seem to be incorrect

[finkrer]

The default import from 'react-phone-number-input/react-hook-form' has type any.

I assume this is because the type

type PhoneInputWithCountrySelectType<InputComponentProps = DefaultInputComponentProps, FormValues> = React.ComponentClass<Props<InputComponentProps, FormValues>, State<Props<InputComponentProps, FormValues>>>

is incorrect.

First of all, the type parameter FormValues follows an optional type parameter, which is not allowed in TypeScript.

In addition to that, FormValues is a required parameter, but the component type uses PhoneInputWithCountrySelectType without any parameters.

declare const PhoneInputWithCountrySelect: PhoneInputWithCountrySelectType;

As a result, it seems that TS cannot parse that and assigns any as the type.

As a workaround, I have swapped the parameters and assigned {} to FormValues, as I have no idea what it's supposed to be, and it works.

[catamphetamine]

Hi. I'm not a TypeScript expert so I don't quite get it what you mean and how could something be potentially fixed. Also, applying suggestions from users on the internet is not guaranteed because there's no team of TS experts that could review the suggestions.

[finkrer]

I'm sorry, didn't you write the type declarations? If you could write them, you can fix them, I think.

Essentially, there is a mistake in the type definitions. Because of that, they don't "compile". So when I use the component there are no types.

I've looked around and found this commit: https://gitlab.com/catamphetamine/react-phone-number-input/-/commit/22c1f46aa55bd879984d1dd2fa5e21e010df9c6b

These changes break the typings. Before the commit, they looked ok. After the commit, it's just not valid TypeScript, so it doesn't work.

I'm not suggesting anything specific, as you wrote the original definitions and only you know what they are supposed to do. I'm hoping you can fix them so that they are actually usable out of the box.

[catamphetamine]

I'm sorry, didn't you write the type declarations?

I did.

If you could write them, you can fix them, I think.

Not necessarily.

I'm not suggesting anything specific, as you wrote the original definitions and only you know what they are supposed to do.

That's not true. Anyone can fix anything, provided they have a sufficient proof of their expertise.

I'm hoping you can fix them so that they are actually usable out of the box.

I guess that won't happen. I'm not a TypeScript user myself. Those typings could be viewed as a "starting point" for some TypeScript expert to finish them.

[catamphetamine]

I've looked around and found this commit: https://gitlab.com/catamphetamine/react-phone-number-input/-/commit/22c1f46aa55bd879984d1dd2fa5e21e010df9c6b

So you're suggesting that adding FormValues generic type broke the typings.

Maybe.

I don't know how to write it properly with that generic in place. As I've said, I'm not a TypeScript expert.

[catamphetamine]

From this stackoverflow discussion, seems like something like that would work:

type FunctionType = <TValue>(value: TValue) => void;
const bar: FunctionType = <TValue>(value) => { ... }

• That would only work for functions. I guess PhoneInputWithCountrySelectType is a function.
• We don't have a "definition" of the variable (the right part). Just a "declaration". So it's unclear how to declare const while also having a generic not specified.

Or maybe it shouldn't be declare const and should be something else. declare const technique is only used there to export default.

[catamphetamine]

Or maybe you're using it incorrectly, which is also a possibility. So ask TypeScript experts if you decide to investigate further. Otherwise, won't be fixed.

[catamphetamine]

Or, you could see if the following works:

Replace <InputComponentProps = DefaultInputComponentProps, FormValues>

With <InputComponentProps = DefaultInputComponentProps, FormValues = Record<string, unknown>>

In index.d.ts of react-phone-number-input/react-hook-form

https://stackoverflow.com/questions/40641370/generic-object-type-in-typescript

If that works, and if you post your code, then maybe that could be a fix.

[catamphetamine]

Or, I'd rather prefer Record<string, any> because it's in react-hook-form's official TypeScript docs.

const Form = <TFormValues extends Record<string, any>({
  onSubmit,
  children
}: FormProps<TFormValues>) => {
  const methods = useForm<TFormValues>();
  return (
    <form onSubmit={methods.handleSubmit(onSubmit)}>{children(methods)}</form>
  );
};

I guess I'll publish this one then.

[catamphetamine]

published react-phone-number-input@3.2.8

[finkrer]

Thanks for the quick fix, eveything works now.

If I go into TypeScript expert mode, looking at the types, I can also say that this makes FormValues useless. The API is essentially the same as before that commit I mentioned. FormValues is only used for Control, which already provides a default of Record<string, any>, as you noted. So you are now drilling through the type hierarchy only to replace the default type with the exact same type.

The only difference is that you can in theory import the generic type manually and provide a different value. But the exported component's type is non-generic and can't be changed by the developer.

You can safely leave this as it is, I just already typed that when you released the fix, haha.

[catamphetamine]

The only difference is that you can in theory import the generic type manually and provide a different value. But the exported component's type is non-generic and can't be changed by the developer.

Yeah, I didn't find any info on how could one export a React component that would also support passing custom generics. Or maybe I did and then it broke things. Or something like that.

You can safely leave this as it is, I just already typed that when you released the fix, haha.

Cool, thx.

[finkrer]

Yeah, I didn't find any info on how could one export a React component that would also support passing custom generics. Or maybe I did and then it broke things. Or something like that.

Aah, well, if that's what you wanted, I have a fix for you.

Instead of

type PhoneInputType<InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues> = (props: Props<InputComponentProps, FormValues>) => JSX.Element;

use

type PhoneInputType = <InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues>(props: Props<InputComponentProps, FormValues>) => JSX.Element;

That way the actual function type is generic and not the wrapper type.

And then you can write something like this

<PhoneInput<MyProps, MyFormValues> ... />

Tested this and it works.

[catamphetamine]

@finkrer

Tested this and it works.

Hmm, and can one also write something like this and it'll work too?

<PhoneInput ... />

i.e. without specifying any custom generics?

[finkrer]

You can.

I've played around with this a little, and while the generics work, they are also less convenient in some cases. If you have a wrapping component that adds PhoneInput's props to its own, right now you can just use typeof PhoneInput to extract the types automatically. With generics, the parameters evaluate to <unknown, any> in this case, which doesn't accept anything. As a workaround, any wrapper has to also be generic and construct the props type manually.

I'm not sure what the use would be for generics anyway. Normally generic components are useful because TS infers the type from one prop and uses it to validate others, but here each parameter is only used once. So it's probably not worth it in this case.

[catamphetamine]

@finkrer Hmm, I see.

Generics could be used in case of using a custom InputComponent in order to pass the properties for it: all ...rest properties are passed through to InputComponent.

For example, some people use Material UI input, or something like that.

I guess my TypeScript experience is not enough to make a decision on this topic. Perpahs we should leave it as is, and I'll add a link to this discussion in the TypeScript files.

[ercgrat]

I have a use case for the generics proposal above. I am wrapping the ReactPhoneNumberInput in an internal design system component so we can constrain the props and styles to our app's preferences. I'm using generics on the wrapper component to properly type the control and name props in relation to one another, i.e.:

type Props<
  TFieldValues extends FieldValues,
  TName extends keyof TFieldValues
> = {
  control: Control<TFieldValues>;
  name: TName;
}

When written this way, the idea is that TFieldValues is inferred from the control that gets passed, and an error will appear if you typed a name that is not actually a key of the control.

However, because of the way the phone number input is typed, it rejects the control and name types: Type 'Control<TFieldValues>' is not assignable to type 'Control<DefaultFormValues>'. Basically, it compares TFieldValues and DefaultFormValues and determines they're not compatible. There's no way right now of specifying the types for the phone number input to expect. Would love to be able to say ReactPhoneNumberInput<Props, TFieldValues> with @finkrer's suggestion.

[catamphetamine]

@ercgrat So looks like I've been declaring React components incorrectly?

For example, instead of this:

export type DefaultInputComponentProps = {
    [anyProperty: string]: any;
}

type PhoneInputWithCountrySelectType<InputComponentProps = DefaultInputComponentProps> = React.ComponentClass<Props<InputComponentProps>, State<Props<InputComponentProps>>>

declare const PhoneInputWithCountrySelect: PhoneInputWithCountrySelectType;

export default PhoneInputWithCountrySelect;

it should've been:

declare function PhoneInputWithCountrySelect<InputComponentProps extends object={}> (
    props: Props<InputComponentProps>
): JSX.Element;

export default PhoneInputWithCountrySelect;

That is, if the component was implemented as a functional one rather than a "class" one.

Is that assumption correct? Would there be any drawbacks?

@finkrer has previously mentioned that in that case, typeof PhoneInputWithCountrySelect wouldn't work. I wonder why wouldn't it? It has the default values for the generics, so why wouldn't it simply assume typeof PhoneInputWithCountrySelect< = DefaultInputComponentProps> in such case?

If simple typeof wouldn't work, I guess we could force developers to always write typeof with generics. I guess that wouldn't be too much of a burden? Is it something that is expected in TypeScript world?

I also wonder if we could get away with declaring a "class" component as a function in TypeScript? I guess it wouldn't really care if it was a function or a class?

Also, maybe the generics should be rewritten from InputComponentProps to just typeof InputComponent?

type PropsOf<
    C extends keyof JSX.IntrinsicElements | React.JSXElementConstructor<any>
    > = JSX.LibraryManagedAttributes<C, React.ComponentPropsWithoutRef<C>>

and then replace InputComponentProps with PropsOf<InputComponent>.

[finkrer]

@finkrer has previously mentioned that in that case, typeof PhoneInputWithCountrySelect wouldn't work. I wonder why wouldn't it? It has the default values for the generics, so why wouldn't it simply assume typeof PhoneInputWithCountrySelect< = DefaultInputComponentProps> in such case?

Because it has opposite variance here.

When generating a PhoneInputWithCountrySelect, we can assume the user doesn't mind a PhoneInputWithCountrySelect<DefaultInputComponentProps>.

When accepting a PhoneInputWithCountrySelect's type as an argument (to get the type of its props), we can't assume all PhoneInputWithCountrySelects are PhoneInputWithCountrySelect<DefaultInputComponentProps>. They might have custom input props, hence the inferred type is PhoneInputWithCountrySelect<unknown>, and the resulting props give you stuff like Control<unknown>, which isn't accepted by the actual PhoneInputWithCountrySelect component.

The workaround is essentially something like this:

import { Props as PhoneNumberInputProps } from 'react-phone-number-input/react-hook-form-input'

const PhoneNumberInput: FC<Props, (props: PhoneNumberInputProps<PropsOf<CustomInputComponent>>) => JSX.Element> = (
  { name, control, hasError, ...rest },
  ref
) => {
  ...
  return (
    <PhoneInput<PropsOf<CustomInputComponent>>
      ref={ref}
      inputComponent={CustomInputComponent}
      name={name}
      control={control}
      {...rest}
    />
  )
}

The main problem here is that you have to describe the type of the PhoneInput again, saying, yeah, it's a (props: PhoneNumberInputProps<PropsOf<CustomInputComponent>>) => JSX.Element>, which is a handful.

A solution would be to have a second type which is exactly as it is in the current version:

type PhoneInputType<InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues> = (props: Props<InputComponentProps, FormValues>) => JSX.Element;

Now this can be used to construct the props type, like so:

const PhoneNumberInput: FC<Props, PhoneInputType<PropsOf<CustomInputComponent>>> =

Which is as good as it gets.

So basically you need both types, one for the actual component's type, that's the one I proposed earlier, and the other to represent the component's type with fixed generic parameters, that's the one that is there right now. The second one needs to be exported as well.

[catamphetamine]

@finkrer Thx for the explanation. I wonder why the following wouldn't work though?

export type PhoneInputComponentType = FC<Props, PhoneInputType<PropsOf<CustomInputComponent>>>

and then:

const PhoneNumberInput: PhoneInputComponentType = ...

I mean, why would anyone use PhoneInputType to re-implement PhoneInputComponentType if there already would be PhoneInputComponentType exported from the library.

[finkrer]

@finkrer Thx for the explanation. I wonder why the following wouldn't work though?

export type PhoneInputComponentType = FC<Props, PhoneInputType<PropsOf<CustomInputComponent>>>

Because it's a type for the user component that wraps your component, so you don't really know Props — those are the user component's props. And CustomInputComponent is, of course, a custom component, so you don't know it either. That's why you have to expose a generic type (PhoneInputType in this case).

In any case, it's totally fine for the user to define their own component's type. What you can do is to make it easy for them to get your component's type, hence the need for the second type.

[catamphetamine]

@finkrer I see, so an exported type can't be "genericised"?

export type PhoneInputComponentType<CustomInputComponent> = FC<Props, PhoneInputType<PropsOf<CustomInputComponent>>>

[finkrer]

@catamphetamine It can be, it's just that giving a type for the user's wrapper component is not something you need to do. In my code base, most components are typed as FC<Props, Something>. Obviously I wouldn't want a library trying to make me use something else. Especially since in my case FC is a custom type as well, so you can't use it like that.

So, I don't need a PhoneInputComponentType for my wrapper component, I just need a PhoneInputType to represent your component in case I need it.

[catamphetamine]

@finkrer Ok.

So, the changes would be: prepend export before component types and also move generics from the left side of the = to the right side of the =.

react-hook-form/index.d.ts

From

type PhoneInputWithCountrySelectType<InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues> = React.ComponentClass<Props<InputComponentProps, FormValues>, State<Props<InputComponentProps, FormValues>>>;

To

export type PhoneInputWithCountrySelectType = <InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues>React.ComponentClass<Props<InputComponentProps, FormValues>, State<Props<InputComponentProps, FormValues>>>;

react-hook-form-core/index.d.ts

From

type PhoneInputWithCountrySelectType<InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues> = (props: Props<InputComponentProps, FormValues>) => JSX.Element;

To

export type PhoneInputWithCountrySelectType = <InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues>(props: Props<InputComponentProps, FormValues>) => JSX.Element;

react-hook-form-input/index.d.ts

From

type PhoneInputType<InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues> = (props: Props<InputComponentProps, FormValues>) => JSX.Element;

To

export type PhoneInputType = <InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues>(props: Props<InputComponentProps, FormValues>) => JSX.Element;

react-hook-form-input-core/index.d.ts

From

type PhoneInputType<InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues> = (props: Props<InputComponentProps, FormValues>) => JSX.Element;

To

export type PhoneInputType = <InputComponentProps = DefaultInputComponentProps, FormValues = DefaultFormValues>(props: Props<InputComponentProps, FormValues>) => JSX.Element;

Does that sound right?

Does it resolve the type declaration inconvenience you were having?

Will it enable users to write <PhoneInput/> as well as <PhoneInput<CustomProps, CustomFormValues>/>?

[finkrer]

@catamphetamine Actually, I just found out that there's a new TS feature that allows you to fix a generic function's type. So you can just write typeof PhoneInput<CustomProps, CustomFormValues> now.

In that case, we don't need the export. Just move the generic to the right and it should all work great.

[catamphetamine]

Ok, published react-phone-number-input@3.2.13

cc @ercgrat

[ercgrat]

Thank you so much @catamphetamine! Never seen a maintainer respond so quickly.

I think there is an issue with the change. I am using refs and it appears that the ref prop has been masked by the typing change so that the component no longer accepts ref, regardless of whether I specify the generic.

[catamphetamine]

@ercgrat Can you post a screenshot of the error? And the exact error message. Perhaps we could have a look.