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

Types derived for oneOf construct result in TS2502 error #1665

Open
1 of 2 tasks
swachter opened this issue May 17, 2024 · 7 comments
Open
1 of 2 tasks

Types derived for oneOf construct result in TS2502 error #1665

swachter opened this issue May 17, 2024 · 7 comments
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library

Comments

@swachter
Copy link
Contributor

swachter commented May 17, 2024

Description

Processing the OpenAPI spec of the Mongo Atlas Admin API I found that the oneOf construct yields to types that have a cyclic reference that result in TypeScript error TS2502. E.g.:

src/openapi/mongo-atlas-admin-v2.mts:10468:5 - error TS2502: 'IngestionSource' is referenced directly or indirectly in its own type annotation.

10468     IngestionSource: ({

This is the OpenAPI spec of IngestionSource:

      "IngestionSource": {
        "type": "object",
        "description": "Ingestion Source of a Data Lake Pipeline.",
        "discriminator": {
          "mapping": {
            "ON_DEMAND_CPS": "#/components/schemas/OnDemandCpsSnapshotSource",
            "PERIODIC_CPS": "#/components/schemas/PeriodicCpsSnapshotSource"
          },
          "propertyName": "type"
        },
        "oneOf": [
          {
            "$ref": "#/components/schemas/OnDemandCpsSnapshotSource"
          },
          {
            "$ref": "#/components/schemas/PeriodicCpsSnapshotSource"
          }
        ],
        "properties": {
          "type": {
            "type": "string",
            "description": "Type of ingestion source of this Data Lake Pipeline.",
            "enum": ["PERIODIC_CPS", "ON_DEMAND_CPS"]
          }
        },
        "title": "Ingestion Source"
      },

The reason is that in the member types the discriminator property is explicitly omitted, thereby creating the cyclic refernce:

    IngestionSource: ({
      /**
       * @description Type of ingestion source of this Data Lake Pipeline.
       * @enum {string}
       */
      type?: "PERIODIC_CPS" | "ON_DEMAND_CPS";
    }) & (components["schemas"]["OnDemandCpsSnapshotSource"] | components["schemas"]["PeriodicCpsSnapshotSource"]);

...

    OnDemandCpsSnapshotSource: {
      type: "ON_DEMAND_CPS";
    } & Omit<components["schemas"]["IngestionSource"], "type"> & {
      /** @description Human-readable name that identifies the cluster. */
      clusterName?: string;
      /** @description Human-readable name that identifies the collection. */
      collectionName?: string;
      /** @description Human-readable name that identifies the database. */
      databaseName?: string;
      /**
       * @description Unique 24-hexadecimal character string that identifies the project.
       * @example 32b6e34b3d91647abb20e7b8
       */
      groupId?: string;
    };

If the Omit<components["schemas"]["IngestionSource"], "type"> part is omitted ( ;-) ), then the cyclic reference is cut. TypeScript is still able to handle the resulting discriminated union type.

Note: If such invalid types are imported from d.ts files then tsc just falls back to any, loosing all type safetyness.

Name Version
openapi-typescript 6.7.5

Reproduction

Run openapi-typescript on the mentioned OpenAPI spec and compile it by tsc.

@swachter swachter added bug Something isn't working openapi-ts Relevant to the openapi-typescript library labels May 17, 2024
@mzronek
Copy link
Contributor

mzronek commented May 19, 2024

Please try it with version 7 and let us know if the problem still arises. The oneOf discriminator support was completely rewritten and does not use Omit anymore.

@swachter
Copy link
Contributor Author

I just checked with 7.0.0-rc.0. There are still cyclic references. The OnDemandCpsSnapshotSource type still includes the Omit<components["schemas"]["IngestionSource"], "type"> part.

@EggOxygen
Copy link

EggOxygen commented May 27, 2024

I encountered the same issue here. I tested it locally with 7.0.0-rc.0 using the following specifications.

{
    "openapi": "3.0.1",
    "components": {
        "schemas": {
            "Animal": {
                "required": [
                    "type"
                ],
                "type": "object",
                "properties": {
                    "type": {
                        "type": "string"
                    }
                },
                "description": "Animal",
                "discriminator": {
                    "propertyName": "type",
                    "mapping": {
                        "DOG": "#/components/schemas/Dog",
                        "CAT": "#/components/schemas/Cat"
                    }
                },
                "oneOf": [
                    {
                        "$ref": "#/components/schemas/Dog"
                    },
                    {
                        "$ref": "#/components/schemas/Cat"
                    }
                ]
            },
            "Cat": {
                "type": "object",
                "allOf": [
                    {
                        "$ref": "#/components/schemas/Animal"
                    },
                    {
                        "type": "object",
                        "properties": {
                            "age": {
                                "type": "integer",
                                "format": "int32"
                            }
                        }
                    }
                ]
            },
            "Dog": {
                "type": "object",
                "allOf": [
                    {
                        "$ref": "#/components/schemas/Animal"
                    },
                    {
                        "type": "object",
                        "properties": {
                            "name": {
                                "type": "string"
                            }
                        }
                    }
                ]
            }
        }
    }
}

I believe my issue might be related to this, but I'm uncertain. It would be helpful to confirm if this is indeed the case.

@mzronek
Copy link
Contributor

mzronek commented May 29, 2024

You need to remove the oneOf from Animal. It is inferred by the inherited discriminator.

Please check the unit tests for supported discriminator usages:
https://github.com/drwpow/openapi-typescript/blob/main/packages/openapi-typescript/test/discriminators.test.ts

@EggOxygen
Copy link

You need to remove the oneOf from Animal. It is inferred by the inherited discriminator.

Thank you for your reply. I have already looked for the test when I first encountered this issue. It initially started at #1574.


I know we can solve this issue by remove the oneOf. But I checked the sample that @swachter provided, which is also similar to my case. Isn't this also a valid way to do this? Even if it's inferred by the inherited discriminator.

      "IngestionSource": {
        "type": "object",
        "description": "Ingestion Source of a Data Lake Pipeline.",
        "discriminator": {
          "mapping": {
            "ON_DEMAND_CPS": "#/components/schemas/OnDemandCpsSnapshotSource",
            "PERIODIC_CPS": "#/components/schemas/PeriodicCpsSnapshotSource"
          },
          "propertyName": "type"
        },
        "oneOf": [
          {
            "$ref": "#/components/schemas/OnDemandCpsSnapshotSource"
          },
          {
            "$ref": "#/components/schemas/PeriodicCpsSnapshotSource"
          }
        ],
        "properties": {
          "type": {
            "type": "string",
            "description": "Type of ingestion source of this Data Lake Pipeline.",
            "enum": [
              "PERIODIC_CPS",
              "ON_DEMAND_CPS"
            ]
          }
        },
        "title": "Ingestion Source"
      },
      "OnDemandCpsSnapshotSource": {
        "type": "object",
        "allOf": [
          {
            "$ref": "#/components/schemas/IngestionSource"
          },
          {
            "type": "object",
            "properties": {
              "clusterName": {
                "type": "string",
                "description": "Human-readable name that identifies the cluster."
              },
              "collectionName": {
                "type": "string",
                "description": "Human-readable name that identifies the collection."
              },
              "databaseName": {
                "type": "string",
                "description": "Human-readable name that identifies the database."
              },
              "groupId": {
                "type": "string",
                "description": "Unique 24-hexadecimal character string that identifies the project.",
                "example": "32b6e34b3d91647abb20e7b8",
                "maxLength": 24,
                "minLength": 24,
                "pattern": "^([a-f0-9]{24})$",
                "readOnly": true
              }
            }
          }
        ],
        "description": "On-Demand Cloud Provider Snapshots as Source for a Data Lake Pipeline.",
        "title": "On-Demand Cloud Provider Snapshot Source"
      },

@mzronek
Copy link
Contributor

mzronek commented Jun 2, 2024

The JSON Schema Project and the OpenAPI Specification do not restrict the usage of circular references. They pass the problem on to the tooling to be solved (a linter or a validator for example). Details here.

If I throw the Animal schema against an OpenAPI validator I am getting a recursion limit error on the first oneOf reference, that is resolved. So this case is an invalid schema. In very rare cases openapi-typescript handles invalid schemas if the case is for example very common. In my opinion this is not true here.

@swachter
Copy link
Contributor Author

swachter commented Jun 2, 2024

Many thanks for the clarification, @mzronek .

I wonder if the code generator could do better in certain situations. For example manually removing the Omit part from the created code did work for me. Maybe such situations could be detected.

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