perf: replace querystring with fast-querystring
PR Checklist
Please check if your PR fulfills the following requirements:
- [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
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
- [x] Other: Performance boost
What is the current behavior?
Replacing querystring with fast-querystring improves performance
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
- [ ] Yes
- [ ] No
Other information
This is about an 25% increase compared to the original querystring library. 727674.83 op/sec vs 913716.89 op/sec (ran the benchmarks on my machine to ensure the same number of requests) so the increase is nice, but the library also only has 3 stars and depends on a library that hasn't been touched since 2019 with 19 stars. That library truly may be feature complete, and it doesn't have any dependencies, but it's still something for us to consider.
My vote is to keep an eye on this parsing implementation, but not to bring it into Nest quite yet, as it's still very fresh.
Thanks for the comment @jmcdo29. You're absolutely right about the age of the library. I released it couple of days ago, but the benchmarks and the test suite (which is a replica of node.js querystring module) speak for itself.
PS: find-my-way and fastify are already in progress of replacing node:querystring with fast-querystring (https://github.com/delvedor/find-my-way/pull/300) and I thought nest.js can share the same performance boost when the time comes.
Regarding fast-decode-uri-component, it has 19 stars, but has a weekly download of 784,858 and provides a faster solution to decodeURIComponent function. The star count & fork count, does not really mean anything in terms of performance & engineering since there is a query string parsing library with 7300+ stars, and 3x worse performance than this package.
the benchmarks and the test suite (which is a replica of node.js querystring module) speak for itself.
True, and that's definitely good to see. I'm usually hesitant with brand new libraries just to make sure that there will be long term support for them.
PS: find-my-way and fastify are already in progress of replacing node:querystring with fast-querystring
Oh interesting, seeing that fastify is already planning for this, looked like in February/March, we may try to do the same around then. Let's see what @kamilmysliwiec thinks as well.
The star count & fork count, does not really mean anything in terms of performance & engineering
True, but it can help give insight into how much the library is used by the community and how backed it is in the case of a dev leaving maintenance of it (more stars, more likely to have someone fork it usually).
Somehow npm i updates package-lock.json with a massive change list. (I'm using Node 18.8 with npm 8.18)
Somehow
npm iupdates package-lock.json with a massive change list. (I'm using Node 18.8 with npm 8.18)
Ah, yeah, we're still using npm 6 with a package-lock.json v1. npm 8 uses v2 and changes the structure
Hey @jmcdo29 I just released 0.7.0 which includes more performance improvements. Can you redo your benchmark when you have the chance? I'm quite curious about a real-time improvement of fast-querystring. Thanks!