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

Metadata gets incorrectly applied/merged when using loadPluginMetadata #2974

Closed
2 of 4 tasks
ReneZeidler opened this issue Jun 10, 2024 · 4 comments
Closed
2 of 4 tasks

Comments

@ReneZeidler
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

When using SwaggerModule.loadPluginMetadata to load metadata generated by the Swagger CLI plugin (which is necessary when using the SWC builder), some of the metadata gets overriden by @ApiProperty annotations even if the annotation doesn't explicitly set that metadata (e.g. the type).

This behavior is different from using the TSC builder with the Swagger CLI plugin directly. @ApiProperty don't override the type inferred by the CLI plugin unless the annotation explicitly contains the type or schema.

Minimum reproduction code

https://stackblitz.com/edit/nestjs-typescript-starter-r45zy5?file=src%2Fapp.dto.ts,src%2Fmain.ts

Steps to reproduce

  1. Open the minimal reproduction. This reproduction contains a metadata.ts file that has been generated by the Swagger CLI plugin and is loaded with SwaggerModule.loadPluginMetadata in main.ts. The content of the metadata file is correct, i.e. the type of both properties myValues and myOtherValues in HelloResponse is number[].
    • Note: Usually you would use nest build -b swc --type-check with this setup, but SWC doesn't work in StackBlitz. However the behavior with TSC is exactly the same when the Swagger CLI plugin is disabled and the metadata is loaded with loadPluginMetadata.
    • Note: The metadata.ts file was generated by temporarily enabling the CLI plugin, then running nest build -b swc --type-check. The build will fail but the metadata will still be generated. You can also run this locally instead of on StackBlitz to actually use a working SWC build, but the bug seems to be in loadPluginMetadata and also reproduces with TSC.
  2. Observe the schema of the return value of the /hello endpoint:
    • myValues has type [string] even though it should be type [number]. This is caused by the @ApiResponseProperty annotation, which doesn't explicitly override the type and should only set readOnly: true.
      • It's interesting that the type is [string] and not string, which suggests that isArray: true is kept from the metadata, while type: "number" gets overriden by the default value type: "string". The resulting type [string] is incorrect and is neither a default value nor makes sense from the source code.
    • myOtherValues has the correct type [number], which is loaded from the metadata.
  3. Compare with the same minimal reproduction with TSC and the CLI plugin. This example doesn't use loadPluginMetadata and instead uses the Swagger CLI Plugin directly (which is only possible with the TSC builder).
    • myValues has the expected type [number]. The @ApiResponseProperty annotation just sets readOnly: true, while the complete type is inferred by the CLI plugin.

Expected behavior

Using SwaggerModule.loadPluginMetadata with a metadata.ts file generated by nest build -b swc --type-check should result in the same API schema as when using nest build -b tsc with the Swagger CLI plugin.

@ApiProperty annotations should not override metadata with values that are not explicitly set by the annotation itself. All the metadata included in the metadata.ts should be used and not get overriden by default values unless explicitly set.

Package version

7.3.1

NestJS version

10.3.9

Node.js version

20.14.0 / 18.20.3

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@ReneZeidler
Copy link
Author

I added another example to the same reproduction URL. The endpoint /badRequest has the annotation @ApiBadRequestResponse({ description: 'Bad request' }). With TSC and the plugin, this gets added to the list of return codes for the endpoint (the default 200 response with the return type inferred from the method return type is still there).
With loadPluginMetadata, the annotation overwrites the defaut response code.

@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this issue?

@ReneZeidler
Copy link
Author

I tried to look if there was a simple fix, but I'm not familiar enough with the codebase to find the root cause of this issue. So I unfortunately can't provide a PR.

@kamilmysliwiec
Copy link
Member

Let's track this here #3051

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants