-
Notifications
You must be signed in to change notification settings - Fork 511
Description
Did you read the migration guide?
- I have read the whole migration guide
Is there an existing issue that is already proposing this?
- I have searched the existing issues
Potential Commit/PR that introduced the regression
Versions
11.1.3 -> 11.1.4
Describe the regression
It's a bit ironic, since I'm the one who contributed this PR.
When relying solely on this library, throwing this error log makes sense. Although, for my specific use case, it doesn't. I'll elaborate.
Initially, I used this package and the CLI to generate the OpenAPI spec as strict as I could. Over time, I understood that solely relying on the CLI to automatically type the schemas would be very challenging. Since reflect-metadata is quite restricted, it doesn't support out of the box interfaces, unions, enums, etc. Although I could leverage the decorators that this library brings (such as @ApiProperty
), it felt like a lot of manual work. Additionally, non-exported DTOs weren't "inlined" to the exported one, but rather defined in the document, which caused lots of conflicts. For example:
class NonExportedInnerDto {
value: string;
}
export class ExportedWrapperDto {
inner: NonExportedInnerDto;
}
was transformed to:
{
"NonExportedInnerDto": {
"type": "object",
"properties": {
"value": {
"type": "string"
}
},
"required": ["value"]
},
"ExportedWrapperDto": {
"type": "object",
"properties": {
"inner": {
"$ref": "#/components/schemas/NonExportedInnerDto"
}
},
"required": ["inner"]
}
}
Instead of solely relying on the library to generate the schemas, I ended up using this library for tracking the relevant endpoints, and ts-json-schema-generator
for generating the schemas. There's of course a tradeoff where it's AOT rather that JIT, but the way it worked, I ran a script which:
- Creates an OpenAPI document using this library
- Leverages
ts-json-schema-generator
to scan all of the exported modules inside*.dto.ts
- Merged the definitions to the document
- Treeshaked the end result (so it removes "orphan" schemas).
And then I expose this generated JSON in a NestJS controller. This way, non exported DTOs were inlined to the exported one with all of the other benefits.
I know that my setup is very specific and throwing this kind of error would probably make sense for other people who're relying on reflect-metadata, but I truly believe that we shouldn't throw this message, or at least disable it under a feature flag.
As a side note, I think this library should pursue after better schema inference than reflect-metadata.
Minimum reproduction code
No response
Expected behavior
Since inlining the non-exported interfaces is a big change for this library, I think hiding the error using a feature flag would be ok
Other
No response