airjp73 / rvf

Easy form validation and state management for React and Remix
https://rvf-js.io
MIT License
848 stars 66 forks source link

[Feature]: Types should infer from validator #386

Closed JakeGinnivan closed 2 months ago

JakeGinnivan commented 2 months ago

What is the new or updated feature that you are suggesting?

Just trying v6, and noticed that the types of the form are not sourced from the validator, instead they are from the default values.

And if the validator/default values are mismatched there are no type check errors.

Why should this feature be included?

The validator should be the source of truth for the types, with defaultValues being constrained to those types and the returned form also using the validators type. Otherwise it's very easy for the default values and the scope calls to not match the validator, and have runtime errors.

JakeGinnivan commented 2 months ago

I think the issue is more that the input and output types are distinct. It would be great if they could be shared so they aligned.

Trying to understand what is the use case for input/output types being different?

airjp73 commented 2 months ago

One reason the input and output types are different is because validation libraries like zod can encode transformations. So the data in the form itself isn't coupled to the output type of the schema. As an example, you might have a schema like this:

const form = useForm({
  defaultValues: {
    likesPizza: 'yes' as 'yes' | 'no',
  },
  validator: withZod(
    z.object({
      likesPizza: z.union([
        z.literal('yes').transform(() => true),
        z.literal('no').transform(() => false),
      ]),
    })
  )
})

// this form might have a radio group like this
<label>
  <input {...form.getInputProps('likesPizza', { type: 'radio', value: 'yes' })} />
  Yes
</label>
<label>
  <input {...form.getInputProps('likesPizza', { type: 'radio', value: 'no' })} />
  No
</label>

In this case, inferring the data type from the validator would cause the types to require likesPizza to be a boolean, even though that isn't correct. This is how v5 handled things.

Zod also keeps track of the input type of a schema, so it would be natural to try to use this to infer the default value types. However, that also isn't accurate. A simple case where this approach breaks down is a checkbox.

const form = useForm({
  defaultValues: {
    likesPizza: true
  },
  validator: withZod(
    z.union([
      z.literal("on").transform(() => true),
      z.undefined().transform(() => false),
    ])
  )
})

<input {...form.getInputProps("likesPizza", { type: "checkbox" })} />

In this case, using the input type would require us to set the default value of likesPizza to "on" or undefined.

To dig a little deeper, this gets even trickier with controlled components. Because we don't just have an input and output type -- there's really three types.

A good example for this might be using something like react-select.

This might look like this:

const form = useForm({
  defaultValues: {
    user: { label: "John Doe", value: 123 },
  },
  validator: withZod(
    z.object({
      user: z
        .string()
        .transform(JSON.parse)
        .pipe(
          z
            .object({ label: z.string(), value: z.number() })
            .transform((option) => option.value),
        ),
    }),
  ),
});

// And your component might look like
<input {...form.getHiddenInputProps("user", { serialize: JSON.stringify })} />
<ReactSelect {...form.getControlProps("user")} />

which ends up giving us these types:

Since this middle type is intrinsically not typesafe, I'm not really sure of a way to guarantee type safety here. I'm open to discussion on this though, I've definitely been caught be mismatches between these two places in the past.

JakeGinnivan commented 2 months ago

Interesting. That was very handy, I'm gonna have a think about this and play around with some patterns in our codebase to see if I can find anything which could improve on this.

Makes sense though.

airjp73 commented 2 months ago

Since this isn't something we're able to do, I'm going to go ahead and close the issue. ^.^

JakeGinnivan commented 1 month ago

Yeah thanks @airjp73. I want to circle back around and see if I can bring useful suggestions to the table, but no need to keep this issue opened.