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

Yup's resolver complains about schemas typed with Yup.Schema<MyFormType> in @hookform/resolvers v3.1.0 with yup v1.1.1 #549

Closed
danielmarcano opened this issue Apr 22, 2023 · 26 comments
Labels
question Further information is requested

Comments

@danielmarcano
Copy link

Describe the bug
When using @hookform/resolvers v3.1.0 and creating a Yup (v.1.1.1) schema, typing it as Yup.Schema<MyFormType> yupResolver complains about its type:

export type UserForm = {
  firstName?: string;
};

const formSchema: Yup.Schema<UserForm> = Yup.object().shape({
  firstName: Yup.string().optional()
});

const { control, reset } = useForm<UserForm>({
  mode: "all",
  resolver: yupResolver(formSchema),
});

To Reproduce

  1. Install yup v1.1.1 and @hookform/resolvers v3.1.0 packages

  2. Have TypeScript configured in your project

  3. Create your form's type:

export type UserForm = {
  firstName?: string;
};
  1. Create a Yup schema and type it as Yup.Schema<MyFormType>:
const formSchema: Yup.Schema<UserForm> = Yup.object().shape({
  firstName: Yup.string().optional()
});
  1. Pass your form type to useForm, and formSchema to yupResolver within your useForm's configuration object's resolver property:
const { control, reset } = useForm<UserForm>({
  mode: "all",
  resolver: yupResolver(formSchema)
});
  1. Notice the error that yupResolver is showing regarding the passed formSchema

Codesandbox link (Required)

Here's a codesandbox reproducing the issue.

Expected behavior

The yupResolver function should not complain about the given formSchema. It should consider it valid, and recognize its type, since this is considered valid code when using @hookform/resolvers v2.9.10 and yup v1.1.1.

Screenshots

image

Desktop (please complete the following information):

  • OS: iOS 13.2.1
  • Browser Chrome
  • Version 112.0.5615.137

Additional context

The codesandbox code works correctly and yupResolver does not complain about formSchema when downgrading @hookform/resolvers to v2.9.10, and using the same yup version v1.1.1.

@bluebill1049 bluebill1049 added the question Further information is requested label Apr 29, 2023
@francescovenica
Copy link

any update on this?

@henrikvolmer
Copy link
Contributor

I created a pull request to resolve this issue.

#563

jorisre pushed a commit that referenced this issue Jun 12, 2023
* Fix schema type error mentioned in issue #549 

* Improves yup type inference

Improves yup type inference to make generic casting for useForm unnecessary
@wand2016
Copy link

This change breaks existing codes...

@henrikvolmer
Copy link
Contributor

Can you explain it a bit more in detail, please?

@henrikvolmer
Copy link
Contributor

@danielmarcano : I fixed your codesandbox example.

https://codesandbox.io/s/affectionate-shadow-cym6g6?file=/src/logic.ts

@bluebill1049
Copy link
Member

Someone else reported breaking: #574

@carlos-menezes
Copy link

This change breaks existing code.

@henrikvolmer
Copy link
Contributor

I would like to help you but if you don't provide me more details about your case, I can't

@carlos-menezes
Copy link

In our use case, we have defined a type T which is composed of not only but also a salesTime object:

export type TicketTierFormValues = {
  // ...
  salesTime: {
    start: Date | string
    end: Date | string
  }
  // ...
}

Our schema is defined as:

const schema = object({
   // ...,
   salesTime: object({
     start: date()
       .required('Start date is required')
       .test(
         'valid start date',
         'Ticket tier start date must be in the future',
         (value) => {
           if (value)
             return (
               inputs?.salesTime?.start?.disabled ||
               isFuture(convertToUtc(value, timezone))
             )
           return false
         },
       ),
     end: date()
       .required('End date is required')
       .test(
         'valid end date',
         'Ticket tier end date must be in the future',
         (value, context) => {
           const startDate: Date | null | undefined = context.parent.start
           if (value)
             return (
               inputs?.salesTime?.end?.disabled ||
               isBefore(
                 startDate
                   ? convertToUtc(startDate, timezone)
                   : convertToUtc(newDate(), timezone),
                 convertToUtc(value, timezone),
               )
             )
           return false
         },
       ),
   }),
})

When invoking useForm like in the code below, we get an error (see below the code):

const methods = useForm<TicketTierFormValues>({
    defaultValues,
    mode: 'onChange',
    resolver: yupResolver(schema),
  })
Type 'Resolver<{ name: string; totalSupply: number; salesTime: { start: Date; end: Date; }; description: string | undefined; bookingNotice: string | undefined; restricted: NonNullable<boolean | undefined>; type: NonNullable<...>; price: number | undefined; }>' is not assignable to type 'Resolver<TicketTierFormValues, any>'.
 Types of parameters 'values' and 'values' are incompatible.
   Type 'TicketTierFormValues' is not assignable to type '{ name: string; totalSupply: number; salesTime: { start: Date; end: Date; }; description: string | undefined; bookingNotice: string | undefined; restricted: NonNullable<boolean | undefined>; type: NonNullable<...>; price: number | undefined; }'.ts(2322)

This code was working before. I've tried setting the type of start and end to mixed<Date | string> but to no avail.

@henrikvolmer
Copy link
Contributor

henrikvolmer commented Jun 13, 2023

@carlos-menezes : Your type is not assignable to your schema. Don't cast your useForm (with the generic <ThicketTierFormValues>). The type will be inferred by the schema. The yupResolver return type was like any before. The type check was disabled completely. That's why it worked before.

If you provide a Codesandbox example, I'll try to fix the issue for you.

@danielmarcano
Copy link
Author

Hi, @henrikvolmer, thank you for the codesandbox. I have a couple of doubts, though, since your solution basically has us keep Yup's schema as the source of truth of our form type, and that is everything we wanted to avoid with Yup.Schema<OurType>:

const formSchema = Yup.object().shape({
  firstName: Yup.string().optional()
});

export type UserForm = Yup.InferType<typeof formSchema>;

So my doubt is: is there no longer a way to create a Yup schema based on an existing type, and have it work with useForm as it used to?

export type UserForm = {
  firstName?: string;
};

const formSchema: Yup.Schema<UserForm> = Yup.object().shape({
  firstName: Yup.string().optional()
});

Isn't this considered a breaking change of v3 of @hookform/resolvers?

@henrikvolmer
Copy link
Contributor

But why you want a type, which is not completely assignable to your schema? Makes no sense to me tbh.

So you can type your schema, but this is completely unnecessary.

Here is my example: https://codesandbox.io/s/determined-dhawan-8f72hm?file=/src/logic.ts

@danielmarcano
Copy link
Author

@henrikvolmer I think it depends, but it's definitely not only to type the schema.

In our case, we define all form types in a types.ts file. It would no longer be possible to just keep types in this file if we use Yup.InferType<typeof formSchema>.

Also, these form types are used in several other places, and not all form schemas can be created outside of the custom hook which handles the form logic, as some of them even depend on variables that come as arguments of these functions, so how would we extract the types of these form schemas if we created them this way?

Last but not least, having both a form type created in our types.ts file, and the formSchema completely detached from such type, will lead to inconsistencies, and will force us to keep track of both of them, and to always remember to update the formSchema with any updates we make to the form type within the types.ts file.

Perhaps there's something I am not seeing / have the incorrect mental model about this that you could help me with?

I still think that this is a breaking change that should be documented somewhere, though, since many people depend on Yup.Schema<OurType>.

@henrikvolmer
Copy link
Contributor

henrikvolmer commented Jun 13, 2023

@danielmarcano: I truly agree with you, that this is a breaking change. This is, why I commented under my PR (#563).

To your problem:

It's really hard to analyze your problem without having the code, but you can use the Yup.InferType<typeof formSchema> everywhere in your project. Just export const formSchema = Yup.object(...); and use it, wherever you want.

@fmorenodesigns
Copy link

https://codesandbox.io/s/nifty-pine-8pl9n2?file=/src/logic.ts

@henrikvolmer I'm seeing some odd behaviour with nullable fields after this bump. It could just be lack of understanding on my part, but in the case above, why isn't the schema compatible with the form type I'm providing?

@henrikvolmer
Copy link
Contributor

@fmorenodesigns : The problem in your code is, that the property has to be exists for this schema, but can be undefined.

There is a difference between ?: and undefined.

Take a look at my example:

https://codesandbox.io/s/clever-silence-m9x39p?file=/src/logic.ts

@fmorenodesigns
Copy link

@fmorenodesigns : The problem in your code is, that the property has to be exists for this schema, but can be undefined.

There is a difference between ?: and undefined.

Take a look at my example:

https://codesandbox.io/s/clever-silence-m9x39p?file=/src/logic.ts

Ah gotcha. Thanks for the reply @henrikvolmer . But then what would the appropriate yup schema declaration for it? Seeing that yup.string().nullable().optional() isn't it.

@henrikvolmer
Copy link
Contributor

@fmorenodesigns : This isn't possible, I guess. It makes no sense anyway because with the generic you say, which possible types your schema has. If you want the type for the schema, you should use something like this:

type Inferred = Yup.InferType<typeof formSchema>

But the generic is not necessary in this case. Just leave it blank and the type should fit for you.

Example: https://codesandbox.io/s/old-pine-4r5fwg?file=/src/logic.ts

@jorisre
Copy link
Member

jorisre commented Jun 14, 2023

Here is the updated release note, sorry for the mistake, it should be at least a minor version instead of a patch

https://github.com/react-hook-form/resolvers/releases/tag/v3.1.1

@aakhtar76900
Copy link

Is there a plan to add support for defining types with useform or should we plan to change code?

@jorisre
Copy link
Member

jorisre commented Aug 17, 2023

@aakhtar76900 You can provide your own types, but it's better to let the type be inferred.

@jorisre jorisre closed this as completed Aug 17, 2023
@freiserg
Copy link

@jorisre How to add a custom type when not all fields are inferred from the form?

We use so props:

export type FormValues = {
  account: Account;
  currency: Currency;
  amount: string;
};

Account and Account types have many props.

type Account {
  accountId: string;
  name: string;
  type: Type;
  ...
}

account and currency fields are custom Select components.

We need to check that an element is selected.

const validationSchema = Yup.object().shape({
  account: Yup.object().required('Account is required'),
  currency: Yup.object().required('Currency is required'),
  amount: Yup.string().required('Amount is required.')
});

@jorisre
Copy link
Member

jorisre commented Aug 17, 2023

Can you open a new issue with a CodeSandbox?

@danielmarcano
Copy link
Author

@jorisre, @henrikvolmer I still think that passing the type to useForm should be possible, as this was a key aspect of the TypeScript + Yup workflow before version 3.1.0 for many.

Inferring the type from the schema just won't make it for many of us, as Yup's formSchema is sometimes built within a function, as it sometimes depends on variables passed to it, in when clauses, for example.

Besides that, having types as the source of truth of the form itself was key for not having to maintain several types and avoiding a disconnect between the form type and the Yup schema, so every time we updated a type, we got immediate feedback from Yup's formSchema reminding us that we needed to update the schema as well, to match the new type.

@freiserg
Copy link

@jorisre I created the issue #624

@shahbazyanartur
Copy link

Try npm i @hookform/resolvers@1.3.7 resolves issue

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

No branches or pull requests