Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix resolvers typings with react-hook-form@7.15.0 #233

Merged
merged 11 commits into from
Sep 18, 2021
Merged

Fix resolvers typings with react-hook-form@7.15.0 #233

merged 11 commits into from
Sep 18, 2021

Conversation

julienfouilhe
Copy link
Contributor

@julienfouilhe julienfouilhe commented Sep 6, 2021

I upgraded to the latest react-hook-form and @hookform/resolvers versions and all of a sudden all my forms that were using the yupResolver were throwing a typescript error while those using the io-ts resolver (I'm progressively migrating to io-ts) didn't.

I checked what could be the source and figured the typings were not infered properly when wrapping the generic function.

Example of something that does not work:

const { register } = useForm<Type>({
  resolver: yupResolver(schema),
});

Fixes #234

@bluebill1049
Copy link
Member

thanks for the contribution 💪

@julienfouilhe julienfouilhe changed the title Fix yupResolver typings Fix resolvers typings Sep 6, 2021
@julienfouilhe julienfouilhe changed the title Fix resolvers typings Fix resolvers typings with react-hook-form@7.15.0 Sep 6, 2021
@julienfouilhe julienfouilhe changed the title Fix resolvers typings with react-hook-form@7.15.0 Fix resolvers typings with react-hook-form@7.15.0 Sep 6, 2021
superstruct/src/types.ts Outdated Show resolved Hide resolved
@alb7h
Copy link

alb7h commented Sep 9, 2021

@bluebill1049 Any idea when this'll get merged?

@bluebill1049
Copy link
Member

let's wait for @jorisre he is the master mind behind resolver.

@julienfouilhe
Copy link
Contributor Author

@bluebill1049 Any idea when @jorisre will be available to review this? Upgrading to the latest version of react-hook-form is impossible because of this issue.

@bluebill1049
Copy link
Member

Hey @julienfouilhe I will review it now, and I have also pinged Joris too.

export type Resolver = <
T extends { [_: string]: any },
TFieldValues extends FieldValues,
TContext,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -64,22 +64,22 @@ export const zodResolver: Resolver =
options.shouldUseNativeValidation && validateFieldsNatively({}, options);

return {
errors: {},
errors: {} as FieldErrors<any>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering why have to start casting now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know exactly but seems like an empty object is not assignable to the DeepMap with the new version of FieldErrors from react-hook-form?

bluebill1049
bluebill1049 previously approved these changes Sep 16, 2021
Copy link
Member

@bluebill1049 bluebill1049 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jorisre
Copy link
Member

jorisre commented Sep 16, 2021

Sorry for the delay. I'll look at it tonight

@bluebill1049
Copy link
Member

Sorry for the delay. I'll look at it tonight

Merci

@jorisre
Copy link
Member

jorisre commented Sep 16, 2021

We loose the return type for the resolvers:
Screenshot 2021-09-16 at 12 30 17

We are getting

{
    [x: string]: any;
}

instead of the values. That why the resolvers types is a bit different

@julienfouilhe
Copy link
Contributor Author

julienfouilhe commented Sep 16, 2021

@jorisre What would be the expected signature? I feel like values should always be typed unknown, and we should infer the TFieldValues from the schema, like this:

import { ResolverResult, ResolverOptions, FieldValues } from 'react-hook-form';
import { z } from 'zod';

export type Resolver = <TFieldValues extends FieldValues, TContext>(
  schema: z.Schema<TFieldValues, any>,
  schemaOptions?: Partial<z.ParseParamsNoData>,
  factoryOptions?: { mode?: 'async' | 'sync' },
) => (
  values: unknown,
  context: TContext | undefined,
  options: ResolverOptions<TFieldValues>,
) => Promise<ResolverResult<TFieldValues>>;

This would actually allow not to specify the FieldValues to useForm since it is automatically infered from the zod schema:

useForm({
  resolver: zodResolver(schema),
})

jorisre
jorisre previously approved these changes Sep 18, 2021
Copy link
Member

@jorisre jorisre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution @julienfouilhe !

LGTM! I'll make some minor changes in another PR.

Sorry again for the delay.

@julienfouilhe
Copy link
Contributor Author

@jorisre I fixed the tests sorry about that. Thought I had run them.

I had to add unpacking NestedValue support to io-ts

@jorisre jorisre merged commit 10ba224 into react-hook-form:master Sep 18, 2021
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@bluebill1049
Copy link
Member

thank you @julienfouilhe & @jorisre ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yupResolver raise Types of parameters 'options' and 'options' are incompatible at react-hook-form 7.15.0
4 participants