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

discriminator mapping ignored when using allOf with oneOf #1690

Open
1 task
CalebJamesStevens opened this issue Jun 7, 2024 · 5 comments
Open
1 task

discriminator mapping ignored when using allOf with oneOf #1690

CalebJamesStevens opened this issue Jun 7, 2024 · 5 comments
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library

Comments

@CalebJamesStevens
Copy link

Description

Here's a very basic example

openapi: 3.0.0

components:
  schemas:
    A:
      oneOf:
        - $ref: '#/components/schemas/C'
        - $ref: '#/components/schemas/D'
      discriminator:
        propertyName: pet_type
        mapping:
          c: '#/components/schemas/C'
          d: '#/components/schemas/D'
    B:
      allOf:
        - $ref: '#/components/schemas/A'
    C:
      type: object
      properties:
        pet_type:
          type: string
          enum: [c]
    D:
      type: object
      properties:
        pet_type:
          type: string
          enum: [d]
/**
 * 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: {
        A: components["schemas"]["C"] | components["schemas"]["D"];
        B: {
            pet_type: "B";
        } & Omit<components["schemas"]["A"], "pet_type">;
        C: {
            /**
             * @description discriminator enum property added by openapi-typescript
             * @enum {string}
             */
            pet_type: "c";
        };
        D: {
            /**
             * @description discriminator enum property added by openapi-typescript
             * @enum {string}
             */
            pet_type: "d";
        };
    };
    responses: never;
    parameters: never;
    requestBodies: never;
    headers: never;
    pathItems: never;
}
export type $defs = Record<string, never>;
export type operations = Record<string, never>;

As you can see we should be able to use this syntax to easily cut down the fluff and add more properties to B while maintaining the discriminators mapping. Instead the pet_type is omitted and a new pet_type is created overwriting C and D casing A to be completely invalid.

Name Version
openapi-typescript openapi-typescript@next
Node.js 18.16.0
OS + version macOS 13

Reproduction

How can this be reproduced / when did the error occur?

npx openapi-typescript@next test.yaml -o ./testnext.d.ts

Expected result
something like

/**
 * 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: {
        A: components["schemas"]["C"] | components["schemas"]["D"];
        B: components["schemas"]["A"];
        C: {
            /**
             * @description discriminator enum property added by openapi-typescript
             * @enum {string}
             */
            pet_type: "c";
        };
        D: {
            /**
             * @description discriminator enum property added by openapi-typescript
             * @enum {string}
             */
            pet_type: "d";
        };
    };
    responses: never;
    parameters: never;
    requestBodies: never;
    headers: never;
    pathItems: never;
}
export type $defs = Record<string, never>;
export type operations = Record<string, never>;

(in case it’s not obvious)

Checklist

@CalebJamesStevens CalebJamesStevens added bug Something isn't working openapi-ts Relevant to the openapi-typescript library labels Jun 7, 2024
@drwpow-figma
Copy link

This is actually a common misconception about how discriminator works—the mapping is optional and it’s actually designed to automatically infer to the propertyName without the mapping. See a more detailed explanation. This was designed as a timesaver to prevent people from having to manually map too many types; the mapping is just there in case the inference is wrong. So it’s actually working as-intended to infer B has pet_type: "B".

@CalebJamesStevens
Copy link
Author

This is actually a common misconception about how discriminator works—the mapping is optional and it’s actually designed to automatically infer to the propertyName without the mapping. See a more detailed explanation. This was designed as a timesaver to prevent people from having to manually map too many types; the mapping is just there in case the inference is wrong. So it’s actually working as-intended to infer B has pet_type: "B".

I'm not sure why this would a behavior that we want. The property that maps the reference in this example is pet_type which exists on C & D. Having this behavior essentially makes it impossible to use the discriminator properly because it will always be overwritten to have pet_type as the value B.

@drwpow
Copy link
Contributor

drwpow commented Jun 10, 2024

Oh I’m fully in agreement with you—I also would prefer the mapping be manual. But this is how the JSONSchema (and by extension, OpenAPI 3) specification is designed to work, at least going through the examples. They explicitly call out this behavior. If you reference any object where the discriminator.propertyName is used, it will infer its value. This is present in the specification itself.

@CalebJamesStevens
Copy link
Author

Oh I’m fully in agreement with you—I also would prefer the mapping be manual. But this is how the JSONSchema (and by extension, OpenAPI 3) specification is designed to work, at least going through the examples. They explicitly call out this behavior. If you reference any object where the discriminator.propertyName is used, it will infer its value. This is present in the specification itself.

I'm not sure that this isn't a bug still. With our schema uploaded to swagger the schemas are being shown as expected, its only when using this typescript generation tool that we are seeing the bug. And it's only when using it with allOf which is necessary to do something like so:

openapi: 3.0.0

components:
  schemas:
    A:
      oneOf:
        - $ref: '#/components/schemas/C'
        - $ref: '#/components/schemas/D'
      discriminator:
        propertyName: pet_type
        mapping:
          c: '#/components/schemas/C'
          d: '#/components/schemas/D'
    B:
      allOf:
        - $ref: '#/components/schemas/A'
      properties: 
        prop:
          type: string
    C:
      type: object
      properties:
        pet_type:
          type: string
          enum: [c]
    D:
      type: object
      properties:
        pet_type:
          type: string
          enum: [d]

@CalebJamesStevens
Copy link
Author

@drwpow-figma
This is a work around that i came up with that works for the TS generator, need to validate if it works for swagger

openapi: 3.0.3
info:
  title: Sample API
  version: 1.0.0
paths: {}
components:
  schemas:
    B:
      type: object
      properties:
        prop:
          type: string
        type:
          type: string
          enum: [b]
    C:
      type: object
      properties:
        otherProp:
          type: string
        type:
          type: string
          enum: [c]
    TypeADiscriminator:
      oneOf:
      - $ref: '#/components/schemas/B'
      - $ref: '#/components/schemas/C'
      discriminator:
        propertyName: type
        mapping:
          b: '#/components/schemas/B'
          c: '#/components/schemas/C'
    TypeA:
      $ref: '#/components/schemas/TypeADiscriminator'
    ExtendedTypeA:
      allOf:
        - $ref: '#/components/schemas/TypeA'
        - type: object
          properties:
            name:
              type: string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library
Projects
None yet
Development

No branches or pull requests

3 participants