openapi-ts / openapi-typescript

Generate TypeScript types from OpenAPI 3 specs
https://openapi-ts.dev
MIT License
5.62k stars 453 forks source link

Discriminator property not set for 2-level inheritance types #1158

Closed melvinnijholt closed 1 month ago

melvinnijholt commented 1 year ago

Description

For 1-level inheritance types the discriminator property is to a static value that identifies the type. For 2-level inheritance types this is not the case which makes it not possible to detect the type that is returned by the api endpoint.

Take for example Pet -> Dog -> Poodle. When a discriminator is specified for Pet this will be set on Dog but it will not be set on Poodle.

I don't know if this is excepted behavior, if it is than I will change this to a feature request.

Name Version
openapi-typescript 6.2.4
Node.js 16.19.0
OS + version Windows 11

Reproduction

I have a swagger.json that contains the following definition for Pet that defines a discriminator named _petType.

"Pet": {
  "required": [
    "_petType"
  ],
  "type": "object",
  "properties": {
    "_petType": {
      "type": "string"
    },
    "nickName": {
      "type": "string",
      "nullable": true
    }
  },
  "additionalProperties": false,
  "discriminator": {
    "propertyName": "_petType"
  }
}

It also contains a definition for Dog that defines the allOf property to Pet

"Dog": {
  "type": "object",
  "allOf": [
    {
      "$ref": "#/components/schemas/Pet"
    }
  ],
  "properties": {
    "bark": {
      "type": "boolean"
    }
  },
  "additionalProperties": false
}

and a definition for Poodle that defines the allOf property to Dog

"Poodle": {
  "type": "object",
  "allOf": [
    {
      "$ref": "#/components/schemas/Dog"
    }
  ],
  "properties": {
    "showDog": {
      "type": "boolean"
    }
  },
  "additionalProperties": false
}

The generated swagger.ts contains Dog specifying a value _petType: "Dog" for the discriminator. The discriminator value for Poodle is missing.

export interface components {
  schemas: {
    Pet: {
      _petType: string;
      nickName?: string | null;
    };
    Dog: {
      _petType: "Dog";
      bark?: boolean;
    } & Omit<components["schemas"]["Pet"], "_petType">;
    Poodle: {
      showDog?: boolean;
    } & components["schemas"]["Dog"];
  };
  responses: never;
  parameters: never;
  requestBodies: never;
  headers: never;
  pathItems: never;
}

I would the generated swagger to contain a Poodle specifying a value _petType: "Poodle" for the discriminator like swagger-expected.ts that

export interface components {
  schemas: {
    Pet: {
      _petType: string;
      nickName?: string | null;
    };
    Dog: {
      _petType: "Dog";
      bark?: boolean;
    } & Omit<components["schemas"]["Pet"], "_petType">;
    Poodle: {
      _petType: "Poodle";
      showDog?: boolean;
    } & Omit<components["schemas"]["Dog"], "_petType">;
  };
  responses: never;
  parameters: never;
  requestBodies: never;
  headers: never;
  pathItems: never;
}

Checklist

mnijholt commented 1 year ago

If someone could give some insights if this is working as expected our not, I willing to put effort in this to fix it. But I would like to know in front if I'm not wasting time because it works like this by design.

drwpow commented 1 year ago

@melvinnijholt correct me if I’m wrong but I don’t see "_petType": "Poodle" in your original schema. So how would openapi-typescript know to generate that?

The implementation followed comes from the 3.1 spec. I believe you may be missing some fields to make it work properly.

But if there’s an official example of valid syntax that looks different from this, if you provide a link I’d be happy to add alternate behavior.

mnijholt commented 1 year ago

@drwpow You can find the complete example in this repo:

https://github.com/melvinnijholt/openapi-typescript-discriminator-issue/

Your correct that in my schema Poodle doesn't contain a _petType. But dog also doesn't. Dog is omitting it from Pet and then adds the new value of "Dog". I would expect Poodle to do the same. Omit the _petType from Dog and it with the value of "Poodle".

But I'm not sure if this multi-level inherentance is supported by the specifcations.

drwpow commented 1 year ago

Please see an example:


MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
  discriminator:
    propertyName: petType
    mapping:
      dog: '#/components/schemas/Dog'
      monster: 'https://gigantic-server.com/schemas/Monster/schema.json'

~In your schema, you are lacking discriminator.mapping.~ (Edit: I was originally mistaken in this; discriminator.mapping is completely optional and is not required) Further, Dog has "_petType": "Dog". But Poodle does not have any "_petType": "Poodle" declared anywhere. These aren’t inferred; they have to be explicated. They don’t have to match the schema object name and it’s up to you to manually type them all.

drwpow commented 1 year ago

Wanted to provide an apology and a correction—yes your original issue is how the spec outlines discriminators to work and infers they do. They are more “magical” than I originally understood it to be!

The expectation you posted seems correct to me, and is how it should work as outlined in the spec.

However, currently, this library does require type: "string"; enum: ["someValue"] set on each type to work properly. So as a temporary workaround, you could set {_petType: {type: 'string', enum: ['Dog']}} on Dog and similar for Poodle. Until this bug is fixed and the generated types from this library match expected behavior.

github-actions[bot] commented 1 month ago

This issue is stale because it has been open for 90 days with no activity. If there is no activity in the next 7 days, the issue will be closed.

github-actions[bot] commented 1 month ago

This issue was closed because it has been inactive for 7 days since being marked as stale. Please open a new issue if you believe you are encountering a related problem.