react-hook-form / resolvers

📋 Validation resolvers: Yup, Zod, Superstruct, Joi, Vest, Class Validator, io-ts, Nope, computed-types, typanion, Ajv, TypeBox, ArkType, Valibot, effect-ts and VineJS
https://react-hook-form.com/
MIT License
1.7k stars 155 forks source link

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 closed 1 year ago

danielmarcano commented 1 year ago

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):

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.

francescovenica commented 1 year ago

any update on this?

henrikvolmer commented 1 year ago

I created a pull request to resolve this issue.

https://github.com/react-hook-form/resolvers/pull/563

wand2016 commented 1 year ago

This change breaks existing codes...

henrikvolmer commented 1 year ago

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

henrikvolmer commented 1 year ago

@danielmarcano : I fixed your codesandbox example.

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

bluebill1049 commented 1 year ago

Someone else reported breaking: https://github.com/react-hook-form/resolvers/issues/574

carlos-menezes commented 1 year ago

This change breaks existing code.

henrikvolmer commented 1 year ago

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

carlos-menezes commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@danielmarcano: I truly agree with you, that this is a breaking change. This is, why I commented under my PR (https://github.com/react-hook-form/resolvers/pull/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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

jorisre commented 1 year ago

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

freiserg commented 1 year ago

@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 commented 1 year ago

Can you open a new issue with a CodeSandbox?

danielmarcano commented 12 months ago

@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 commented 12 months ago

@jorisre I created the issue https://github.com/react-hook-form/resolvers/issues/624

shahbazyanartur commented 2 months ago

Try npm i @hookform/resolvers@1.3.7 resolves issue

OldManMeta commented 1 month ago

This may have been listed as closed but at v1.4.0 I found all my existing code broken.

I'm not sure how others are using this, but in my code, my type definition of the form object is the master - any other library or usage of that type must infer it correctly and entirely beyond simple primitives, not require a duplication of type declarations etc, which is just asking for trouble on large solutions.

Thank you @shahbazyanartur - that version certainly works.