-
Notifications
You must be signed in to change notification settings - Fork 46
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 nestjs fastify platform warn #231
fix nestjs fastify platform warn #231
Conversation
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.
We used optional chaining in fastify/fastify-static#410, I'd like to use it here as well, just to be safe
having the same problem currently |
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
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.
I wanted to fast-track this PR but it is breaking older fastify versions now, after I applied the suggestions of @gurgunday
Which version is it breaking? The CI Looks Green To Me :) |
Checkout this repo and checkout this PR. install fastify 4.20.0. Ensure that you have in node modules fastify version 4.20.0. run npm test. |
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.
lgtm
Let's bump a major, they are cheap. |
Let me confirm you that |
It's because of the test file that was changed in #230
|
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.
Test file needs to be fixed as well
@mcollina I wish we'd just done that from the beginning, but so far we've fixed breakages instead |
Can you please patch this PR accordingly? Fix the tests locally and then in this page press the "." key on your keyboard. This opens the github editor for this Pr. You can then apply the changes and can commit as usual. I am not at my pc, so can not do it myself. |
Wow, I didn't know this existed, so cool haha Tests now pass on [email protected] |
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.
LGTM
Now somebody has to release |
fixed warn:
FastifyDeprecation: Request#context property access is deprecated. Please use "Request#routeConfig" or "Request#routeSchema" instead for accessing Route settings. The "Request#context" will be removed in fastify@5.
dep:
"@nestjs/platform-fastify": "10.0.3",
Checklist
npm run test
andnpm run benchmark
and the Code of conduct