-
Notifications
You must be signed in to change notification settings - Fork 62
Zod 4 support #840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zod 4 support #840
Conversation
ianmacartney
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great!
| >; | ||
| }; | ||
|
|
||
| function customFnBuilder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is almost identical to zod3 except for a few lines. Ok for now, but is the eventual plan to deprecate zod3 or to unify or keep as is? It's ok to keep it all separate. maybe a comment to keep them up to date is enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I added a comment in fdf6e5b
| type IsUnion<T, U extends T = T> = T extends unknown | ||
| ? [U] extends [T] | ||
| ? false | ||
| : true | ||
| : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is some more magic I don't yet understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes from https://stackoverflow.com/a/59687759/4652564. I don’t understand in detail how it works, but from my testing it looks like it works correctly.
| type IsUnknownOrAny<T> = | ||
| // any? | ||
| 0 extends 1 & T | ||
| ? true | ||
| : // unknown? | ||
| unknown extends T | ||
| ? [T] extends [unknown] | ||
| ? true | ||
| : false | ||
| : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these standard type utilities, llm-produced, or your own invention?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this one I used a LLM to solve the following problems:
- knowing whether a generic type is
any - knowing whether a generic type is
unknown
I got consistent answers after asking multiple times and my type tests pass, so I think that the solution is correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the same solution for IsAny being used here: https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/types/utils.ts
and I also see a simpler IsUnknown type in the TypeScript codebase:
so I simplified the type in a262a74
| vRequired(valueValidator), | ||
| ); | ||
|
|
||
| function extractStringLiterals( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason these helper functions can't be outside of the function body? not sure if it has an impact/ the interpreter is smart enough, but I wouldn't want every invocation of this function to create these functions if not necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they were just here to define them in the only place where they are used. I moved them to the global scope in e43940d
| > | ||
| : never; | ||
|
|
||
| function vRequired(validator: GenericValidator) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one optimization would be to return the existing validator if it's already required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I did it in 54e0622725e77f7de84673b4bc07f7edc84f5e76
|
|
||
| type NotUndefined<T> = Exclude<T, undefined>; | ||
|
|
||
| type VRequired<T extends Validator<any, OptionalProperty, any>> = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think this would be useful in the core package? or just inline it here to start?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a good idea. I can do this as a follow-up task, I created an issue at #847
Nicolapps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ianmacartney for your review!
| type IsUnion<T, U extends T = T> = T extends unknown | ||
| ? [U] extends [T] | ||
| ? false | ||
| : true | ||
| : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes from https://stackoverflow.com/a/59687759/4652564. I don’t understand in detail how it works, but from my testing it looks like it works correctly.
| >; | ||
| }; | ||
|
|
||
| function customFnBuilder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I added a comment in fdf6e5b
| vRequired(valueValidator), | ||
| ); | ||
|
|
||
| function extractStringLiterals( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they were just here to define them in the only place where they are used. I moved them to the global scope in e43940d
| > | ||
| : never; | ||
|
|
||
| function vRequired(validator: GenericValidator) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I did it in 54e0622725e77f7de84673b4bc07f7edc84f5e76
|
|
||
| type NotUndefined<T> = Exclude<T, undefined>; | ||
|
|
||
| type VRequired<T extends Validator<any, OptionalProperty, any>> = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a good idea. I can do this as a follow-up task, I created an issue at #847
| type IsUnknownOrAny<T> = | ||
| // any? | ||
| 0 extends 1 & T | ||
| ? true | ||
| : // unknown? | ||
| unknown extends T | ||
| ? [T] extends [unknown] | ||
| ? true | ||
| : false | ||
| : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this one I used a LLM to solve the following problems:
- knowing whether a generic type is
any - knowing whether a generic type is
unknown
I got consistent answers after asking multiple times and my type tests pass, so I think that the solution is correct
| type IsUnknownOrAny<T> = | ||
| // any? | ||
| 0 extends 1 & T | ||
| ? true | ||
| : // unknown? | ||
| unknown extends T | ||
| ? [T] extends [unknown] | ||
| ? true | ||
| : false | ||
| : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the same solution for IsAny being used here: https://github.com/remix-run/react-router/blob/main/packages/react-router/lib/types/utils.ts
and I also see a simpler IsUnknown type in the TypeScript codebase:
so I simplified the type in a262a74
This pull request adds full support for Zod 4 in convex-helpers.
Migration
The methods in
convex-helpers/server/zodhave been moved toconvex-helpers/server/zod3. The new Zod 4-compatible methods can be imported fromconvex-helpers/server/zod4.For convenience, some methods in
convex-helpers/server/zod(zodToConvex,zodOutputToConvex,zodToConvexFields,zodOutputToConvexFields, andwithSystemFields) now accept both Zod 3 and Zod 4 schemas. If you’re using Zod 4 in your codebase, we strongly recommend that you import directly fromconvex-helpers/server/zod4, as it simplifies the work that the TypeScript compiler needs to do.Codecs support
When using Zod 4.1, developers can benefit from Zod codecs to roundtrip values to/from functions and the database. As explained in https://www.notion.so/convex-dev/Convex-Zod-4-Codecs-integration-294b57ff32ab809fa067fe63c740ca74?source=copy_link, the code of convex-helpers doesn’t need to do anything related to codecs:
Input = Convex valueandOutput = Developer-facing valueInput = Developer-facing valueandOutput = Convex valueInput = Convex valueandOutput = Developer-facing value. (convex-helpers doesn’t provide a database wrapper that encodes values with Zod, but if you’re writing one, you would want to call the.encode()and.decode()functions of Zod schemas).This strategy allows developers to keep using
z.transform()in bothargsandreturns, while keeping support for Zod 3 and 4.0.Implementation details
zidis implemented as az.customcall with a registry that stores the table name in aWeakMap. I considered extendingz.ZodCustomorz.ZodType, but this isn’t officially supported by Zod (see https://discord.com/channels/893487829802418277/1359688107594748054/1367295316365152308).zodToConvexandzodOutputToConvexsupport circular schemas (v.any()is returned when a type is used more than once).z.templateLiteral()is now supported.z.tuple()is now supported: it might return validators that are more complex than necessary (e.g.[number, number]will be converted tov.union(v.number(), v.number())).z.literal()now supports literals with multiple values (e.g.z.literal([1, 2, 3])) correctly.z.tuple()andz.literal(), the output type is loosened to not assume a specific order in the resultingVUnion. This is necessary since the type information in Zod only contains a union, which don’t have a guaranteed order in TypeScript. The Zod 3 implementation didn’t handle this correctly (but wasn’t updated).z.record()now usesv.object()when the key is a literal (or union of literals), sincev.union(v.literal(…), …)isn’t a valid Convex validator.Acknowledgments
This pull request is inspired by the work done by @ksinghal in #818 and by @natedunn in vendpark#1 and vendpark#2. Thanks to both of you!
Co-Authored-By: Karan Singhal [email protected]
Co-Authored-By: Nate Dunn [email protected]