Required flag missing on property when it is referenced with 'discriminator'

[simon-csaba]

openapi-typescript version

7.6.1

Node.js version

20.17.0

OS + version

WSL Ubuntu 22.04.1 LTS

Description

We're trying to generate a type for an update request (UpdatePet) where the attributes need to be determined by a discriminator (sound), with the two possible types being CatProps and DogProps, these have one optional property each: kittens and puppies respectively. We're also trying to generate two additional types Cat and Dog , which reference CatProps and DogProps, with kittens and puppies being required.

When the discriminator is added to the UpdatePet schema the Cat and Dog types incorrectly generate with kittens and puppies becoming optional. If we remove the discriminator, the Cat and Dog schemas generate correctly.

Reproduction

openapi: 3.0.0
info:
  title: Bug test
  version: 1.0.0
  description: Api specification for bug report
components:
  schemas:
    CatProps:
      type: object
      properties:
        kittens:
          type: number
          format: integer
    Cat:
      type: object
      required: [kittens]
      allOf:
        - $ref: "#/components/schemas/CatProps"

    DogProps:
      type: object
      properties:
        puppies:
          type: number
          format: integer

    Dog:
      type: object
      required: [puppies]
      allOf:
        - $ref: "#/components/schemas/DogProps"

    UpdatePet:
      oneOf:
        - $ref: "#/components/schemas/DogProps"
        - $ref: "#/components/schemas/CatProps"
      discriminator:
        propertyName: sound
        mapping:
          bark: "#/components/schemas/DogProps"
          meow: "#/components/schemas/CatProps"

Generate a ts schema from this yml, Cat type will be: Cat: components["schemas"]["CatProps"]
Then delete the whole discriminator, Cat type will be: Cat: WithRequired<components["schemas"]["CatProps"], "kittens">

Expected result

Cat type should be the following: Cat: WithRequired<components["schemas"]["CatProps"], "kittens">

The type of UpdatePet should have no effect on the seemingly unrelated Cat and Dog types.

Required

• My OpenAPI schema is valid and passes the Redocly validator (npx @redocly/cli@latest lint)

Extra

• I’m willing to open a PR (see CONTRIBUTING.md)

[duncanbeevers]

There are a few things going on here, so I'll shoot some feedback and you can let me know if I'm getting warm.

First, sound is never defined as a property anywhere in this schema. This means its a little hard to say what the final type should be. Without an explicit additionalProperties schema, it seems like the type should be sound: never.

Next, the schemas for Cat and Dog seem mis-formed. When you're composing schemas, think of allOf as a wrapper around all the schemas you're composing, and not a way to mix ingredients into the middle.

Finally, I couldn't get different behavior from this schema when I left-in, and removed the discriminator section, so there may be a red herring here.

Here is how I think this schema is supposed to be constructed.

• CatProps defines an object schema with an optional kittens property
• DogProps defines an object schema with an optional puppies property
• PetProps defines an object schema with a required sound property
• Cat defines a composition of 3 schemas; CatProps, PetProps, and an object schema with a required kittens property
• Dog defines a composition of 3 schemas; DogProps, PetProps and an object schema with a required puppies property

openapi: 3.0.0
info:
  title: Bug test
  version: 1.0.0
  description: Api specification for bug report
components:
  schemas:
    CatProps:
      type: object
      properties:
        kittens:
          $ref: '#/components/schemas/OffspringCount'
    Cat:
      allOf:
        - $ref: '#/components/schemas/CatProps'
        - $ref: '#/components/schemas/PetProps'
        - type: object
          properties:
            kittens:
              $ref: '#/components/schemas/OffspringCount'
          required: [kittens]
    DogProps:
      type: object
      properties:
        puppies:
          $ref: '#/components/schemas/OffspringCount'
    Dog:
      allOf:
        - $ref: '#/components/schemas/DogProps'
        - $ref: '#/components/schemas/PetProps'
        - type: object
          properties:
            puppies:
              $ref: '#/components/schemas/OffspringCount'
          required: [puppies]
    OffspringCount:
      type: number
      format: integer
    PetProps:
      type: object
      properties:
        sound:
          type: string
      required: [sound]
    UpdatePet:
      oneOf:
        - $ref: '#/components/schemas/DogProps'
        - $ref: '#/components/schemas/CatProps'
      discriminator:
        propertyName: sound
        mapping:
          bark: '#/components/schemas/DogProps'
          meow: '#/components/schemas/CatProps'

I threw the types generated from this updated schema into the TS Playground to verify the final type of Cat#kittens, and saw the expected required number property.

TS Playground

Please let me know if I've missed something critical here and I'll be happy to re-assess.

[efteel]

Thanks for your detailed answer!

