cyclosproject / ng-openapi-gen

An OpenAPI 3.0 codegen for Angular
MIT License
397 stars 132 forks source link

support type narrowing using the discriminator when you have derived types #227

Closed stocksr closed 1 year ago

stocksr commented 2 years ago

Hard to explain exactly what I would like so an example: current output

export interface FooBaseType {
  '$type': string;
  baseProperty?: string;
}
export type DriviedClassName2 = FooBaseType & {
'additionalProperty'?: number;
};
export type DriviedClassName1 = FooBaseType & {
};

desired output

export interface FooBaseType {
  baseProperty?: string;
}
export type DriviedClassName1 = FooBaseType & {
'$type': 'DriviedClassName1, Assembly';
};
export type DriviedClassName2 = FooBaseType & {
'additionalProperty'?: number;
'$type': 'DriviedClassName2, Assembly';
};

source api spec

{
  "openapi": "3.0.1",
  "info": {
    "title": "Blah",
    "version": "1"
  },
  "paths": {},
  "components": {
    "schemas": {
      "FooBaseType": {
        "required": [
          "$type"
        ],
        "type": "object",
        "properties": {
          "$type": {
            "type": "string"
          },
          "baseProperty" : {
            "type": "string"
          }
        },
        "additionalProperties": false,
        "description": "",
        "discriminator": {
          "propertyName": "$type",
          "mapping": {
            "DriviedClassName1, Assembly": "#/components/schemas/DriviedClassName1",
            "DriviedClassName2, Assembly": "#/components/schemas/DriviedClassName2"
          }
        }
      },
      "DriviedClassName1": {
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/FooBaseType"
          }
        ],
        "additionalProperties": false,
        "description": "No Additional Properties"
      },
      "DriviedClassName2": {
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/FooBaseType"
          }
        ],
        "properties": {
          "additionalProperty": {
            "type": "integer",
            "format": "int32"
          }
        },
        "additionalProperties": false,
        "description": "With Additional Property"
      }
    }
  }
}

Justification is to enable typescript magic type narrowing.

test(x: DriviedClassName1  | DriviedClassName2) {
        if (
            x.$type === 'DriviedClassName2, Assembly' \\ this line is a build error if I get the magic string wrong
        ) {
           // this next line is not a compile error bcasuse typescript knows that if  the above condition is true then x must be of type DriviedClassName2
            console.log(x.additionalProperty);
        }
    }

Because I really want this I have hacked together a proof of concept. (see this branch )

Questions:

  1. Does this change "fit" with the goals of this project? If not, no worries just close this issue and I will just use my fork.
  2. Do you want a PR for this? if so a. I have made no attempt to follow your coding styles / naming conventions, I will sort that out before I create the PR - but it might take me a while to get back to this. b. Does my approach to a solution work? if not please see the Attempt 1 comment in the same branch is that a better approach?
luisfpg commented 2 years ago

Hi. I'm struggling with the time to support the project, specially because as it is it works for us for years already. New features are always from PR's, and this feature is logical. If you'd like to submit a PR with the code conventions and test cases, it will be merged. Otherwise, just close this issue. I don't see a reason to make it optional, to be honest. But as I said, I don't have much time to invest in this project, so review / merge may take a while.

fdipuma commented 1 year ago

Bringing back this thread because I've the same issue. My current solution would keep the discriminator property on the base type, but also force the discriminator into the derived types.

@luisfpg Would you be okay with a PR with this behavior?

fdipuma commented 1 year ago

Hi @luisfpg , do you think you have time to check the PR? it seems to me everything should be ok (no breaking changes, tests are all green).

Thanks!

luisfpg commented 1 year ago

Took me some time, but this is released in 0.25.0. Thanks!