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

feat(plugin) add support for @Expose() and @Exclude() decorators #2777

Merged
merged 1 commit into from
Feb 7, 2024
Merged

feat(plugin) add support for @Expose() and @Exclude() decorators #2777

merged 1 commit into from
Feb 7, 2024

Conversation

kurt-west
Copy link
Contributor

@kurt-west kurt-west commented Jan 15, 2024

PR Checklist

Please check if your PR fulfills the following requirements:


PR Type

What kind of change does this PR introduce?

  • 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?

The Swagger plugin will automatically annotate all DTO properties with @ApiProperty unless @ApiHideProperty is used.

Issue Number: #2603

What is the new behavior?

This PR adds support to include or hide DTO properties from the generated OpenAPI definitions via the existing class-transformer decorators.

The new classTransformerShim option can be set to one of the following:

  • false: No changes to the current behavior
  • true: Any property decorated with @Exclude() will be hidden from the the generated definition; same behavior as @ApiHideProperty()
  • 'exclusive': Only properties that are decorated with @Expose() or @ApiProperty will be included in the generated definition

NOTE: default is classTransformerShim: false

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Small unrelated change to test/plugin/readonly-visitor.spec.ts to help with test failing due to end-of-line characters differences between host machine and PluginMetadataPrinter output.

Edit: grammar

@kamilmysliwiec kamilmysliwiec merged commit 876d17a into nestjs:master Feb 7, 2024
1 check passed
@kamilmysliwiec
Copy link
Member

LGTM

@gustavopedroni
Copy link

👀

@gustavopedroni
Copy link

@kurt-west
It not working when @expose or @exclude is decorating a class

@kurt-west
Copy link
Contributor Author

@kurt-west It not working when @expose or @exclude is decorating a class

@gustavopedroni - this PR follows the existing plugin pattern of decorating properties, not classes.

The class visitor could be updated to pass @expose()/@exclude() metadata down to the property visitor to match the class-transformer pattern of class or property decoration. @kamilmysliwiec - if you'd support this I can submit a PR.

@kalu06
Copy link

kalu06 commented Mar 20, 2024

Thanks for this contribution, that's a great feature.
While playing with it, it seems to not take into account @Expose groups, making it impossible to have correct Swagger definition when reusing the same DTO but with different serialization groups.

Any idea how to achieve that?

Thanks

@kurt-west
Copy link
Contributor Author

@kalu06

How it works:
The implementation of this feature is based on the existing decision logic of the @ApiProperty() and @ApiHideProperty() decorators. Ignoring the annotateAllProperties: true option, if a property is decorated with @ApiProperty() or @Expose() it is allowed to pass onto the property inspection method.

Is it possible:
If you can manually generate your desired schema using the @ApiProperty() decorator with only the knowledge of the class-transformer decorator arguments and the property itself then it should technically be possible to update the property inspector to generate the desired schema.

If you post a manually generated dto I can dig deeper and see if we can come up with a feature proposal for the Nest team.

@kurt-west
Copy link
Contributor Author

@kurt-west It not working when @expose or @exclude is decorating a class

@gustavopedroni - this PR follows the existing plugin pattern of decorating properties, not classes.

The class visitor could be updated to pass @expose()/@exclude() metadata down to the property visitor to match the class-transformer pattern of class or property decoration. @kamilmysliwiec - if you'd support this I can submit a PR.

@gustavopedroni
I have have class-level expose/exclude working in my personal project, if I don't run into any issues by the end of the month I'll submit a PR.

@thw0rted
Copy link

thw0rted commented Nov 1, 2024

Hi @kurt-west , did you ever wind up submitting the PR for class-level exclude? I didn't see it in a cursory search so I figured I'd ask.

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

Successfully merging this pull request may close these issues.

5 participants