Nullable property containing discriminated union, generates incorrect type

[Swimburger]

Description

When I have a nullable property, but when not null, is a discriminated union, I get unexpected TS output.

Name	Version
openapi-typescript	6.7.1
Node.js	v20.5.1
OS + version	macOS 14.0

Reproduction

OpenAPI:

openapi: 3.1.0

components:
  schemas:
    Foo:
      type: object
      properties:
        bar:
          oneOf:
            - $ref: "#/components/schemas/Bar"
            - type: "null"

    Bar:
      oneOf:
        - type: object
          properties:
            status:
              type: string
              const: "ok"
            otherProp:
              type: boolean
        - type: object
          properties:
            status:
              type: string
              const: "not ok"
      discriminator:
        propertyName: status

Output TS:

/**
 * This file was auto-generated by openapi-typescript.
 * Do not make direct changes to the file.
 */

export type paths = Record<string, never>;

export type webhooks = Record<string, never>;

export interface components {
  schemas: {
    Foo: {
      bar?: {
        status: "Foo";
      } & (Omit<components["schemas"]["Bar"], "status"> | null);
    };
    Bar: {
      /** @constant */
      status?: "ok";
      otherProp?: boolean;
    } | {
      /** @constant */
      status?: "not ok";
    };
  };
  responses: never;
  parameters: never;
  requestBodies: never;
  headers: never;
  pathItems: never;
}

export type $defs = Record<string, never>;

export type external = Record<string, never>;

export type operations = Record<string, never>;

Expected result

The expected output for Foo is:

Foo: {
      bar?: components["schemas"]["Bar"] | null);
    };

Checklist

• [ ] My OpenAPI schema passes the Redocly validator (npx @redocly/cli@latest lint)
• [ ] I’m willing to open a PR (see CONTRIBUTING.md)

[drwpow]

This looks like the discriminator usage is correct, but the | null is in the wrong place. I think it should be:

export interface components {
  schemas: {
    Foo: {
      bar?: ({
        status: "Foo";
      } & Omit<components["schemas"]["Bar"], "status">) | null;
    };

Discriminators have a slightly-confusing behavior outlined in the specification where they infer the name of their parent schema object:

It is implied, that the property to which discriminator refers, contains the name of the target schema. In the example above, the objectType property should contain either simpleObject, or complexObject string. If the property values do not match the schema names, you can map the values to the names. To do this, use the discriminator/mapping keyword

source (emphasis added)

In other words, mapping is optional. So status: "Foo" is actually correct, as outlined by the spec (even if it’s not how you expected it to work). The Omit<…> helper is necessary so the static value can be overwritten.

But back to | null, we want that to be a union on the top level (probably). & null as an intersection probably isn’t desired behavior anywhere. Does all that sound correct?

[Swimburger]

That's great context. Thank you for educating me on this :) We want the union to be in the Bar schema.

The desired output for our use case is components["schemas"]["Bar"] | null. Is there a way in OpenAPI to specify it that way, without it inferring the parent name as the discriminator value?

[drwpow]

Is there a way in OpenAPI to specify it that way, without it inferring the parent name as the discriminator value?

Discriminators are just weird 😅. I believe even if you used allOf: ["#/components/schemas/Bar"] (docs), allOf still counts as that automatically-inferred discriminator; even when you are truly trying to “reuse” the name/value it will still get remapped.

As far as I know if, the only way to rewrite the values of a discriminator type is with explicit mapping. If you wanted to reuse the same value for multiple types, then make that explicit in the mapping (i.e. duplicate the values manually).

I also think that mapping benefits from having actual $refs, so keeping your oneOf structure, but converting them from inline schema objects to explicit named $refs would let you map specific objects to specific values more easily, and also probably help openapi-typescript (and other tools) make better decisions on what the schema is suggesting should happen.

[Swimburger]

We actually do use $ref's and explicit mappings, but I tried to provide a minimal repro :)

If we move the discriminated union including "null" directly into the property, the output is as what we desire, except that we want the union to be in a separate type for code generation.

openapi: 3.1.0

components:
  schemas:
    Foo:
      type: object
      properties:
        bar:
          oneOf:
            - $ref: "#/components/schemas/BarOk"
            - $ref: "#/components/schemas/BarNotOk"
            - type: "null"
          discriminator:
            propertyName: status
            mapping:
              ok: "#/components/schemas/BarOk"
              not_ok: "#/components/schemas/BarNotOk"
      required: [bar]
    Bar:
      oneOf:
        - $ref: "#/components/schemas/BarOk"
        - $ref: "#/components/schemas/BarNotOk"
      discriminator:
        propertyName: status
        mapping:
          ok: "#/components/schemas/BarOk"
          not_ok: "#/components/schemas/BarNotOk"
    BarStatus:
      type: string
      enum: ["ok", "not_ok"]
    BaseBar:
      type: object
      properties:
        status:
          $ref: "#/components/schemas/BarStatus"
      required: [status]
    BarOk:
      allOf:
        - $ref: "#/components/schemas/BaseBar"
        - type: object
          properties:
            status:
              type: string
              const: "ok"
            otherProp:
              type: boolean
          required: [status, otherProp]
    BarNotOk:
      allOf:
        - $ref: "#/components/schemas/BaseBar"
        - type: object
          properties:
            status:
              type: string
              const: "not_ok"
          required: [status]

TS:

    Foo: {
      bar: components["schemas"]["BarOk"] | components["schemas"]["BarNotOk"] | null;
    };

So, the above is more what we desire, but we want components["schemas"]["Bar"] | null. I'm guessing it's not possible in OpenAPI.

[drwpow]

Ah I see. Yeah this may just be one of those scenarios where the specification itself isn’t clearly explicit on how to handle it. Making a discriminator property nullable in general probably should be approached cautiously (or at least on TypeScript’s side, trying to discriminate over a nullable property in some scenarios can result in never types). But from the OpenAPI side, I’m not aware of a clearly correct way to describe this.

But as a sidenote, openapi-typescript does often have to make arbitrary decisions sometimes in the translation from OpenAPI → TS. And once a decision is made, we generally don’t reverse it unless we find it conflicts with an explicit part of the spec we missed (and this is rare). If something generates a certain way right now, you can more-or-less depend on it continuing to generate that way in future versions of openapi-typescript (and I’ll also add a test for this to catch unintentional regressions for this).

[Swimburger]

Got it, we're already using the Node API with transform to change the result for this. Thank you for the explanation!