Skip to content
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

Allow more types in discriminated unions #1075

Open
dbeckwith opened this issue Apr 7, 2022 · 12 comments
Open

Allow more types in discriminated unions #1075

dbeckwith opened this issue Apr 7, 2022 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@dbeckwith
Copy link

It seems like z.discriminatedUnion currently has quite narrow requirements for what you can use it with. Here are some example use cases that seem sound to me (in the sense that the discriminator should still be able to uniquely determine the union member) but currently aren't allowed:

const A = z.object({ type: z.literal('a') });
const B = z.object({ type: z.literal('b') });
const C = z.object({ type: z.literal('c') });
const D = z.object({ type: z.literal('d').optional() });
const E = z.object({ type: z.enum(['e', 'ee', 'eee']) });

const AorB = z.discriminatedUnion('type', [A, B]);

// using another discriminated union with the same discriminator as a union member
const AorBorC = z.discriminatedUnion('type', [AorB, C]);

// the discriminator is optional in one union member
const CorD = z.discriminatedUnion('type', [C, D]);

// the discriminator is an enum
const CorE = z.discriminatedUnion('type', [C, E]);

Any chance these cases can be supported? It would great if the requirement for discriminated union members could be widened to something like "any type that has the discriminator as a field and the type of the discriminator is a subtype of string | null | undefined and is mutually exclusive between the discriminated union members", but supporting at least the cases I mentioned above would still be much appreciated.

@scotttrinh
Copy link
Collaborator

Yeah, all of those cases seem reasonable. Furthermore, true/false/Number should be valid discriminator types. Basically, it would be good to support any value that TypeScript itself supports, and I think with Map we can get the right behavior. PRs welcome!

@vlovich
Copy link

vlovich commented Apr 21, 2022

Another one is intersection:

z.discriminatedUnion('method', [
  z.intersection(
    z.object({ 'method': 'get' }),
    getSchema,
  ),
  z.intersection(
    z.object({ 'method': 'put' }),
    putSchema,
  ),
])

@vlovich
Copy link

vlovich commented Apr 21, 2022

That one can almost be solved by using .merge instead of .intersection but that's not always possible. .union/.or also poses a challenge.

@watsonsong
Copy link

It seems zod not support null and undefined discriminants.
For example I want to define the structure: microsoft/TypeScript#27631
type Result<T> = { error?: undefined, value: T } | { error: Error };

I test this code but report error:

type Result = { error?: undefined, value: string } | { error: Error };
const ResultSchema = z.discriminatedUnion('error', [
    z.object({error: z.string().min(1)}),
    z.object({error: z.undefined(), value: z.string().min(1)}),
]);

@tianhuil
Copy link

@dbeckwith, @scotttrinh, check out #1213, which at least partially solves these issues.

@stale
Copy link

stale bot commented Aug 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Aug 30, 2022
@bjwrk
Copy link

bjwrk commented Sep 21, 2022

Pinging to see if that unmarks this with the stale bot. Apologies to all receiving notifications.

@stale
Copy link

stale bot commented Nov 20, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No activity in last 60 days label Nov 20, 2022
@joshuakb2
Copy link

I would also appreciate more flexible discriminated union input types. I don't think I'll have time to look at the code for a while but I am willing to take a stab at it.

Mainly commenting to un-stale this issue because I think it's important. Right now my code works fine with regular union, but the resulting error messages are crappy, so I would rather use discriminatedUnion, but my parsers are a bit too complicated I guess.

@stale stale bot removed the stale No activity in last 60 days label Nov 21, 2022
@maxArturo
Copy link
Contributor

I ran into this earlier and thought the same thing as the commenters here. @scotttrinh @colinhacks I'd love to take a stab at submitting a PR that solves this generally.
Also @dbeckwith @tianhuil while triaging, enums seem to work as of 6ce18f3 (you can verify this with the following code in playground.ts):

  /**
   * the discriminator is an enum ==> this works as of 6ce18f3
   */
  const C = z.object({ type: z.literal("c") });
  const E = z.object({ type: z.enum(["e", "ee", "eee"]) });
  const CorE = z.discriminatedUnion("type", [C, E]);
  CorE.parse({ type: "e" });

@maxArturo
Copy link
Contributor

Please take a look at #1589, would love to know your thoughts. It addresses several of the issues listed here:

The following are now well-supported discriminator values (enum/undefined could have been already)

z.undefined()
z.null()
z.enum()
z.discriminatedUnion()

@hornta
Copy link

hornta commented Feb 16, 2023

Would it be possible to add support for using a union as a discriminator value?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
10 participants