Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Use Node.js's built-in 'querystring' module instead of 'qs' #563

Open
jviide opened this issue Jul 21, 2024 · 0 comments · May be fixed by #564
Open

Proposal: Use Node.js's built-in 'querystring' module instead of 'qs' #563

jviide opened this issue Jul 21, 2024 · 0 comments · May be fixed by #564
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@jviide
Copy link

jviide commented Jul 21, 2024

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:

Screenshot 2024-07-21 at 18 13 57

(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:

// When forming the querystring, override the serializer so that array
// values are serialized like this:
// myParams: ['one', 'two'] ---> 'myParams=one&myParams=two'

It can be also configured to encode spaces as %20 instead of + like described in this code comment:

// This serializer also encodes spaces in the querystring as `%20`,
// whereas the default serializer in gaxios encodes to a `+`.

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:

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

@jviide jviide added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Jul 21, 2024
@jviide jviide linked a pull request Jul 21, 2024 that will close this issue
4 tasks
@jviide jviide changed the title Proposal: Use Node.js's builtin 'querystring' module instead of 'qs' Proposal: Use Node.js's built-in 'querystring' module instead of 'qs' Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants