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

ROUTE_ARGS_METADATA missing after using @Transactional decorator #15

Closed
labibramadhan opened this issue Mar 25, 2019 · 6 comments
Closed

Comments

@labibramadhan
Copy link
Contributor

labibramadhan commented Mar 25, 2019

When @Transactional is being used on a method, the method decorator @Body(), @Param(), and @Query() documentations will be gone later on the Swagger docs.

Below is my inspections:

Here is @nestjs/common injecting ROUTE_ARGS_METADATA to controller methods by using its method name as the key, the ROUTE_ARGS_METADATA itself is mainly stored on the target.constructor which is the class where the method is defined, this is going to be used later for @nestjs/swagger to explore available methods/endpoints and its object structure as well that can be found here

The problem is that @Transactional is overwriting the method without preserving the method name, this makes the createPipesRouteParamDecorator() function of @nestjs/swagger will not find any kind of ROUTE_ARGS_METADATA that have been previously stored by @nestjs/common, because the key itself to retrieve ROUTE_ARGS_METADATA stored on the class is the method name

@odavid
Copy link
Owner

odavid commented Mar 31, 2019

Hi @labibramadhan,
Thank you for the contribution!
I believe the issue you describe here, can happen to any decorator declared on the controller method, not just the Transactional.

So according to your suggestion, every decorator need to be changed if used together with NestJS. This does not sound reasonable to me.

Did you try to change the order of decorator declarations?
Meaning the NestJS decorators close to the method and the transactional at the top - something like this:

@Transactional()
@Post() // first the nestJS decorator is fetching the metadata of the method params
async create(@Body() createCatDto: CreateCatDto) {
  this.catsService.create(createCatDto);
}

@labibramadhan
Copy link
Contributor Author

labibramadhan commented Apr 1, 2019

Yeah i tried to reorder and put the @Transactional to the first and then everything gone on the Swagger documentation, the @Post or @Get or @Put or @Delete metadata are missing after being replaced by @Transactional decorator. So when i put the @Transactional decorator to the first (applied last), all endpoint documentations doesn't exists on the Swagger documentation, but when @Transactional being put on the last decorator (applied first), the endpoint still exists on the Swagger documentation but missing the payload @Body or @Query or @Param documentation. So the @Transactional decorator is destroying metadata everything that comes before it.

@odavid
Copy link
Owner

odavid commented Apr 1, 2019

Ok, I will try to find the time to investigate that a bit more, since there is no special code here, just a simple decorator.
It seems to me that no other decorator will work with NestJS and swagger, based on these findings. I am not sure the fix should be done here. But again, need to investigate that...

Cheers

@joemaidman
Copy link

I've also just hit this issue, looks like another 3rd party library may have faced something similar before. Found some links that may help:
nestjs issue: nestjs/nest#1180
nestjs-config 3rd party library PR linked from the nestjs issue: nestjsx/nestjs-config#34

@joemaidman
Copy link

Seems there is already a PR to fix this from @labibramadhan #16

@odavid
Copy link
Owner

odavid commented Apr 26, 2019

#16 Merged - Closing this issue.

@odavid odavid closed this as completed Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants