StefanTerdell / zod-to-json-schema

Converts Zod schemas to Json schemas
ISC License
917 stars 76 forks source link

improve output for discriminatedUnion: use allOf with if-then conditionals instead of anyOf #135

Closed thomasjahoda closed 3 months ago

thomasjahoda commented 3 months ago

This is a proposal for changing the output of discriminated unions. allOf with if-then conditionals seem to have better auto-completion support, should be much more efficient during validation and are more strict. Using them, we would model discriminated unions in the exact same way as Zod does internally. I did not research anything to support my claims and this work is not fully completed, I simply want to open the discussion whether it would be better. There was already some discussion on changing the output for discriminated unions in https://github.com/StefanTerdell/zod-to-json-schema/issues/60, but it was simply about moving to oneOf instead of anyOf.

There are a few relevant TODOs. I did not want to fully polish the code before discussing whether the maintainers even want this change. I definitely needed it for a project though, so I think others should also benefit from those changes.

StefanTerdell commented 3 months ago

Thanks for the PR. I'm sorry but I really don't see the improvement.

In the case of oneOfs the issue is that you can easily and unintentionally create overlapping members of primitive unions (which share a parser with discriminated unions) rendering them unusable.

For instance, z.union([z.string().email(), z.string()]) would produce a schema that was never valid for an email, since it would also be a valid string.

When it comes to objects I just don't see the point. If you have a discriminator it's always present, always required, and should produce a schema that any type generator worth its salt would have no issue producing a discrimination from.

For instance, running this

{
  "anyOf": [
    {
      "type": "object",
      "required": ["type", "foo"],
      "properties": {
        "type": {
          "type": "string",
          "const": "A"
        },
        "foo": true
      },
      "additionalProperties": false
    },
    {
      "type": "object",
      "required": ["type", "bar"],
      "properties": {
        "type": {
          "type": "string",
          "const": "B"
        },
        "bar": true
      },
      "additionalProperties": false
    }
  ]
}

...through this:

https://github.com/bcherny/json-schema-to-typescript

Produces this:

export type MySchema =
  | {
      type: "A"
      foo: true
    }
  | {
      type: "B"
      bar: true
    }

Switching on type works just fine, and narrows the options to display autocomplete for foo/bar.

This is the output from the schema in a test in this PR:

export type MySchema = {
  [k: string]: unknown
} & {
  type: "A" | "B"
  [k: string]: unknown
}

Please @ me if you want to continue the discussion since I don't check closed PRs/issues and won't receive a notification if you don't :)