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
fix(core): middleware does not apply to a controller that has versioning #10835
Conversation
4516157
to
5fc41a9
Compare
76cc416
to
6670ba2
Compare
6670ba2
to
01b014b
Compare
Can we add an integration test that confirms that this PR solves the issue? |
01b014b
to
de41804
Compare
de41804
to
682db93
Compare
Done |
Pull Request Test Coverage Report for Build bd211e24-0917-494a-a6f1-e5fdf684b84f
💛 - Coveralls |
return { | ||
path, | ||
requestMethod, | ||
targetCallback: instanceCallback, | ||
methodName, | ||
version, | ||
version: methodVersion || controllerVersion || globalVersion, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIR this may cause some unexpected side-effects.
RouterExplorer#applyPathsToRouterProxy
assumes that "version" attribute represents the method-level version https://github.com/nestjs/nest/pull/10835/files#diff-652791ce58d92c15826b2be408ac4b6b0b1c7269664536c3d2e89b9e6f49699fR144 while now it equals to "method"/"controller"/"global"-level.
Ideally, to avoid regressions, we should make sure not to update the "scanForPaths" method (leave it as is) :(
Hi contributors, Currently, I'm facing this issue in my application, I'm currently running with these packages versions: "@nestjs/common": "9.4.3",
"@nestjs/core": "9.4.3", The solution has been published to any latest version of NestJS? |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: 10847
However, the middleware didn't apply to controllers which contain versioning at the class level (
version
property inside@Controller
decorator)What is the new behavior?
Passing a controller that has class-level versioning or global versioning is enough to apply middleware on it.
Does this PR introduce a breaking change?
Other information