googleapis / googleapis/google-cloud-node

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

Open
#7,705 1 comment 7 reactions 0 assignees View on GitHub
library: google-cloud-node-core priority: p3 type: feature request
Dominant language
TypeScript
Stars
3.2k
Forks
712
Avg merge
2d 3h
Merged PRs (30d)
99

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

![Screenshot 2024-07-21 at 18 13 57](https://github.com/user-attachments/assets/bce914b9-c158-4578-9f65-1f5853bb8574)

(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`](https://nodejs.org/api/querystring.html) 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: https://github.com/nodejs/node/issues/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`](https://cloud.google.com/nodejs/docs/reference/gaxios/latest/gaxios/gaxiosoptions#gaxios_GaxiosOptions_paramsSerializer_member) 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](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams), supported since Node.js 10. However, according to a member of the [Node.js performance team](https://github.com/nodejs/performance):
> 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: https://github.com/43081j/ecosystem-cleanup/issues/7#issuecomment-2227991384)

* 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](https://github.com/43081j/ecosystem-cleanup) issue here: https://github.com/43081j/ecosystem-cleanup/issues/7

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.