feat: replace querystring with fast-querystring

[anonrig]

Solves issue: fastify/fastify#4252

[delvedor]

Heya, the library looks great! If we end up merging this, we should release it in a major. Before doing so, can you add more test to fast-querystring? Ideally the same test suite of querystring core.

[anonrig]

@delvedor I added all of the node.js querystring tests and released a new version (0.3.0) with the necessary changes to replicate the behavior of the node:querystring package.

[ivan-tymoshenko]

LGTM! Update the doc, please.

[anonrig]

@ivan-tymoshenko I just updated fast-querystring module to 0.3.1 which comes with ~7% faster parsing and updated the README.

[anonrig]

Updated to 0.4.0 with lots of great improvements.

[Fdawgs]

This repo supports node >=14, but fast-querystring uses replaceAll(), which requires Node >=15.

[mcollina]

This will likely land in a major version, ideally shipping February/March 2023, in which we will drop support for Node v14 and v16.

(v16 LTS ends Sept 2023 due to OpenSSL).

[anonrig]

I might replace replaceAll with replace if that is a deal breaker. The performance difference is acceptable since even without it, the package is quite fast. @Fdawgs

[anonrig]

FYI: Updated this pull-request to 0.7.0 (40% faster than querystring atm), dropped replaceAll in favor of supporting Node 14.

[mcollina]

lgtm, I think this would be a significant benefit for the project.

[RafaelGSS]

LGTM. Good job!

[anonrig]

I released and updated the pull request to use 1.0.0 version. I think fast-querystring is stable.

[delvedor]

LGTM

[mcollina]

@delvedor should I cut a major?

[ivan-tymoshenko]

@mcollina @delvedor Can you explain to me why we should land it as a major? It's not a breaking change. It's not even a new feature. It might have a good querystring performance parsing upgrade, but it will not make a significant performance difference along with routing. And we have the 7.1.0 version now. Why can't we land it as a 7.2.0?

[mcollina]

I'm ok with a minor too.

[anonrig]

If we are ok with this pull request, can we merge & release @mcollina @delvedor?

[delvedor]

Can you explain to me why we should land it as a major?

The main reason is that this module has an average of 3.6 millions downloads per months, and you don't want to risk it :D
If we are confident there are no hidden breaking changes, we can do a minor.

[mcollina]

I'll try a minor and in case revert