-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Deprecating z.discriminatedUnion
?
#2106
Comments
While I really liked the simple API of |
Any reason why Or, introduce a new API that just accepts an array of schemas that then calls the switch function under the hood. One strength of zod is how closely defining schemas resembles defining the typescript types. Requiring a function to be defined in userland for unions deviates from how that user would define the discriminated union in standard typescript. To solve the recursive issue, there could be a requirement that only allows top level discriminators, or that the discriminator defined would need to define the full path to the key in a lodashy way, like |
@colinhacks thanks for making a better system! Can't wait to try it out. Do you have an idea of when it will be released? |
Currently, const ab = z.discriminatedUnion("type", [
z.object({ type: z.literal("a"), value: z.string() }),
z.object({ type: z.literal("b"), value: z.string() }),
]);
type AB = z.infer<typeof ab> This will result in type AB = {
type: "a";
value: string;
} | {
type: "b";
value: string;
} Which is exactly what I would expect. How will type inference work with |
I really like the new API as it opens up new options for building any validation logic. const schema = z.switch((input) => {
switch(input.key){
case "a":
return z.object({ key: z.literal("a"), value: z.string() })
case "b":
return z.object({ key: z.literal("b"), value: z.number() })
default:
return z.never()
}
});
schema.parse({ /* data */ }); you would probably want to validate z.object({key: z.enum(['a', 'b'] as const)}).passthrough(); And then you should have the Therefore I would suggest implementing switch similar to const schema = z.object({key: z.enum(['a', 'b'] as const)}).passthrough().switch((input) => {
// Here `input` is of type `{key: 'a' | 'b'}`
switch(input.key){
case "a":
return z.object({key: z.literal("a"), value: z.string()})
case "b":
return z.object({key: z.literal("b"), value: z.number()})
default:
return z.never()
}
});
schema.parse({ /* data */ }); Inability to compose distributed unions led me to build a temporary switch type based on the implementation of export interface ZodSwitchDef<T extends ZodTypeAny, B extends ZodTypeAny = ZodType<unknown>> extends z.ZodTypeDef {
getSchema: (value: B['_output']) => T
base: B
typeName: 'ZodSwitch'
}
export class ZodSwitch<
T extends ZodTypeAny,
B extends ZodTypeAny
> extends ZodType<T['_output'], ZodSwitchDef<T, B>, B['_input']> {
_parse(input: z.ParseInput): z.ParseReturnType<any> {
const {ctx} = this._processInputParams(input)
if (ctx.common.async) {
const handleAsync = async () => {
const inResult = await this._def.base._parseAsync({
data: ctx.data,
path: ctx.path,
parent: ctx,
})
if (inResult.status !== 'valid') return z.INVALID
return this._def.getSchema(inResult.value)._parseAsync({
data: inResult.value,
path: ctx.path,
parent: ctx,
})
}
return handleAsync()
} else {
const inResult = this._def.base._parseSync({
data: ctx.data,
path: ctx.path,
parent: ctx,
})
if (inResult.status !== 'valid') return z.INVALID
return this._def.getSchema(inResult.value)._parseSync({
data: inResult.value,
path: ctx.path,
parent: ctx,
})
}
}
static create<T extends ZodTypeAny, B extends ZodTypeAny>(
getSchema: (value: B['_output']) => T,
base: B,
): ZodSwitch<T, B> {
return new ZodSwitch({
base,
getSchema,
typeName: 'ZodSwitch',
})
}
}
// ...
class ZodType {
switch<T extends ZodTypeAny>(getSchema: (value: Output) => T): ZodSwitch<T, this> {
return ZodSwitch.create(getSchema, this);
}
} Note, unlike ZodPipeline, if "base" validation failed, I had to return INVALID, as requirement for the switch function ( Inference works as expected: const ab = ZodSwitch.create(({type}) => {
switch (type) {
case 'a':
return z.object({type: z.literal('a'), value: z.string()})
case 'b':
return z.object({type: z.literal('b'), value: z.number()})
default:
return z.never()
}
}, z.object({type: z.enum(['a', 'b'] as const)}).passthrough())
type AB = z.infer<typeof ab>
// type AB is {type: "a", value: string} | {type: "b", value: number} Is this somewhat related to what you have in mind for the new API? |
@pato1 Almost exactly, aside from // side node: you don't need `as const` in `z.enum`
z.object({key: z.enum(['a', 'b'])}).pipe((input) => {
switch(input.key){
case "a":
return z.object({key: z.literal("a"), value: z.string()})
case "b":
return z.object({key: z.literal("b"), value: z.number()})
default:
return z.never()
}
}); As with your |
Unfortunately though, over the course of trying to explain the problems with
Conceptually, we could loop over the set of options ( Some special logic is required to make nesting work (passing other |
Because the hard part is extracting out the literal discriminator values from the schemas passed in
How is this different from the current API for
Remember that there is no syntax for "discriminated unions" in typescript. There are just unions. Zod only includes a separate API as a performance optimization when parsing unions, and I think |
I realized here what I was proposing was actually the API of |
@colinhacks I like this idea, even more now you're considering rewriting
Also one benefit to making
This is just an idea, bear with me. One other way to achieve that even for other types of Options that closely resembles TypeScript would be to provide "indexed access". If types had a method, let's say Yes, it would be calling possibly deep for some complex schemas, but it would use public API of those types, instead of poking around in internals. Pseudo examples:
|
FWIW, the issue can also be solved with an API like this z.discriminatedUnion("type", [
["a", z.object({ type: z.literal("a"), value: z.string() })],
["b", z.object({ type: z.literal("b"), value: z.string() })],
]) It's a bit more verbose, but the implementation doesn't have to assume anything about the structure of the inner types. |
z.discriminatedUnion
z.discriminatedUnion
?
I think this is a pretty important point that we shouldn't overlook: At the very least we should provide some avenue to annotate the class in a way that allows for introspection of all of the possible types, perhaps as an opt-in? Or something really unwieldy like providing an array of schemas and then presenting the function with those schemas as arguments. z.switch(
[z.number(), z.string()],
(input, [number, string]) =>
Math.random() > 0.5 ? number : string
); Edit: I mean, this just feels like a new optional capability to add to |
+1 on the need to be able to introspect if possible. I contribute to @asteasolutions/zod-to-openapi and my own zod-openapi and we rely on being able to grab the options to be able to generate schema. |
I believe Discriminated union conceptually has a lot going for it. There's lots or scenario's where similar (inherited / sibling) types are involved. And because of the fact that for the discriminated union it's a given that all the available options have a lot of similarity it provides the opportunity to easily extend en or modify the underlying options. Even if the underlying types are hidden away in an external library.
The first adds the ability to use nested discriminated unions, which also makes it possible to add a new option to an existing discriminated union. The second adds most of the functionality available on ZodObject, which allows you to perform schema manipulations on all the underlying schema's in a single statement. |
I have a use case I couldn't get working with I have a Schema that has 2 discriminating fields (with no overlap) Promotion {
name // this stays the same
// first set of dependant values
valueType // either 'percentage' or 'fixed_amount'
value // if 'valueType' is 'percentage' has min(0).max(100)
// second set of dependant values
targeted // boolean
targetCode // if 'targeted' then required else optional
} I tried having 2 discriminated unions, one for each set and I couldn't const schema = z.switch((input) => {
return z.object({
name: z.string(),
valueType: z.enum(['percentage', 'fixed_amount']),
value: input.valueType === 'percentage' ? z.number().min(0).max(100) : z.number().min(0),
targeted: z.boolean(),
targetCode: input.targeted ? z.string() : z.string().optional()
})
}); |
I also have a use case where I use |
@colinhacks Similar to the @anatine/zod-openapi team, I also depend on zod introspect to generate my own schemas, so the idea of replacing |
Hi @colinhacks, and thanks for your work in this project! I'm currently working on a production application, and I'd love to use Zod but I need much better discriminated unions. I really like your proposed |
I think I actually ran into this case and would love an advised pass forward. Example: // Pseudo Code
function inflate(a: z.ZodObject<A>, b: z.ZodObject<B>) {
return z.preprocess((data) => {
Object.keys(b.shape).forEach((key) => {
if (snakeCase(key) in data) {
data[key] = data[snakeCase(key))
}
})
}, a.merge(b.shape))
} I have an api schema that uses this type in a discriminateUnion previously but I cannot use that anymore because preprocess returns a ZodEffect. const aSchema = z.object({ test_value: z.string(), type: z.nativeEnum(EnumType) })
const bSchema = z.object({ testValue: z.string() })
const combineSchema = inflate(aSchema, bSchema)
const unionSchema = z.discriminatedUnion('type', [
combineSchema,
anotherSchemaOfTheEnum
]) Is there a pattern that would be more sustainable here? |
Is this available ? I'm on 3.20.2 and can't see it. |
@colinhacks Anything happening with this one? Just found myself trying to add an intersection schema into a discriminated union. Can we not get improvements to |
Is it available now? Im on 3.22.4 and still cant see it. |
To anyone curious, But it would be nice if some maintainer could clarify:
|
) ## Summary <!-- Succinctly describe your change, providing context, what you've changed, and why. --> Add the ability to use [`z.discriminatedUnion()`](https://zod.dev/?id=discriminated-unions) and [`z.union()`](https://zod.dev/?id=unions) (as long as it's only a union of valid types). Note that `z.discriminatedUnion()` will likely be deprecated in lieu of `z.switch()`, though this API is not yet available. See colinhacks/zod#2106. ## Checklist <!-- Tick these items off as you progress. --> <!-- If an item isn't applicable, ideally please strikeout the item by wrapping it in "~~"" and suffix it with "N/A My reason for skipping this." --> <!-- e.g. "- [ ] ~~Added tests~~ N/A Only touches docs" --> - [ ] ~~Added a [docs PR](https://github.com/inngest/website) that references this PR~~ N/A - [x] Added unit/integration tests - [x] Added changesets if applicable ## Related <!-- A space for any related links, issues, or PRs. --> <!-- Linear issues are autolinked. --> <!-- e.g. - INN-123 --> <!-- GitHub issues/PRs can be linked using shorthand. --> <!-- e.g. "- inngest/inngest#123" --> <!-- Feel free to remove this section if there are no applicable related links.--> - Fixes #396 - colinhacks/zod#2106
Will it be possible to, say, generate a JSON Schema or OpenAPI schema from a |
My use case is to have a type field in the parent object (so in a sibling property) - this is messy using the current discriminated union, and the e.g.
where the I believe |
@sluukkonen what about this: z.discriminatedUnion('type', {
a: z.object({ value: z.string() }),
b: z.object({ value: z.number() }),
c: z.union([z.object({ foo: z.string() }), z.object({ bar: z.number() })]),
d: z.array(z.boolean()),
}) Advantages:
The internal implementation could be something like export const discriminatedUnion = <
Discriminator extends string,
Schemas extends Record<string | number | symbol, z.ZodType>
>(
discriminator: Discriminator,
schemas: Schemas
): { [K in keyof Schemas]: z.infer<Schemas[K]> & { [D in Discriminator]: K } }[keyof Schemas] => {
return z.switch(input => {
return input?.type in schemas ? schemas[input?.type] : z.never()
})
} Here's a typescript playground of the types working (minus the actual runtime @colinhacks would this avoid the pitfalls you mentioned about the previous Edit: Re-reading, this is pretty much what you suggested in the |
@mmkal The only problem I see with that is that you wouldn't be able to support discriminated unions on non-literal discriminator types like the OP states. Though to be honest, I'm personally okay with that as a compromise if it makes maintaining this form of discriminated union easier. Mainstream validators like JSON Schema don't support that anyway I believe (correct me if I'm wrong). |
This sounds like a really cool idea! Could someone provide an update on the current status of this issue? Is there an active pull request, and perhaps share when it might be integrated into the library? |
I am also keen on finding some resolution to this conversation. I threw up a PR (#3171) that implements I really like Thoughts? |
It seems to me that |
Hi, will this be resolved? |
is this going somewhere ? |
Likely no given the above issue |
Superceded by #3407
I'm planning to deprecate
z.discriminatedUnion
in favor of a "switch" API that's cleaner and more generalizable. You can dynamically "switch" between multiple schemas at parse-time based on the input.I expand more on the
z.switch
API later. Let's talk aboutz.discriminatedUnion
.Why
z.union
naively tries each union element until parsing succeeds. That's slow and bad. Zod needed some solution.z.discriminatedUnion
was a mistake. The API was good but I had reservations about the implementation. It required fiddly recursive logic to exrtract a literal discriminator key from each union element.It's a bad sign when a method or class requires weird recursive traversal of other schemas. For starters, Zod is designed to be subclassable. Users can theoretically subclass
ZodType
to implement custom schema types. But logic like thisinstanceof
switch statement don't and can't account for any user-land schema types.But the main problem is just that this kind of pattern is bad and introduces a lot of edge cases. It means that only certain kinds of schemas are allowed as discriminators, and others will fail in unexpected ways. There are now dozens of issues that have been opened regarding these various edge cases. The PRs attempting to solve this problem are irredeemably complex and introduce even more edge cases.
discriminatedUnion
withlazy
? #1504discriminatedUnion
errors withz.object().transform()
#1477.union()
vs.discriminatedUnion()
#1424discriminatedUnion
produces TS error when.default
or.preprocess
are applied #1490Many of those issues are asking for non-literal discriminator types:
Imagine each of those elements are represented with Zod schemas. Zod would need to extract the
type
field from each of these elements and find a way to match the incominginput.type
against those options. In the general case, Zod would extract thetype
field from the shape of each componentZodObject
and checkinput.type
against those schemas until a match is found. At that point, we're back to doing a parse operation for each element of the union, which is whatz.discriminatedUnion
is supposed to avoid doing. (It's still doing less work than the naivez.union
but still.)Another issue is composability. The existing API expects the second argument to be an array of
ZodObject
schemas.This isn't composable, in that you can't nest discriminated unions or add additional members.
Yes, Zod could support both
(ZodObject | ZodDiscriminatedUnion)[]
as union members, but that requires additional messy logic that reflects a more fundamental problem with the API. It also makes increasingly difficult to enforce typesafety on the union - it's important that all union elements have atype
property, otherwise the union is no longer discriminable.Replacement:
z.switch
A discriminated union looks like this:
Ultimately the
z.switch
API is a far more explicit and generalizable API. Zod doesn't do any special handling. The user specifies exactly how theinput
will be used to select the schema.z.switch()
accepts a function. TheResultType
of that function is inferred. It will be the union of the schema types returned along all code paths in the function. For instance:Zod sees that the return type of the switcher function is
ZodString | ZodNumber
. The result of thez.switch
isZodSwitch<ZodString | ZodNumber>
. The result ofschema.parse(...)
isstring | number
.You can represent discriminated unions explicitly like this:
This can be written in a more condensed form like so:
It's marginally more verbose. It's also explicit, closes 30+ issues, eliminates a lot of hairy logic, and lets Zod represent the full scope of TypeScript's type system.
z.discrimininatedUnion
is too fragile and causes too much confusion so it needs to go.The text was updated successfully, but these errors were encountered: