forge42dev / remix-hook-form

Open source wrapper for react-hook-form aimed at Remix.run
MIT License
329 stars 26 forks source link

[suggestion] Implicit Inference of Form Schema in `getValidatedFormData` and `validateFormData` #55

Closed MonsterDeveloper closed 9 months ago

MonsterDeveloper commented 9 months ago

Hello!

I am currently exploring the remix-hook-form library and observed that when utilizing getValidatedFormData and validateFormData, the form schema needs to be explicitly included in the generic.

Considering that both functions require a resolver, it seems feasible to allow automatic inference of the T type from the resolver.

I could open a PR to introduce this enhancement. But before that, I would appreciate some clarification if there is a specific reason for not implementing this feature that I might be unaware of.

AlemTuzlak commented 9 months ago

@MonsterDeveloper No real reason, I actually didn't even think about using the resolver type to resolve it because I relied on the schemas themselves. Would love this PR

MonsterDeveloper commented 9 months ago

Played a bit with the types, turns out inferring the resolver output type is not possible as the resolver doesn't expose its schema:

export type Resolver = <T extends z.Schema<any, any>>(schema: T, schemaOptions?: Partial<z.ParseParams>, factoryOptions?: {
    // ...
    // no `T` here
}) => <TFieldValues extends FieldValues, TContext>(values: TFieldValues, context: TContext | undefined, options: ResolverOptions<TFieldValues>) => Promise<ResolverResult<TFieldValues>>;

So when we're doing typeof resolver it doesn't allow to access the schema that was passed to the resolver initially. This is a bug in react-hook-form.

AlemTuzlak commented 9 months ago

Does it return the data as typed? If so there would be a way to do it 🤔

MonsterDeveloper commented 9 months ago

That's the thing — it doesn't. So no way of getting the type of data passed to the resolver function. Not now, at least.

AlemTuzlak commented 9 months ago

@MonsterDeveloper let me look into this, I have an idea

MonsterDeveloper commented 9 months ago

I've looked a bit at the source code of the @hookform/resolvers, and it seems to me that only some resolvers (e.g. zodResolver or arktypeResolver) don't infer the schema type. Some resolvers, like yupResolver, do:

// No generics for inference
export const arktypeResolver: Resolver =
  (schema, _schemaOptions, resolverOptions = {}) =>
  (values, _, options) => {
// Return type is inferred
export function yupResolver<TFieldValues extends FieldValues>(
  schema:
    | Yup.ObjectSchema<TFieldValues>
    | ReturnType<typeof Yup.lazy<Yup.ObjectSchema<TFieldValues>>>,
  schemaOptions: Parameters<(typeof schema)['validate']>[1] = {},
  resolverOptions: {
    // skipped
): Resolver<Yup.InferType<typeof schema>> {

For zodResolver there're already requests to make it generic: react-hook-form/resolvers#447 react-hook-form/resolvers#429 react-hook-form/resolvers#551, but there isn't any progress.

While we could implement inferring resolver's FieldValues in remix-hook-form, it'll still only work with resolvers that do support schema inference.

antlanc7 commented 9 months ago

In fact, not even react-hook-form itself is inferring the type of the form data from the resolver, but only from the explicit generic or from default values.