Proposal: Use Node.js's built-in 'querystring' module instead of 'qs'
Opened this issue · 0 comments
The qs
library is currently used to serialize query strings. The library has previously been free of dependencies, but version 6.10 added a multitude of them. In fact, a large part of googleapis-common
's dependency tree now comes from qs
:
(Reference: https://npmgraph.js.org/?q=googleapis-common#zoom=w&select=exact%3Aqs%406.12.3)
Describe the solution you'd like
I suggest replacing the qs
module with Node.js's built-in querystring
module.
The built-in module is battle-tested, used by Gaxios, and supported since very early versions of Node.js. It was momentarily marked as "legacy" (different to "deprecated"), but the decision has since been reverted: nodejs/node#44911.
There are also potential performance benefits. The above issue links to a performance comparison of different querystring serializers that shows querystring
outperforming other competitors (including qs
): https://github.com/anonrig/fast-querystring
The change can likely be done in a backwards compatible way. Like qs
(as it is used in this library), querystring
does serialize string arrays like myParams: ['one', 'two']
as 'myParams=one&myParams=two'
, as described in this code comment:
nodejs-googleapis-common/src/apirequest.ts
Lines 183 to 185 in 459f034
It can be also configured to encode spaces as %20
instead of +
like described in this code comment:
nodejs-googleapis-common/src/apirequest.ts
Lines 186 to 187 in 459f034
Unlike querystring
, qs
does support serializing arbitrarily nested objects, but this is likely not relevant given that the GaxiosOptions.paramsSerializer
type doesn't allow them.
I have submitted a pull request #564 that suggests a way to implement this change.
Describe alternatives you've considered
Other options include:
-
Using the web standard URLSearchParams, supported since Node.js 10. However, according to a member of the Node.js performance team:
URLSearchParams has significant performance issues. I would actually recommend to use node native querystring implementation. We purposefully removed the deprecation tag so that people dont think legacy means broken.
-
Limiting the
qs
version range to 6.9.x.
Additional context
This library and its dependency to qs
has been mentioned in the JS ecosystem cleanup repository issue here: es-tooling/ecosystem-cleanup#7