zod
zod copied to clipboard
#1171 support for refine, superRefine, transform and lazy in discriminatedUnion
Fix #1171
Also add support for lazy, refine and superRefine in discriminatedUnions
Deploy Preview for guileless-rolypoly-866f8a ready!
| Name | Link |
|---|---|
| Latest commit | c39d1746f179d39290867566953bf76d54d83303 |
| Latest deploy log | https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/63734fd48b336a00080d614f |
| Deploy Preview | https://deploy-preview-1290--guileless-rolypoly-866f8a.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
@roblabat Looking good! I'll try to take some time in the next day or two to look a little more deeply at the implementation here, but first quick pass looks promising, thanks for taking this on.
Just ran into this problem, anything holding up this PR?
Unfortunately I don't think getting this clever with extraction of literal values is wise. I modified this PR to make z.discriminatedUnion moderately more robust, but all union options still have to be ZodObject instances. Technically your approach that unwraps ZodEffects isn't sound, since ZodEffect is the class that contains preprocess/transform logic. The discriminator key must be available on the input at the outset in order to determine which option to use. It must also exist in the Output type of the schema, otherwise the discriminated union has been broken. Supporting ZodEffect instances jeopardizes both of these and increases the odds someone shoots themselves in the foot here.
I dislike a lot of things about z.discriminatedUnion, but mostly that it's implementation depends on the internals of a bunch of first-party Zod types. In theory, anyone could subclass ZodType to create their own Zod schema classes, but using all this internal logic breaks portability. Approaches like the one in this PR make that even more true, which is why I'm slightly allergic to it.
If you want complex unions with refine, transforms, lazy, etc, I'd recommend just using z.union. ZodDiscriminatedUnion is intended as an optimization for simple cases.
Unfortunately I don't think getting this clever with extraction of literal values is wise. I modified this PR to make
z.discriminatedUnionmoderately more robust, but all union options still have to beZodObjectinstances. Technically your approach that unwrapsZodEffectsisn't sound, sinceZodEffectis the class that contains preprocess/transform logic. The discriminator key must be available on the input at the outset in order to determine which option to use. It must also exist in theOutputtype of the schema, otherwise the discriminated union has been broken. SupportingZodEffectinstances jeopardizes both of these and increases the odds someone shoots themselves in the foot here.I dislike a lot of things about
z.discriminatedUnion, but mostly that it's implementation depends on the internals of a bunch of first-party Zod types. In theory, anyone could subclassZodTypeto create their own Zod schema classes, but using all this internal logic breaks portability. Approaches like the one in this PR make that even more true, which is why I'm slightly allergic to it.If you want complex unions with refine, transforms, lazy, etc, I'd recommend just using
z.union. ZodDiscriminatedUnion is intended as an optimization for simple cases.
@colinhacks I don't understand why the discriminator key should be available in the output of the discriminated union. DiscriminatedUnion is just an optimisation of unions with this discriminator key helping the code to find the correct type apply quicker. But like in a union the output doesn't have to be one inputs but one of the outputs of all types of the union if no output type of the discriminated union have the discriminator key I don't see why it should in the output has it already made it's office selecting the correct type of the union.
By the way I'm not really happy with the implementation like you as it's much more a work around to make it work with ZodEffect and ZodLazy where it should be working by checking the key of the Input of the ZodType to really make it's office. But I currently don't have a solution to do that properly.
But To conclude for me the PR should be reopened as your commit revert all the implementation of the PR and related issues or no more solved by this merge and should then be reopened as this merge doesn't solve issues asking for a discriminatedUnion working with transform, refine, superRefine and lazy.
ps: Even if using unions could work I think discriminatedUnion is a good optimisation pattern that should be more than just a quick a dirty solution working on just objects. It can be really powerfull especially with transform.
Release notes say "There are no breaking API changes", but there are at least 2 breaking changes in this pull request:
-
The signature of
ZodDiscriminatedUnionwas changed fromclass ZodDiscriminatedUnion< Discriminator extends string, DiscriminatorValue extends Primitive, Option extends ZodDiscriminatedUnionOption<Discriminator, DiscriminatorValue>>toclass ZodDiscriminatedUnion< Discriminator extends string, Options extends ZodDiscriminatedUnionOption<Discriminator>[]> -
optionsproperty ofZodDiscriminatedUnionwas renamed tooptionsMapand newoptionsproperty introduced, having different type.