edmundhung / conform

A type-safe form validation library utilizing web fundamentals to progressively enhance HTML Forms with full support for server frameworks like Remix and Next.js.
https://conform.guide
MIT License
1.98k stars 107 forks source link

[getZodConstraint] Required fields in discriminatedUnion are false #683

Open dkAndFed opened 4 months ago

dkAndFed commented 4 months ago

Describe the bug and the expected behavior

Unsure if this is an error in implementation or it's intentional (unsure why though), but using getZodConstraint with z.discriminatedUnion the fields are always set to as false on required although they are required by the schema.

Conform version

v1.1.4

Steps to Reproduce the Bug or Issue

const s = z.discriminatedUnion('a', [
  z.object({
    a: z.literal('1'),
    b: z.string(), // b should be required
    z: z.object({ // z should be required
      u: z.string(), // z.u should be required
      y: z.string().optional(),
    }),
  }),
  z.object({
    a: z.literal('2'),
    c: z.string(), // c should be required
  }),
]);

Output from getZodConstraint

{
  "a": {
    "required": true
  },
  "b": {
    "required": false // should be true
  },
  "z": {
    "required": false // should be true
  },
  "z.u": {
    "required": false // should be true
  },
  "z.y": {
    "required": false
  },
  "c": {
    "required": false // should be true
  }
}

What browsers are you seeing the problem on?

No response

Screenshots or Videos

No response

Additional context

The behavior seems to be caused by the following line of code; removing it resolves the issue and produces expected required values.

https://github.com/edmundhung/conform/blob/main/packages/conform-zod/constraint.ts#L90

lifeiscontent commented 4 months ago

@dkAndFed I think you need a z.string().min(1) as outlined in the zod docs

edmundhung commented 4 months ago

It was intentional to keep only common properties as required. As it will make the form impossible to submit before JS loaded and it is too complex to update the constraint based on the current value.

dkAndFed commented 4 months ago

@edmundhung Thanks for the explanation.

I suppose when using getZodConstraint directly with useForm, as intended, it makes sense what you are saying. My use however is without @conform-to/react as I am relying on a different form library and I only need to know which fields are required.

I suppose I could write my own function to parse the schema and infer which fields are required.