Investigate replacing `qs`

[TheDevMinerTV]

Pros

• Less subdependencies and smaller size:
  • ~1MB (qs) vs. ~30 kB (fast-querystring)
  • https://npmgraph.js.org/?q=qs vs https://npmgraph.js.org/?q=fast-querystring
  • Also less bloat (14 deps vs 1 deps)
    • qs's subdeps have a lot of recursion in require cycles as well
    • all of qs and it's dependencies is maintained by the same developer (could be a security concern)
  • Might be useful for serverless folks that use express (if there's platform that allows express to be used, at least)

Cons

• fast-querystring requires at least Node 8:
  • it uses object destructuring and trailing commas
    • it might be possible to convince the author to remove the trailing commas, then it would be usable with Node 6
  • Node 8 has been out of support for more than 5 years now (support stopped on Jan 1st 2019, according to endoflife.date/nodejs)
  • Node 0.10 is even older, I can't even find an EOL date for it
• fast-querystring doesn't support setting prototypes
  • this is unsafe either way, though it can probably get released in express v5

Notes

• Slightly better performance:
  
  • this test was done using the benchmark folder in this repo and just replacing the require call to fast-querystring on a idling 7950X running Node v8.17.0
• It might be useful to maybe port to using URLSearchParams which has been supported since Node 10
  • just looked at Replacing querystring with fast-querystring #4990 which mentioned URLSearchParams to be pretty slow, though, this needs to be benchmarked first

[ctcpip]

It can't be done in Express 4; as you correctly noted, it would require a higher minimum version of Node. Express 5 will require a minimum of Node 18, and if a dependency is no longer required, I would rather just drop it. However, a significant perf difference could be a compelling enough reason to use it over built-ins.

[TheDevMinerTV]

Sounds good, do you want me to make a PR to use the built-in URLSearchParams?

[ctcpip]

@TheDevMinerTV if you want to make a PR targeting 5.x, that would be great

[TheDevMinerTV]

Will do ^^
Can you assign me to this issue?

[krzysdz]

In #4990 it was suggested to replace querystring (built-in Node module) with URLSearchParams.
Now Express offers 2 "query parser" values:

1. "simple" - using querystring, default since 5.0-beta.1
2. "extended" - using qs, default in 4.x

Does replacing both of them with URLSearchParams make sense? Doesn't qs have more features than URLSearchParams?

[TheDevMinerTV]

URLSearchParams is the standard way of doing query param parsing nowadays. We should probably stick to that, especially since it will work in all runtimes because it's in the ECMAscript URL spec. Though, using node:querystring would also be fine, I guess.

[ctcpip]

your point is valid, but note that URLSearchParams is part of the URL spec, not ECMAScript

node:querystring is not sufficient because it doesn't handle deep nesting

[TheDevMinerTV]

URLSearchParams doesn't seem to parse /?foo[0][bar]=baz&foo[0][fizz]=buzz&foo[]=done! into {"foo":[{"bar":"baz","fizz":"buzz"},"done!"]}, but rather as {"foo[0][bar]":"baz","foo[0][fizz]":"buzz","foo[]":"done!"}. So it doesn't support automatic array parsing (which can be worked around, but that might be vulnerable). This was also described here: sindresorhus/got#1559 (comment)

Edit: fast-querystring also doesn't implement this.

Edit: This is very likely because the spec never mentions arrays. I'm not sure how to proceed here. Should we implement (or pull in a package for) the array parsing algorithm, if so, do we use the comma or the index syntax. Or should we drop the feature? IMO we should implement this ourselves with the index syntax.

[ctcpip]

sounds like we should continue to use qs

[TheDevMinerTV]

There seems to be picoquery which is ~40KB and seems to support the different modes but seems to fails with parsing on some stuff because it only supports on syntax at a time. I've opened an issue on their repository ^

[dominikg]

sounds like we should continue to use qs

In the spirit of the very first line of your readme

Fast, unopinionated, minimalist web framework for Node.js.

I believe it would be great to provide a URLSearchParams based interface by default and offer pluggable support for qs or other parsers to allow users to decide for themselves without forcing qs on them.

qs is quire large https://pkg-size.dev/qs and most applications today are just fine with using URLSearchParams on the server and client. It even has basic array support built in new URLSearchParams('foo=1&foo=2').getAll('foo')

[krzysdz]

I believe it would be great to provide a URLSearchParams based interface by default and offer pluggable support for qs or other parsers to allow users to decide for themselves without forcing qs on them.

There is a query parser setting, which allows users to configure any query parser they want. Since 5.0 it defaults to "simple" (which may move from node:querystring to URLSearchParams - #4990 (comment); PRs welcome), with qs left as the "extended" option for compatibility.

Completely removing qs and the "extended" option probably would require deprecating it first.

[43081j]

it probably does make sense that the query parser setting can be simple or a function (in some future breaking version)

since then you can use whatever query parser you like by passing it in, without having to bloat express itself with such a library

[dominikg]

Another option for 5.0 could be to make qs an optional peerDependency and users opting for the "extended" setting would have to install it manually.