Remove duplicate properties from objects

[OlPainless]

I noticed that generated code can have duplicate properties in case allOf section also includes a reference to an object that has the same properties.

I hope my ad-hoc fix is okay!

Regards, Andreas

[sbusch]

Is there a problem having duplicate properties in the generated code (besides being ugly) that needs to be fixed?

I see a problem with your change: for same property names but different definitions (e.g. different types), information gets lost. IMO the comparison needs to be performed deeply, not shallowly by just comparing the names.

[OlPainless]

Actually, the generated code is invalid:

export type Foo = {
    'name' ? : string;
    'rank' ? : number;
};

export type Bar = {
    'name' ? : string;
    'rank' ? : number;
    'rank' ? : number;
};

which Typescript complains about. I also tested the spec in https://editor.swagger.io/, and only one instance of a parameter is present.

However, I did discover that there is a difference in ordering (if we have different types):

  Bar:
    allOf:
      - $ref: '#/definitions/Foo'
      - type: object
        properties:
          rank:
            type: string

produces the following object (in Swagger editor, that is):

{
 name:  string
 rank:  string
}

While if we switch places:

  Bar:
    allOf:
      - type: object
        properties:
          rank:
            type: string
      - $ref: '#/definitions/Foo'

we'll get different output:

{
 rank:    number
 name:  string
}

Unfortunately, I don't think typescript-codegen has any information on the order of the constructs, so I'm not sure we can produce exact same output...

Any ideas?

[OlPainless]

Addition: it seems the ordering issue could be fixed by reversing the concatenated array; that way the uniq will pick the overriding properties first.

[mtennoe]

Thanks for contributing @OlPainless! This is one of the places where this package doesnt really follow the spec completely, and we want to follow the spec properly At Some Point(TM).

Re. ordering - We sadly dont have that information today. We can ofc. add that based on the parsed order from the JSON input, but I kinda like your idea of just reversing the concatenated string, as it seems like a simple way to address the issue. We should reverse it before outputting again though to keep the original order

[OlPainless]

Alright, cool!

We should reverse it before outputting again though to keep the original order

Did I understand correctly that we'd just do this:

const properties = reverse(
    uniqBy(
      reverse(
        concat(
          getAllOfProperties(swaggerType, swagger),
          getObjectProperties(swaggerType, swagger, requiredPropertyNames)
        )
      ),
      "name"
    )
  );

If so, I can make the change immediately. I'll also add a comment explaining what is going on with the code.

[mtennoe]

Yup, something like that should work. Would be nice with some descriptive temporary consts for the sake of readability

[mtennoe]

@OlPainless - Approved, merged and published as 1.9.7. Thanks for contributing!

[OlPainless]

Thanks everyone, it was quite fun :)