-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Specific Optional Type #152
Comments
Will you accept a PR? I have a branch with my work so far. master...sylvanaar:first-class-optionals |
Go ahead and submit the PR. My initial reaction to this concept was negative because I don't like the redundancy of representation. But I'm warming up to the idea. Your work on this looks excellent! Edit: oops I see you've already submitted it 👍 I'm trying to publish the beta of Zod 2 today so I'm going to use your PR as a jumping off point. |
Practically speaking how do you want Zod's behavior to change? Zod already handles this case correctly: z.object({
name: z.string().optional()
}).parse({}) // passes
// inferred type is { name?: string }
// note the question mark |
First,. thank your considering my PR! Now - I think may seem like a minor detail. First - as you know Optional implies both missing and undefined in TS. Zod's type system only encodes the union with undefined, so it cannot currently express optionals - yet it is supported as you mention. So, because of that it handles this case incorrectly. z.object({
name: z.string().or(z.undefined()),
}).parse({}); // passes This is really a different type than the one in your example, and is inferred incorrectly as
It may seem like a minor point - but I think a core project goal is to implement the Typescript type system. Typescript handles So can we agree I have translated the 2 zod examples above properly into TS below based on what the TS rules say? type union = {
name: string | undefined;
}
type optional = {
name?: string,
}
const union: union = {} // error
const optional: optional = {} Here is a TS Playground I made to illustrate a bit more: https://tsplay.dev/yNakpW |
Gotcha. Unfortunately I prefer the current behavior for a couple reasons. If only const person = z.object({
name: z.string().optional().nullable()
}) I understand the desire to have one-to-one correspondence with the TypeScript type system but I'd rather have an API that works as expected. One potential workaround is to add a method that subtracts the question mark from the inferred type. This wouldn't affect the runtime behavior at all, just the static type. const person = z.object({
name: z.string().optional()
})
// => { name?: string | undefined }
person.noOptionals()
// => { name: string | undefined } |
Though this discussion is somewhat beside the point because this PR was already merged into Zod 2 🤙 I think having a dedicated ZodOptional class will make it easier to implement things like nested But I still don't like the idea of changing Zod's current approach to optional typing in objects. It seems like almost no one needs |
Yes - that is really not an issue for me either in alot of cases there is no difference between ? and | undefined. Basically I saw this Optional as a key enabler for other things like you did. I just wanted to be able to have optional parameters in functions so i didn't have to pass undefined for missing paramters and good error messages for optional types, and it seems like we are there. Thanks alot for all the hard work! |
Thank you too! This saved me a lot of time. The clean optional error messages in Zod 2 are 🔥
This isn't possible yet, like I mentioned in the other issue, but hopefully will be eventually. |
I was just playing around with various things, and I noticed optional-ness is reapplied to things depending on how you write it, for example if I write it this way (see below),
message
is "double optional" since that is what the code says it is. But double optional doesn't have any meaning - so it can be flattened.so i have this union which makes sense given the code below
message: z.ZodUnion<[z.ZodUnion<[z.ZodString, z.ZodUndefined]>, z.ZodUndefined]>;
can it become:
message: z.ZodUnion<[z.ZodString, z.ZodUndefined]>;
Optional is not just a union with undefined though, it also includes "missing" / "not there at all". So there seems to be something that is not being captured currently.
Unions also seem to be a weak point in the error messaging which is otherwise top notch. Having a specific optional - lets you give the error message for the optional type instead of the union which i think is a pretty big deal.
Right now all unions (so all optionals) give the same error:
vs a non-optional of the same
This difference only make sense if you know that optional was being represented by a union with undefined in which we no longer know which option in the union is our type.
Code i was experimenting with is below for reference
The text was updated successfully, but these errors were encountered: