swagger icon indicating copy to clipboard operation
swagger copied to clipboard

fix: allof client generation handling

Open gausnes opened this issue 3 years ago • 2 comments

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [x] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

NestJS swagger currently uses allOf in somewhat of a weird usage based on examples within the documentation. This is fine for rendering the specification, but causes issues when attempting to generate models and clients off of the emitted specification. OpenAPI Generator is not inspect the required flag for composite schemas.

This is not the only solution to this issue, but was the simplest. I spent some time evaluating fixing this within the generator code, but it was significantly more involved. This change does not feel like an explicitly inerrant as the resulting object of compose schema use is an object.

properties:
  id:
    description: Identifier
    allOf:
      - $ref: '#/components/schemas/Identifier'
  name:
    type: string
required:
  - eventId

What is the new behavior?

Typing these properties as objects helps the generator inspect the expected fields for correct generation.

properties:
  id:
    description: Identifier
    type: object
    allOf:
      - $ref: '#/components/schemas/Identifier'
  name:
    type: string
required:
  - eventId

Does this PR introduce a breaking change?

  • [ ] Yes
  • [x] No

Other information

gausnes avatar Mar 31 '22 22:03 gausnes

Is it possible to add these changes to main repo?

TiBarification avatar Apr 03 '23 17:04 TiBarification

We are also impacted by this problem. What is required for the merge?

fredx21 avatar Apr 04 '23 14:04 fredx21