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

fix fastify deprecation warning #630

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

tomasstrejcek
Copy link
Contributor

@tomasstrejcek tomasstrejcek commented Sep 14, 2023

fixing #628 (node:20436) [FSTDEP017] FastifyDeprecation: You are accessing the deprecated "request.routerPath" property. Use "request.routeOptions.url" instead. Property "req.routerPath" will be removed in fastify@5.

tbh I was expecting it to be bigger :)

side note: from my understanding of process-warning used in fastify, each deprecation warning should be surfaced only once, any idea why do I see it again and again (in our test suites at least, wasn't brave enough to send it to production like this)

@tomasstrejcek
Copy link
Contributor Author

@dnlup can we get this out soon please? :)

@dnlup
Copy link
Contributor

dnlup commented Sep 15, 2023

fixing #628 (node:20436) [FSTDEP017] FastifyDeprecation: You are accessing the deprecated "request.routerPath" property. Use "request.routeOptions.url" instead. Property "req.routerPath" will be removed in fastify@5.

tbh I was expecting it to be bigger :)

side note: from my understanding of process-warning used in fastify, each deprecation warning should be surfaced only once, any idea why do I see it again and again (in our test suites at least, wasn't brave enough to send it to production like this)

Thank you @tomasstrejcek ! In the tests in this plugin it should be expected to see the warning multiple times because each test file runs in a separate process, we also create a new fastify app each time in the tests if I am not mistaken (not sure about if this contributes though). I can confirm I only see one warning on prod environments when launching a server.

Copy link
Contributor

@dnlup dnlup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good here, can you just change the commit message to follow conventional commit convention? Something like fix: remove fastify deprecation warning should do it 😉

@tomasstrejcek
Copy link
Contributor Author

@dnlup ready :)

Copy link
Contributor

@dnlup dnlup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dnlup dnlup merged commit ce910ae into immobiliare:main Sep 15, 2023
10 checks passed
@simonecorsi
Copy link
Collaborator

🎉 This PR is included in version 7.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants