You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/12/16 13:46:15 UTC
[GitHub] [couchdb-nano] glynnbird opened a new pull request, #314: Refactor to use fetch instead of axios
glynnbird opened a new pull request, #314:
URL: https://github.com/apache/couchdb-nano/pull/314
## Overview
Replaces `axios` HTTP library with `fetch`, powered-by `undici`. The upshot of this is that we reduce the number of dependencies to 1 and possibly 0 in the future.
> Note this is for merging after April 2023 when Node 14 becomes end-of-life but discussion as to whether Nano should go in this direction is welcome in this PR.
Comments and advice welcome.
### fetch
Some history: originally Nano was built on top of the [request](https://www.npmjs.com/package/request) library which was later deprecated. At this point I refactorted it to use [axios](https://www.npmjs.com/package/axios) instead. This PR eliminates `axios` and other axios-related dependencies and instead uses the new kid on the block: the `fetch` API
The `fetch` feature has found widespread adoption in web browsers as a means of handling outbound HTTP requests. It has found its way into Node.js as a [global function](https://nodejs.org/docs/latest/api/globals.html#fetch) and is marked as an experimental feature in Node 18/19 and will likely be mainstream in Node 20.
> Note: there's a small chance that `fetch` is removed, or implemented differently until it loses its experimental status.
Node.js's `fetch` capability is powered by the [undici](https://www.npmjs.com/package/undici) package which in turn uses Node's low-level network libraries instead of being based on the higher-level `http`/`https` built-in modules. It purports to be [significantly faster](https://undici.nodejs.org/#/?id=benchmarks) (according to its own benchmarks) thant traffic routed through `http`/`https` modules, as is the case with other HTTP libraries like `axios` & `request`.
As we're using a feature marked 'experimental' by Node.js, executing a Node.js script using this branch's Nano would produce the following warning:
> (node:43358) ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time
### Automated testing
Replacing `axios` with `fetch` means also ditching the [nock](http://www.npmjs.com/package/nock) library for mocking HTTP requests and responses, because nock works by intercepting requests originating from the `http`/`https` layer, which is bypassed by `undici`. Fortunately, `undici` provides its own [Mocking tooling](https://undici.nodejs.org/#/docs/api/MockAgent).
### The outcome
This branche's runtime dependents are one npm module: `undici`. This module is required because we use its `Agent` class to allow nano users to override timeouts and connection pooling parameters, and during testing to mock requests with its `MockAgent`. These classes are not exposed by Node.js, so we need the `undici` dependency - for now.
Current dependencies:
```
"dependencies": {
"http-cookie-agent": "^4.0.2",
"@types/tough-cookie": "^4.0.2",
"axios": "^1.1.3",
"qs": "^6.11.0",
"tough-cookie": "^4.1.2",
"node-abort-controller": "^3.0.1"
},
"devDependencies": {
"@types/node": "^18.11.9",
"jest": "^29.2.2",
"nock": "^13.2.9",
"standard": "^17.0.0",
"typescript": "^4.8.4"
}
```
Post-PR dependencies:
```
"dependencies": {
"undici": "^5.14.0"
},
"devDependencies": {
"@types/node": "^18.11.15",
"typescript": "^4.9.4"
}
```
### Backwards compatibility
None of Nano's API has changed _except_ when a user is supplying non-default connection handling parameters. Gone is `requestDefaults` which dates back to the "request" days and instead an optional `agentOptions` can be provided which is documented in the README and in TypeScript.
```js
const agentOptions = {
bodyTimeout: 30000,
headersTimeout: 30000,
keepAliveMaxTimeout: 600000,
pipelining: 6
}
const nano = Nano({ url: 'http://127.0.0.1:5984', agentOptions })
```
### Node versioning
It's not all plain sailing. The `undici` library only works on Node 16.8 onwards. As it happens, Node 14 is E.O.L in April 2023, so that might be a good time to merge this PR and release a version 11 of Nano - older versions of Nano will still work for folks with older Nodes, but Nano 11+ would be for Node 16+ i.e. all Long-Term Supported Nodes.
## In summary
_If_ Node.js ends up sticking with `undici`'s implementation of `fetch`, then this PR would allow this project to have fewer dependencies and potentially faster performance. I'm relucantant to merge this yet until `fetch` is no longer experimental and Node.js 14 is still supported.
## Testing recommendations
The test suite has been rewritten to use the built-in Node.js test runner (one fewer dependency!) and uses the `undici.MockAgent` to simulate responses for each of Nano's API calls, just as Nock did previously.
Run with:
```js
npm run test
```
## GitHub issue number
Fixes https://github.com/apache/couchdb-nano/issues/307
## Related Pull Requests
n/a
## Checklist
- [x] Code is written and works correctly;
- [x] Changes are covered by tests;
- [x] Documentation reflects the changes;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] glynnbird commented on pull request #314: Refactor to use fetch instead of axios
Posted by GitBox <gi...@apache.org>.
glynnbird commented on PR #314:
URL: https://github.com/apache/couchdb-nano/pull/314#issuecomment-1361193936
My simple benchmarking of small HTTP requests measures that Nano "11" would be 5 to 10% faster than Nano 10.1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use fetch instead of axios [couchdb-nano]
Posted by "2colours (via GitHub)" <gi...@apache.org>.
2colours commented on PR #314:
URL: https://github.com/apache/couchdb-nano/pull/314#issuecomment-2083657971
@glynnbird Hi, any update or idea about how to progress this? It's not just one less dependency but axios itself has questionable code quality - along with the workarounds that currently make this package work with axios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org