microsoft / dts-gen

dts-gen creates starter TypeScript definition files for any module or library.
MIT License
2.43k stars 102 forks source link

allOf doesn't deep merge objects or use referenced types #161

Open henhal opened 4 years ago

henhal commented 4 years ago

I found several strange things related to merging of properties with allOf. Consider the following document:

openapi: 3.0.2
info:
  title: Test
  version: "2.0"

components:
  schemas:
    Foo:
      type: object
      properties:
        name: {type: string}
        data:
          type: object
          properties:
            foo: {type: string}

    Bar:
      allOf:
        - $ref: '#/components/schemas/Foo'
        - properties:
            name: {enum: [BAR]}
            data:
              type: object
              properties:
                bar: {type: string}                         
paths: {}

This generates the following:

$ cat schema.yml | dtsgen

declare namespace Components {
    namespace Schemas {
        export interface Bar {
            name?: "BAR";
            data?: {
                bar?: string;
            };
        }
        export interface Foo {
            name?: string;
            data?: {
                foo?: string;
            };
        }
    }
}

As you can see, Bar uses allOf to extend Foo by narrowing the value for name and by adding an additional sub-property of data.

Problem 1 The presence of data in Bar seems to override the data property of Foo, so that only bar is present, not foo and bar.

All other JSON schema parsers I've used successfully merges properties when using allOf, for example ReDoc, swagger-to-ts, and json-schema-to-typescript, so that objects mixed using allOf are merged.

Expected result:

declare namespace Components {
    namespace Schemas {
        export interface Bar {
            name?: "BAR";
            data?: {
                foo?: string;
                bar?: string;
            };
        }
        export interface Foo {
            name?: string;
            data?: {
                foo?: string;
            };
        }
    }
}

Furthermore, if I extract the type of data as a schema of its own, like this:

...
    Data:
      type: object
      properties:
        foo: {type: string}
    Foo:
      type: object
      properties:
        name: {type: string}
        data: {$ref: '#/components/schemas/Data'}

    Bar:
      allOf:
        - $ref: '#/components/schemas/Foo'
        - properties:
            name: {enum: [BAR]}
            data:
              type: object
              properties:
                bar: {type: string}
...

Then the Data type is referenced correctly for Foo:

declare namespace Components {
    namespace Schemas {
        export interface Bar {
            name?: "BAR";
            data?: {
                bar?: string;
            };
        }
        export interface Data {
            foo?: string;
        }
        export interface Foo {
            name?: string;
            data?: Data;
        }
    }
}

If data had been merged, I would have expected Bar.data to be typed as Data & {bar?: string;}, like this:

...
          export interface Bar {
            name?: "BAR";
            data?: Data & {
                bar?: string;
            };
        }
...

Problem 2. As a workaround to the first problem, I explicitly add an allOf also for the data property:

components:
  schemas:
    Data:
      type: object
      properties:
        foo: {type: string}
    Foo:
      type: object
      properties:
        name: {type: string}
        data: {$ref: '#/components/schemas/Data'}

    Bar:
      allOf:
        - $ref: '#/components/schemas/Foo'
        - properties:
            name: {enum: [BAR]}
            data:
              allOf:
                - $ref: '#/components/schemas/Data'
                - type: object
                  properties:
                    bar: {type: string}
...

Then Bar.data at least contains both foo and bar, but foo is inlined instead of using an union with Data.

Actual result:

declare namespace Components {
    namespace Schemas {
        export interface Bar {
            name?: "BAR";
            data?: {
                foo?: string;
                bar?: string;
            };
        }
        export interface Data {
            foo?: string;
        }
        export interface Foo {
            name?: string;
            data?: Data;
        }
    }
}

Expected result:

declare namespace Components {
    namespace Schemas {
        export interface Bar {
            name?: "BAR";
            data?: Data & {
                bar?: string;
            };
        }
        export interface Data {
            foo?: string;
        }
        export interface Foo {
            name?: string;
            data?: Data;
        }
    }
}

For large schemas re-used in multiple sub-schemas, this results in much more verbose types with less readability.

Question: Is the behaviour of allOf well defined in the specification or is there room for interpretation? I've been trying several tools for creating TS types from my OpenAPI specs and it seems each one has its own set of quirks (some don't handle oneOf inside allOf, some support oneOf directly in another schema without allOf, ...), so it's been a real pain, but all the other ones seem to handle deep merging of schemas with allOf. So is this a bug or is this how dtsgen interprets the specification?