1. So you are saying that the correct way to make an optional property required in a schema using composition is CatWithRequiredKittensA, and CatWithRequiredKittensB is invalid and should not work.

    CatProps:
      type: object
      properties:
        kittens:
          type: number
          
    CatWithRequiredKittensA:
      allOf:
        - $ref: "#/components/schemas/CatProps"
        - type: object
          properties:
            kittens:
              type: number
          required: [kittens]

    CatWithRequiredKittensB:
      required: [kittens]
      allOf:
        - $ref: "#/components/schemas/CatProps"

    CatExtended:
      allOf:
        - $ref: "#/components/schemas/CatProps"
      properties:
        isPurring:
          type: boolean
      required: [kittens, isPurring]

But in practice both seems to be valid and works, even CatExtended.
In swagger it shows the kittens property required as expected:

The generated TS code also works as expected, although the types are different: CatsWithRequiredKittensB uses the WithRequired utility type to make the kittens property required, so it seems to be intentionally supported. 🤷🏻‍♂️

export interface components {
    schemas: {
        CatProps: {
            kittens?: number;
        };
        CatWithRequiredKittensA: components["schemas"]["CatProps"] & {
            kittens: number;
        };
        CatWithRequiredKittensB: WithRequired<components["schemas"]["CatProps"], "kittens">;
        CatExtended: {
            isPurring: boolean;
        } & WithRequired<components["schemas"]["CatProps"], "kittens">;
    };
  // ...
}

The solution you are suggesting here is logical and certainly works, but when we have tens of properties that we want to make required, it requires a lot of repetition and a lot more work than simply adding the required keyword and listing the property names.

2. It doesn't seem necessary to explicitly define the type of the discriminator property, openapi-ts adds the property to the referenced schemas automatically and even adds the "discriminator enum property added by openapi-typescript" comment above it.

    CatProps:
      type: object
      properties:
        kittens:
          type: number

    UpdatePet:
      oneOf:
        - $ref: "#/components/schemas/CatProps"
      discriminator:
        propertyName: sound
        mapping:
          meow: "#/components/schemas/CatProps"

export interface components {
    schemas: {
        CatProps: {
            kittens?: number;
            /**
             * @description discriminator enum property added by openapi-typescript
             * @enum {string}
             */
            sound: "meow";
        };
        UpdatePet: components["schemas"]["CatProps"];
    };
    // ...
}

Actually it seemingly simply overrides the existing property with same name:

    CatProps:
      type: object
      properties:
        kittens:
          type: number
        sound:
          type: boolean  # note that it's a boolean type and optional

    UpdatePet:
      oneOf:
        - $ref: "#/components/schemas/CatProps"
      discriminator:
        propertyName: sound
        mapping:
          meow: "#/components/schemas/CatProps"

This generates the same output as above: sound: "meow".
Anyway, explicitly adding the discriminator property to the schemas used in the discriminated union doesn't make any difference regarding the following problem.

3. Maybe it's wrong from the start, but let's assume that marking properties as required is correct with the simpler approach (as in CatWithRequiredKittensB above) and the generated code with the WithRequired utility type is also correct:

    CatProps:
      type: object
      properties:
        kittens:
          type: number

    Cat:
      required: [kittens]
      allOf:
        - $ref: "#/components/schemas/CatProps"

export interface components {
    schemas: {
        CatProps: {
            kittens?: number;
        };
        Cat: WithRequired<components["schemas"]["CatProps"], "kittens">;
    };
    // ...
}

Then when I add the following schema to the yaml:

    UpdatePet:
      oneOf:
        - $ref: "#/components/schemas/CatProps"
      discriminator:
        propertyName: sound
        mapping:
          meow: "#/components/schemas/CatProps"

in the generated code the WithRequired disappears from the declaration of Cat!

export interface components {
    schemas: {
        CatProps: {
            kittens?: number;
            /**
             * @description discriminator enum property added by openapi-typescript
             * @enum {string}
             */
            sound: "meow";
        };
        Cat: components["schemas"]["CatProps"];
        UpdatePet: components["schemas"]["CatProps"];
    };
    // ...
}

I would expect that even if opeanapi-ts modifies the CatProps declaration by adding the extra discriminator property, the type of Cat should stay the same: WithRequired<components["schemas"]["CatProps"], "kittens">.

Your suggested solution doesn't have this problem as it doesn't use the WithRequired utility in the generated code. I'm just not convinced yet that the other approach is invalid as it seems to be supported, and if it's valid then the problem with the disappearing WithRequired seems to be a bug.

[duncanbeevers]

Yeah, there's definitely something different going on with that discriminator. I'll discuss with the other maintainers and see if we can arrive at a good understanding and solution.

Thanks for the detailed report and repro case; it can makes investigating these issues so much easier.

[duncanbeevers]

Quick update…

At today's TSC meeting there was some discussion around discriminator; specifically a bit of work has gone into allowing the property to which the discriminator refers to be optional.

Allow discriminator to be optional