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

Discriminated unions became incompatible with zod.input #1196

Closed
azhiv opened this issue Jun 8, 2022 · 8 comments
Closed

Discriminated unions became incompatible with zod.input #1196

azhiv opened this issue Jun 8, 2022 · 8 comments
Labels
wontfix This will not be worked on

Comments

@azhiv
Copy link
Contributor

azhiv commented Jun 8, 2022

The issue is reproducible with Typescript v. 4.6+ and is very likely caused by its new feature called Control Flow Analysis for Destructured Discriminated Unions.
Here is a sandbox that reproduces the error. Run yarn build in the terminal tab.
Essentially, here's the compiler error that I get when trying to reference the result of zod.input that was applied to a discriminated union type constructed using zod.union (classic way):

image

And nothing changes if I use the brand new zod.discriminatedUnion:

image

The issue is not reproducible in Typescript v. 4.5 and older.

@azhiv
Copy link
Contributor Author

azhiv commented Jun 10, 2022

The error seems to be related to the changes in Typescript starting from v4.6. I submitted an issue to track it.

@azhiv azhiv closed this as completed Jun 10, 2022
@azhiv azhiv reopened this Jun 14, 2022
@azhiv
Copy link
Contributor Author

azhiv commented Jun 16, 2022

The TS guys claim that this is a correct behaviour from their side, so that { foo: T | U } is not compatible with { foo: T } | { foo: U }.
In my case zod.input results in type { flag: true } | { flag: false } which is not compatible with the Model having { flag: boolean }.
Is there a way to make infer work in a way to return { flag: boolean } instead of { flag: true } | { flag: false }? Can this be made recursively for all nested types (for example inside of an array { flag: true } | { flag: false }[] => { flag: boolean }[])?

@scotttrinh
Copy link
Collaborator

Yeah, I agree with them that this is correct behavior and I'm not familiar with any way at all we could widen the type that is created to be explicitly narrowed like this. If you want your type to be { flag: boolean } you'll need to make that the type. It's possible that what you're modeling isn't a union at all, maybe we can make some modeling suggestions?

@azhiv
Copy link
Contributor Author

azhiv commented Jun 16, 2022

We use the approach of matching the type inferred from the schema with the real type that is passed to the validator. Having it this way ensures that the schema and the model are in sync, so for example the following code yields an error:

class ModelEditor<TModel extends input<S>, S extends ZodType<unknown>> {
  constructor(public model: TModel, public schema: S) {}
}

const schema = zod.object({
  foo: zod.number()
})
class Model {
  foo: number | undefined;
}
export class AdvancedEditor extends ModelEditor<Model, typeof schema> {}
/*
Type 'Model' does not satisfy the constraint '{ foo: number; }'.
  Types of property 'foo' are incompatible.
    Type 'number | undefined' is not assignable to type 'number'.
      Type 'undefined' is not assignable to type 'number'.ts(2344)
*/

You see, this provides consistency for the schema-model pair.
We used the same approach for schemas that contain discriminated unions. The latter was utilised in order to turn validation on and off for certain parts of the schemas. For example:

const schema = zod.union([
  zod.object({
    flag: zod.literal(true),
    foo: zod.string().min(1),
  }),
  zod.object({
    flag: zod.literal(false),
    foo: zod.string().optional(),
  })
]);

schema.parse({ flag: false });                // succeeds
schema.parse({ flag: true });                 // fails
schema.parse({ flag: true, foo: '' });        // fails
schema.parse({ flag: true, foo: 'bar' });     // succeeds

So now (TS 4.6+) it's no longer possible to define an ordinary Model class that corresponds to a discriminated union provided by zod schema.

@scotttrinh
Copy link
Collaborator

So now (TS 4.6+) it's no longer possible to define an ordinary Model class that corresponds to a discriminated union provided by zod schema.

Isn't the model class:

interface FlagTrue {
  flag: true;
  foo: string;
}

interface FlagFalse {
  flag: false;
  foo?: string;
}

type Flag = FlagTrue | FlagFalse

—?

That is precisely the type-level type that the runtime schema is representing. If your type is:

interface Flag {
  flag: boolean;
  foo?: string;
}

I think you'll want to make your schema reflect that rather than trying to make a union do something clever here that just happened to work before due to TypeScript not having good control-flow semantics with discriminated unions. In other words: Your model (if it must be defined outside of Zod) should actually match the Zod schema. If it doesn't, you'll have problems like this and others as the type system and runtime system try to keep matching each other.

@azhiv
Copy link
Contributor Author

azhiv commented Jun 23, 2022

I get your point now.
However, - just in case someone will get interested - I've found a way to combine { flag: true } | { flag: false } into { flag: boolean } using ts-toolbelt, namely Union.Merge<{ flag: true } | { flag: false }> gives { flag: boolean }. The problem is, this procedure is flat, i.e. the effect is only applied to the immediate properties of the object and doesn't go further to the children. I posted a question to the community on how to achieve this by the means of that library, but received no suggestions yet.
That's why I asked if it is possible to do it inside Zod itself as you guys have access to the whole object structure. Thanks anyway!

@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
@azhiv
Copy link
Contributor Author

azhiv commented Oct 7, 2022

The TS team confirmed a bug on their side, which was fixed in v4.8

@azhiv azhiv closed this as completed Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants