body-parser icon indicating copy to clipboard operation
body-parser copied to clipboard

Replacing `qs` with `qs-esm` or `picoquery`

Open Fuzzyma opened this issue 1 year ago • 2 comments

I want to propose to move from qs to qs-esm or picoquery.

qs seems to pull in a lot of dependencies that are not needed in your specified engine anymore Dependencies_for_qs_dependencies

qs-esm has the same engine requirements than the newest body parser version. However, since require(esm) will most likely not be available for node 18, picoquery seems to be the only viable solution.

I would be happy to create a PR if you are on board with this.

Fuzzyma avatar Dec 08 '24 16:12 Fuzzyma

This is related to #453 (discussion around custom parameters for qs)

Also, worth noting that current usage of qs includes more configuration options than are supported by picoquery:

https://github.com/expressjs/body-parser/blob/80fbb762b1e326d9a7589df9285bf351d5685f35/lib/types/urlencoded.js#L165-L174

(For reference, these are the options supported by picoquery) :

{
  nesting: true,
  nestingSyntax: 'dot',
  arrayRepeat: false,
  arrayRepeatSyntax: 'repeat',
  delimiter: '&'
}

dpopp07 avatar Dec 09 '24 17:12 dpopp07

This has been brought up a bunch of times, and I dont have time right now to go find all my responses, but the TLDR is that I remain unconvinced that transitive dependency count is a metric we want to optimize around. If you can make a case which is more comprehensive around the performance impact of installs then please do so, but this conversation has pretty much stalled out every time I have asked this of anyone in the past.

edit: found this which I think this was the most comprehensive issue opened so far: https://github.com/expressjs/express/issues/5723

wesleytodd avatar Dec 09 '24 17:12 wesleytodd