-
Notifications
You must be signed in to change notification settings - Fork 45
Fix incorrect type inference: ensure ZodString instead of generic ZodType<string> on convexToZod function types #692
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
base: main
Are you sure you want to change the base?
Conversation
…nvexToZod function output
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.
Looking really good so far.
Can you add some type tests in zod.test.ts
, and especially ensure branded and optional types work?
: V extends VLiteral<any, any> | ||
? z.ZodLiteral<V["value"]> |
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.
Could this be VLiteral<infer T, any>
instead of using V["value"]
?
: V extends VObject<any, any, any, any> | ||
? z.ZodObject<{ | ||
[K in keyof V["fields"]]: ZodValidatorFromConvex< |
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.
Same here for extracting the type instead of using V["fields"]
- though maybe your version avoids some inference challenges I'm unaware of? wdyt?
: V extends VString<any, any> | ||
? z.ZodString |
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 we lose branded types here? e.g. the
brandedStringhelper results in a
VString<string & {_brand: "foo" },type - can we pass that through to a more specific
ZodString` type?
@@ -1393,7 +1444,9 @@ export function convexToZod<V extends GenericValidator>( | |||
throw new Error(`Unknown convex validator type: ${convexValidator.kind}`); | |||
} | |||
|
|||
return isOptional ? z.optional(zodValidator) : zodValidator; | |||
return isOptional | |||
? (z.optional(zodValidator) as ZodValidatorFromConvex<V>) |
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 optional types going through? I fear above that we're losing that.
commit: |
What This PR Does
This PR fixes an issue where the inferred type of a Zod schema is too generic (
ZodType<string, ZodTypeDef, string>
), which prevents chaining string-specific methods such as.min()
,.email()
, etc.Problem
When conditionally constructing a schema, the inferred type ends up being the generic
ZodType<string, ZodTypeDef, string>
instead of the more specificZodString
. As a result, methods like.min()
are not recognized by TypeScript:Closes #671