close
Skip to content

ZodSwitch - #3171

Closed
jadamduff wants to merge 1 commit into
colinhacks:masterfrom
jadamduff:master
Closed

jadamduff wants to merge 1 commit into
colinhacks:masterfrom
jadamduff:master

Conversation

@jadamduff

@jadamduff jadamduff commented Jan 22, 2024 •

Copy link
Copy Markdown

Over the past year, I have been following the discussion around deprecating z.discriminatedUnion. The outcome of this discussion is of particular interest to me because I am working with fairly large and complex deeply nested discriminated union types that need to be validated on http request/response.

I've been waiting for some definitive word on the future of z.discriminatedUnion, but I haven't seen any so far. We have been hesitant to use it until its future is more certain, opting to use z.union in its place for the time being. However, we're also a bit nervous about using z.union long term since z.union performance degrades with more options, larger options, and especially more larger options -- all of which we will be adding to our types over the next year or so. We would love to get back to using z.discriminatedUnion for its scalability, or start using something else if a better option is made available. I'm hoping this PR will help move the conversation closer to a resolution.

This PR is an implementation of z.switch as outlined by @colinhacks in #2106.

It looks like this implementation is marginally more performant than z.discriminatedUnion on shallow object schema parsing and marginally less performant on deep schema parsing:
image
image

I really like z.switch because of its ability to discriminate on almost anything. (As pointed out by @colinhacks in #2106). FWIW I think there is room for z.switch, z.discriminatedUnion, and z.union in zod. I think they are different tools for different jobs. The right tool depends on the schema you want to build, the performance you want to achieve, and the
complexity you're willing to encounter. Having all three options provides a ton of flexibility.

Some other things to note are:

  • In this implementation, returning undefined from the switchFn will return z.ZodNever by default.
  • I've included tests that cover the applicable test cases from z.discriminatedUnion, but some additional edge case testing would be nice since you can return just about anything from the switchFn.
  • I added benchmark suites for large nested objects on z.switch, z.discriminatedUnion, and z.union to highlight how performance degrades when we're not dealing with small, simple types.
  • I did not write documentation

If this PR gains support, I would be more than happy to add more tests and write the documentation.

Thank you all for an awesome library and thanks in advance for reviewing this. I hope this can move this conversation forward in some way.

@netlify

netlify Bot commented Jan 22, 2024 •

Copy link
Copy Markdown

✅ Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 2128024
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/65af222fea4ebe0008cdf0c8
😎 Deploy Preview https://deploy-preview-3171--guileless-rolypoly-866f8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@scotttrinh

Copy link
Copy Markdown
Contributor

I think my own general feeling on this approach is that it lacks the ability for the myriad ecosystem tools to introspect the schema to build things like Open API and JSON Schema validators, or even just runtime introspection for user-land code.

My personal preference is still to add an optional "switch function" to the existing union schema as a way to short-cut the evaluation without having to lose any of the introspection capabilities (outlined here)

@colinhacks

Copy link
Copy Markdown
Owner

Thank you for this. Regrettably, the RFC this was based on has been superceded by #3407. It's possible ZodSwitch will happen eventually, but trying to improve z.union should come first, and I have high confidence it will work.

I hate to close such a well-made PR. Thanks for the work you put into this!

@colinhacks colinhacks closed this May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants