Deprecating `z.discriminatedUnion`?

colinhacks commented 1 year ago

Superceded by https://github.com/colinhacks/zod/issues/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.

const schema = z.switch(()=>{
  return Math.random() ? z.string() : z.number()
});

I expand more on the z.switch API later. Let's talk about z.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 this instanceof 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.

The .deepPartial API has this same problem. I'm deprecating it for the same reason.

Many of those issues are asking for non-literal discriminator types:

type MyUnion = 
  | { type: "a", value: string }
  | { type: "b", value: string }
  | { type: "c", value: string }
  | { type: string, value: string }
  | { type: null, value: string }
  | { type: undefined, value: string }
  | { type: MyEnum, value: string }
  | { type: { nested: string }, value: string }
  | { type: number[], value: string };

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 incoming input.type against those options. In the general case, Zod would extract the type field from the shape of each component ZodObject and check input.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 what z.discriminatedUnion is supposed to avoid doing. (It's still doing less work than the naive z.union but still.)

Another issue is composability. The existing API expects the second argument to be an array of ZodObject schemas.

z.discriminatedUnion("type", [
  z.object({ type: z.literal("a"), value: z.string() }),
  z.object({ type: z.literal("b"), value: z.string() }),
])

This isn't composable, in that you can't nest discriminated unions or add additional members.

const ab = z.discriminatedUnion("type", [
  z.object({ type: z.literal("a"), value: z.string() }),
  z.object({ type: z.literal("b"), value: z.string() }),
]);

const abc = z.discriminatedUnion("type", [
  ab,
  z.object({ type: z.literal("c"), value: z.string() }),
]);

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 a type property, otherwise the union is no longer discriminable.

Replacement: `z.switch`

const schema = z.switch(input => {
  return (typeof input) === "string" ? z.string() : z.number();
})

schema.parse("whatever"); // string | number

A discriminated union looks like this:

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 */ });
// { key: 'a', value: 'asdf' } | { key: 'b', value: number }

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 the input will be used to select the schema. z.switch() accepts a function. The ResultType of that function is inferred. It will be the union of the schema types returned along all code paths in the function. For instance:

const schema = z.switch(()=>{
  return Math.random() ? z.string() : z.number()
});

Zod sees that the return type of the switcher function is ZodString | ZodNumber. The result of the z.switch is ZodSwitch<ZodString | ZodNumber>. The result of schema.parse(...) is string | number.

You can represent discriminated unions explicitly like this:

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 */ });
// { key: 'a', value: 'asdf' } | { key: 'b', value: number }

This can be written in a more condensed form like so:

const schema = z.switch((input) => ({
  a: z.object({ key: z.literal("a"), value: z.string() }),
  b: z.object({ key: z.literal("b"), value: z.number() }),
}[input.key as string]));

schema.parse({ key: 'a', value: 'asdf' });
// { key: 'a', value: 'asdf' } | { key: 'b', value: number }

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.

mgreenw commented 1 year ago

While I really liked the simple API of discriminatedUnion and am sad to see it go, this makes sense. Thank you for the detailed write up and your work to make a replacement that’s better!

dvargas92495 commented 1 year ago

You can represent discriminated unions explicitly like this

Any reason why discriminatedUnion can't now call the new switch method in the way you described as its new implementation?

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 key.subpath.

JacobWeisenburger commented 1 year ago

@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?

dgritsko commented 1 year ago

Currently, z.infer plays really nicely with z.discriminatedUnion. For example, given the following:

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 AB being defined as:

type AB = {
  type: "a";
  value: string;
} | {
  type: "b";
  value: string;
}

Which is exactly what I would expect. How will type inference work with z.switch -- will it support inference of "idiomatic" type definitions, similar to this example?

pato1 commented 1 year ago

I really like the new API as it opens up new options for building any validation logic. A thing to consider is the type of input for the switch function. In my opinion, you would almost always need to add switch after some preliminary validation. Given the example replacing distributed union you've provided:

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 data is an object with property key and some additional properties:

z.object({key: z.enum(['a', 'b'] as const)}).passthrough();

And then you should have the input parameter of the switch function typed accordingly (input: {key: 'a' | 'b'}).

Therefore I would suggest implementing switch similar to ZodPipeline in that it can be added to existing validation. Then it could be used as follows:

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 ZodPipeline:

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 (getScheme) parameter is not fulfilled.

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?

colinhacks commented 1 year ago

Is this somewhat related to what you have in mind for the new API?

@pato1 Almost exactly, aside from base. I like the idea of having some "pre-validation" so you get some type safety before attempting the switch logic, and a .switch() method is certainly interesting. I'd propose to go a step further and overload the .pipe() API to accept (input: unknown)=>ZodType. They're conceptually the same - switch is a dynamic pipe.

// 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 ZodSwitch class, we just look at the return type of the "switcher function", which will be some union of all the schema types that are returned along the various code paths. Inference will work as expected. cc @dgritsko

colinhacks commented 1 year ago

Unfortunately though, over the course of trying to explain the problems with z.discriminatedUnion I hit on an implementation that actually solves most of my issues with it.

In the general case, Zod would extract the type field from the shape of each component ZodObject and check input.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 what z.discriminatedUnion is supposed to avoid doing. (It's still doing less work than the naive z.union but still.)

Conceptually, we could loop over the set of options (ZodObject[]) in the discriminated union, pull out <option>.shape[discriminator], and use that to parse input[discriminator]. The first time this succeeds, we have a match, and use that option to validate the whole input. I was originally presenting this as a straw man for why ZodDiscriminatedUnion is inherently flawed...but actually it's a pretty good approach. With this, there's no need to spelunk into the internals of the union's options to extract out literal discriminator values. There will be a slight performance tradeoff relative to the current implementation since we're still executing a .parse operation for each option in the discriminated union. But most discriminator schemas should be simple & fast.

Some special logic is required to make nesting work (passing other ZodDiscriminatedUnion instances into z.discriminatedUnion), which is slightly annoying. It still may come back to bite me if people start asking to include ZodUnion, ZodEffects, ZodOptional, etc. as options in z.discriminatedUnion, at which point we're right back where we started, with a bunch of fiddly special-case logic.

colinhacks commented 1 year ago

Any reason why discriminatedUnion can't now call the new switch method in the way you described as its new implementation?

Because the hard part is extracting out the literal discriminator values from the schemas passed in ZodDiscriminatedUnion. We could rewrite z.discriminatedUnion to return a ZodSwitch but that would be a breaking change - better to just leave it as is and let people switch over at their liesure.

Or, introduce a new API that just accepts an array of schemas that then calls the switch function under the hood.

How is this different from the current API for z.discriminatedUnion? The problem is that it's hard and messy.

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.

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 z.switch actually makes this more explicit.

dvargas92495 commented 1 year ago

How is this different from the current API for z.discriminatedUnion?

I realized here what I was proposing was actually the API of z.union, which runs into the problem you described in your description.

pato1 commented 1 year ago

I'd propose to go a step further and overload the .pipe() API to accept (input: unknown)=>ZodType. They're conceptually the same - switch is a dynamic pipe.

@colinhacks I like this idea, even more now you're considering rewriting z.discriminatedUnion. But shouldn't then the "switcher function" accept (input: A["_output"])=>ZodType?

I hit on an implementation that actually solves most of my issues with it.

Also one benefit to making z.discriminatedUnion work, is generators (Zod to X) like @anatine/zod-openapi. With the switch API there would be no way of getting individual options for schema generation.

Conceptually, we could loop over the set of options (ZodObject[]) in the discriminated union, pull out

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 index(key: string): ZodType than you could just use that instead of <option>.shape[discriminator]. Every type that implemented this method (e.g. ZodDiscriminatedUnion) could then be used in discriminated union option. I can see it being useful even in other cases, for example if I want to validate only one field against some complex schema.

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:

ZodObject.index = (key) => this.shape[key]
ZodIntersection.index = (key) => z.intersection(this.left.index(key), this.right.index(key)
ZodUnion.index = (key) => z.union(this.options.map((option) => option.index(key))

sluukkonen commented 1 year ago

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.

scotttrinh commented 1 year ago

Also one benefit to making z.discriminatedUnion work, is generators (Zod to X) like @anatine/zod-openapi. With the switch API there would be no way of getting individual options for schema generation.

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 z.union really.

samchungy commented 1 year ago

+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.

teunmooij commented 1 year ago

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. In the list of open PRs there's 2 I'd like to point out:

GH-1589
GH-1845

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. You can see these put together here: https://github.com/teunmooij/zod-discriminated-union These things would not be as easily accomplished with a switch and since the before mentioned patterns of enheritance are so common, I think it would be a shame not to provide first-class support for it in Zod.

carl0s-sa commented 1 year ago

I have a use case I couldn't get working with discriminatedUnions and wanted to run it by this switch model to check if it solves it:

I have a Schema that has 2 discriminating fields (with no overlap) e.g.

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 .merge them, would the switch feature solve this use case? Or is refine better suited for this?



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()
  })
});

Zehir commented 1 year ago

I also have a use case where I use discriminatedUnions to have only one main schema and they are discriminated by the schema property on each schema. But the issue is that I can't use .refine on it. I tried to use .innerType(), typescript say it's ok but the refine method is not called. Does the switch method will help in that case ?

ellis commented 1 year ago

@colinhacks Similar to the @anatine/zod-openapi team, I also depend on zod introspect to generate my own schemas, so the idea of replacing z.discriminatedUnion with z.switch is alarming. If the API needs to become more explicit (e.g. the suggestion from @sluukkonen) , that would be fine, but please keep the current introspection capabilities.

stoneLeaf commented 1 year ago

Hi @colinhacks, and thanks for your work in this project! Have you given any more thoughts as to which way you'd like to go on this matter? Any help needed?

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 switch() API (which could exist alongside discriminatedUnion()). It would be very composable and allow much finer discriminations.

tpavlu commented 1 year ago

Unfortunately though, over the course of trying to explain the problems with z.discriminatedUnion I hit on an implementation that actually solves most of my issues with it.

In the general case, Zod would extract the type field from the shape of each component ZodObject and check input.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 what z.discriminatedUnion is supposed to avoid doing. (It's still doing less work than the naive z.union but still.)

Conceptually, we could loop over the set of options (ZodObject[]) in the discriminated union, pull out <option>.shape[discriminator], and use that to parse input[discriminator]. The first time this succeeds, we have a match, and use that option to validate the whole input. I was originally presenting this as a straw man for why ZodDiscriminatedUnion is inherently flawed...but actually it's a pretty good approach. With this, there's no need to spelunk into the internals of the union's options to extract out literal discriminator values. There will be a slight performance tradeoff relative to the current implementation since we're still executing a .parse operation for each option in the discriminated union. But most discriminator schemas should be simple & fast.

Some special logic is required to make nesting work (passing other ZodDiscriminatedUnion instances into z.discriminatedUnion), which is slightly annoying. It still may come back to bite me if people start asking to include ZodUnion, ZodEffects, ZodOptional, etc. as options in z.discriminatedUnion, at which point we're right back where we started, with a bunch of fiddly special-case logic.

I think I actually ran into this case and would love an advised pass forward. I have a helper function that uses z.preprocess to inflate a type. It's pretty simple it just takes in two zod objects and preprocesses the input of A to have camelCase properties that appear in b if they appear in a.

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? I considered using 'transform' instead of pre-process but ideally I actually wanna validate the conversion against schema B. Plus preprocess is a bit more magical to the caller.

tpavlu commented 1 year ago

@colinhacks I threw up a PR adding support for this. I tested it locally by adding my own implementation w/ my use-case and it seems to work fine. https://github.com/colinhacks/zod/pull/2455

Please LMK if you are open to accepting this change, so I can avoid forking. Or if there is any reason I might be overlooking why this change is invalid or dangerous.

Thanks TJ

cl0ckwork commented 1 year ago

👋 Thanks for all the hard work on such a useful lib!

I find myself reaching for object methods on discriminatedUnion a decent amount of the time. And been waiting on this https://github.com/colinhacks/zod/pull/1845 to meet that need.. It appears self-evident but, Im wondering how/if switch will solve this need? Or if you recommend a different approach subsetting the original object? I assume under this switch api it would come down to writing a new switch and mapping each discriminator to its declared subset, is this the intent?

example:


const foo = z.object({a: 'foo', other: 123, unique: 'abc'  })
const bar = z.object({a: 'bar', other: 123 })

// old  - never worked
const union = z.discriminatedUnion('a', [foo, bar]).pick({a: true, other: true })

// new?
z.switch((input) => {
  switch(input){
    case 'foo':
      return foo.pick({ a: true} )
    // ... etc
    }
})

dmeehan1968 commented 1 year ago

I have a use case that requires a discriminatedUnion option to be a ZodEffect:

My input object contains a property (fieldA) with a string value
The string value can be deconstructed using a regex into an object with several properties. These properties can be coerced to numbers, so we now have something like fieldA: { a: 1, b: 2, c: 3 }
In the original object, I no longer want fieldA to appear, but want to use metadata (or whatever) as the property. To do this, I would fluently add .transform(({ fieldA, ...rest }) => ({ metadata: fieldA, ...rest})) to the base schema, but this makes the schema a ZodEffect which cannot then be used in a discriminated union.

Illustration:

const schemaA = z.object({
  type: z.literal('a'),
  fieldA: z.string()
    .transform(v => v.match(/^(?<a>\d+)-(?<b>\d+)-(?<c>\d+)$/)?.groups)
    .pipe(z.object({
      a: z.coerce.number(),
      b: z.coerce.number(),
      c: z.coerce.number(),
    })
}).transform(({ fieldA, ...rest}) => ({ metadata: fieldA, ...rest }))

const schemaDU = z.discriminatedUnion('type', [ schemaA, otherSchemas ])

It's not clear from the discussion above whether its intended that either DU is enhanced to support ZodEffect or whether the z.switch proposal would allow for the output to be used as a discriminated union (from typescripts perspective).

Above tested with 3.21.4 and found to generated an error in the DU array.

jayarjo commented 1 year ago

So is switch part of the lib now? I don't see it mentioned in the docs.

markomitranic commented 1 year ago

In my case, I stumbled upon this discussion as I couldn't force discriminated union to extract schema options from a union of literals:

const NavigationItemZod = z.strictObject({
  _type: z.union([
    z.literal("navigationItemLink"),
    z.literal("navigationItemEsa"),
    z.literal("navigationItemShop"),
  ]),
  label: z.string(),
});

const NavigationItemPostZod = z.strictObject({
  _type: z.literal("navigationItemPost"),
  ref: z.string(),
});

export const NavigationZod = z.strictObject({
  _id: z.string(),
  items: z.discriminatedUnion("_type", [
      NavigationItemZod,
      NavigationItemPostZod,
    ])
});

maidi29 commented 1 year ago

@markomitranic I have the same issue. When using a union with literals in a discriminator property, I get the following error: "A discriminator value for key 'xxx' could not be extracted from all schema options".

I changed it to separate objects, so the discriminator is no union and the error disappeared.

alexandre-embie commented 1 year ago

Is this available ? I'm on 3.20.2 and can't see it.

timosaikkonen commented 11 months ago

@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 .discriminatedUnion() merged while you contemplate the future of this? #1589 would not solve my issue but it would solve many others', and seeings some movement with this might inspire others (possibly myself included) to work on further PRs.

yannick-softwerft commented 11 months ago

Is it available now? Im on 3.22.4 and still cant see it.

denizdogan commented 10 months ago

To anyone curious, z.switch has NOT been released (as of version 3.22.4)

But it would be nice if some maintainer could clarify:

Has it been decided to remove z.discriminatedUnion?
If so, has it been decided to replace it with z.switch?
If so, is the API design ready enough for someone to start working on this?

tomwidmer commented 9 months ago

Will it be possible to, say, generate a JSON Schema or OpenAPI schema from a switch schema? It seems unlikely, since there is no introspection. Would switch will break quite a bit of the zod eco-system? Obviously not all zod features are supported by all tools, but this would replace one that can usually be supported with one that can't.

tomwidmer commented 9 months ago

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 switch approach doesn't fix it AFAICT.

e.g.

interface X {
  type: 'A' | 'B';
  // other props...
  child: A | B;
}

where the type field determines which type should be in the child field.

I believe yup supports this via the when feature.

mmkal commented 8 months ago

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.

@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:

It's non-breaking, since the previous API required an array as the second argument
It's less verbose than the previous API - no need to type z.literal(...) for each schema
It's protected against duplicate keys for free
It doesn't assume anything about the types in the union - they don't have to be objects, they just need to satisfy input?.type === ... at runtime (so they'll usually be objects, but see above - we have a z.union(...) and even z.array(...) there)

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 z.switch(...) part since that doesn't exist yet).

@colinhacks would this avoid the pitfalls you mentioned about the previous discriminatedUnion implementation?

Edit: Re-reading, this is pretty much what you suggested in the more condensed form in the OP, but I think it's worth having the sugar. There's also a bug in my above implementation, in that excluding the type from the RHS of the schemas parameter would mean it would be excluded from the parse result (since z.object({a: z.string()}).parse({type: 'x', a: 'aaa'}) => {a: 'aaa'}). But if it were possible for the discriminatedUnion implementation to ensure the type prop is included in the parsed result, it's all the more useful.

samchungy commented 8 months ago

@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).

uribracha2611 commented 8 months ago

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?

jadamduff commented 8 months ago

I am also keen on finding some resolution to this conversation. I threw up a PR (#3171) that implements z.switch as outlined by @colinhacks.

I really like z.switch. It provides a ton of flexibility in type discrimination and is marginally more performant than z.discriminatedUnion in some cases.

Thoughts?

fabian-hiller commented 7 months ago

It seems to me that z.switch is exactly the same as z.lazy with the difference that z.switch passes the input of the schema when calling the getter function. Zod could pass the input to the getter of z.lazy without a breaking change. This would allow people to freely choose between z.lazy and z.discriminatedUnion.

suhjohn commented 6 months ago

Hi, will this be resolved?

imahammou commented 5 months ago

is this going somewhere ?

samchungy commented 5 months ago

is this going somewhere ?

Likely no given the above issue

colinhacks / zod