-
Notifications
You must be signed in to change notification settings - Fork 651
Description
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: https://github.com/googleapis/nodejs-googleapis-common/blob/459f0340661bf264acdbd76e038d9024bdebb96f/src/apirequest.ts#L183-L185
It can be also configured to encode spaces as %20 instead of + like described in this code comment: https://github.com/googleapis/nodejs-googleapis-common/blob/459f0340661bf264acdbd76e038d9024bdebb96f/src/apirequest.ts#L186-L187
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.
(See: migrate googleapis to object spread/assign and URLSearchParams e18e/ecosystem-issues#7 (comment))
-
Limiting the
qsversion 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: e18e/ecosystem-issues#7
