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.71k stars 155 forks source link

fix(yup): resolve ts error when including optional key in yup schema #625

Closed kotarella1110 closed 12 months ago

kotarella1110 commented 12 months ago

Overview

This PR addresses the issue outlined in https://github.com/react-hook-form/resolvers/issues/575. The problem stems from inaccurate inference of generics (TFieldValue) in yupResolver.

For instance, when passing a schema with optional keys to yupResolver:

const schema = yup.object({ test: yup.number() });

type TestValues = yup.InferType<typeof schema>;

function Form({ values }: { values: TestValues }) {
  const methods = useForm({
    values: values,
    resolver: yupResolver(testSchema)
  });
  return null;
}

The inferred type for yupResolver looks like this:

(alias) yupResolver<{
    test: number | undefined;
}>(schema: Lazy<any, yup.Maybe<yup.AnyObject>, any> | yup.ObjectSchema<{
    test: number | undefined;
}, yup.AnyObject, any, "">, schemaOptions?: yup.ValidateOptions<...> | ... 1 more ... | undefined, resolverOptions?: {
    ...;
}): Resolver<...>
import yupResolver

As you can see, the generics (TFieldValue) for yupResolver should have been inferred as optional keys, but they were inferred as required keys instead. This issue arises from the fact that the ObjectSchema in Yup extends the underlying schema's first argument using MakeKeysOptional. This utility type makes keys optional when the value of an object property is undefined. However, because this utility type is used, the expected inference isn't achieved, resulting in keys being mistakenly inferred as required.

https://github.com/jquense/yup/blob/1ee9b21c994b4293f3ab338119dc17ab2f4e284c/src/object.ts#L99

The incorrect inference of generics (TFieldValue) was causing problems when passing it as the form type to the Resolver. To resolve this, we fixed the return type by passing the correctly inferred schema type obtained through Yup.InferType as the argument to the Resolver.

Resolved CSB

You can view the resolved example in the following CSB link: https://codesandbox.io/s/rhf-625-54jvv4

github-actions[bot] commented 12 months ago

Size Change: 0 B

Total Size: 25.4 kB

ℹ️ View Unchanged | Filename | Size | | :--- | :---: | | `ajv/dist/ajv.js` | 638 B | | `ajv/dist/ajv.module.js` | 594 B | | `ajv/dist/ajv.umd.js` | 750 B | | `arktype/dist/arktype.js` | 328 B | | `arktype/dist/arktype.module.js` | 343 B | | `arktype/dist/arktype.umd.js` | 423 B | | `class-validator/dist/class-validator.js` | 537 B | | `class-validator/dist/class-validator.module.js` | 561 B | | `class-validator/dist/class-validator.umd.js` | 640 B | | `computed-types/dist/computed-types.js` | 387 B | | `computed-types/dist/computed-types.module.js` | 401 B | | `computed-types/dist/computed-types.umd.js` | 477 B | | `dist/resolvers.js` | 467 B | | `dist/resolvers.module.js` | 482 B | | `dist/resolvers.umd.js` | 562 B | | `io-ts/dist/io-ts.js` | 1.28 kB | | `io-ts/dist/io-ts.module.js` | 1.18 kB | | `io-ts/dist/io-ts.umd.js` | 1.41 kB | | `joi/dist/joi.js` | 591 B | | `joi/dist/joi.module.js` | 609 B | | `joi/dist/joi.umd.js` | 696 B | | `nope/dist/nope.js` | 345 B | | `nope/dist/nope.module.js` | 365 B | | `nope/dist/nope.umd.js` | 434 B | | `superstruct/dist/superstruct.js` | 324 B | | `superstruct/dist/superstruct.module.js` | 342 B | | `superstruct/dist/superstruct.umd.js` | 424 B | | `typanion/dist/typanion.js` | 323 B | | `typanion/dist/typanion.module.js` | 336 B | | `typanion/dist/typanion.umd.js` | 418 B | | `typebox/dist/typebox.js` | 464 B | | `typebox/dist/typebox.module.js` | 481 B | | `typebox/dist/typebox.umd.js` | 579 B | | `valibot/dist/valibot.js` | 577 B | | `valibot/dist/valibot.module.js` | 594 B | | `valibot/dist/valibot.umd.js` | 684 B | | `vest/dist/vest.js` | 479 B | | `vest/dist/vest.module.js` | 443 B | | `vest/dist/vest.umd.js` | 569 B | | `yup/dist/yup.js` | 625 B | | `yup/dist/yup.module.js` | 639 B | | `yup/dist/yup.umd.js` | 728 B | | `zod/dist/zod.js` | 580 B | | `zod/dist/zod.module.js` | 600 B | | `zod/dist/zod.umd.js` | 687 B |

compressed-size-action

kotarella1110 commented 12 months ago

This PR does not aim to resolve https://github.com/react-hook-form/resolvers/issues/589. However, regarding this issue, as mentioned in https://github.com/orgs/react-hook-form/discussions/10618#discussioncomment-6826058, I believe using the FormValue's generics should be passed to yupResolver to address the inconsistency issue.

resolver: yupResolver<FormValues>(schema),
github-actions[bot] commented 12 months ago

:tada: This PR is included in version 3.3.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

jorisre commented 12 months ago

Thanks a ton @kotarella1110