perf: replace querystring with fast-querystring

[anonrig]

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[anonrig]

The following two separate tests are only required to be tested with fast-querystring, since fast-querystring does not support additional parameters, and does not apply for fastify-formbody use case.

Here's the link where we put all the node tests: https://github.com/anonrig/fast-querystring/blob/b893058f36c2c1db615d6f238a70d696526ff7af/test/node.ts.

The tests extracted from querystring module that makes sense for stringify

• https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/test/parallel/test-querystring.js#L264
• https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/test/parallel/test-querystring.js#L272

The tests extract from querystring module that makes sense for parse

• https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/test/parallel/test-querystring.js#L212
• https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/test/parallel/test-querystring.js#L222

[anonrig]

@jsumners I forced pushed to include 0.7.1 version where I added more & more tests. It has all the tests on the node.js side, and more taken from several other packages. The code coverage is 98% at the moment.

[jsumners]

I have tried to find a comprehensive test suite for URL search params (https://url.spec.whatwg.org/#urlsearchparams) in both the Firefox and Chromium codebases, but I was unsuccessful. It really seems like there should be a standardized set of strings that implementors can test their code against 😕

I would still rather any direct dependency be mature enough that it is willing to publish itself as v1.0.0.

[kibertoad]

@anonrig what is left on library side until you are comfortable with publishing 1.0.0?

[Uzlopak]

Can we have fast-querystring in the fastify org?
We are not using package-lock.json so we would not see if there are malicious changes. Not saying that anonrig is malicious but thats my only worry as we are basically the first users.

One of the reasons I removed some simple packages from our repos was to reduce the surface for supply chain attacks.

[Uzlopak]

@Fdawgs
@mcollina

We are currently discussing it here

[jsumners]

Can we have fast-querystring in the fastify org?

We are not using package-lock.json so we would not see if there are malicious changes. Not saying that anonrig is malicious but thats my only worry as we are basically the first users.

One of the reasons I removed some simple packages from our repos was to reduce the surface for supply chain attacks.

This is unnecessary. The Fastify org does not need to maintain every library.

[Uzlopak]

I made my arguments.

[mcollina]

A few notes. I'm happy with the fast-querystring module and I also have the publish bit in there.

I asked @anonrig immediately if he could move it to the org, however, he said he would prefer to keep it under his name.

Given that the module is a few days old, let's wait until it's 1.0.0 and then make some decision?

In the meanwhile, would you mind adding an option to configure the querystring module, like it's present in Fastify?

[anonrig]

Sorry for the late reply. I was getting my basic keelboat training in the weekend 🎉

@anonrig what is left on library side until you are comfortable with publishing 1.0.0?

@kibertoad Honestly speaking, nothing. I'm just waiting for a couple of days, where I can't find any more performance improvement.

Can we have fast-querystring in the fastify org? We are not using package-lock.json so we would not see if there are malicious changes. Not saying that anonrig is malicious but thats my only worry as we are basically the first users.

@Uzlopak I've given @mcollina both NPM and Github rights, in case I'm not available to release any changes. If package.json includes a fixed version, this will be averted.

In the meanwhile, would you mind adding an option to configure the querystring module, like it's present in Fastify?

@mcollina I'll update the pull request.

[anonrig]

Fix: Actually fastify-formbody already supports parser as a parameter of opts.

[anonrig]

@jsumners I released 1.0. I think it is stable enough.

[mcollina]

lgtm

[mcollina]

@jsumners PTAL, are you still blocking this?

[jsumners]

@jsumners PTAL, are you still blocking this?

I was at my company "offsite".