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

feat!: fetch (POC) #618

Draft
wants to merge 44 commits into
base: main
Choose a base branch
from
Draft

feat!: fetch (POC) #618

wants to merge 44 commits into from

Conversation

danielbankhead
Copy link
Contributor

@danielbankhead danielbankhead commented Apr 19, 2024

A proof-of-concept for migrating to native fetch. However, there are a few limitations today.

No native proxy support

It is possible to enable proxy support, however customers would have to install unidici downstream. Including it in this library would make it far too large. Here are some numbers:

package version size (minified)
gaxios 6.5.0 38.3 kB
undici 6.5.0 402.6 kB
@google-cloud/storage 7.10.0 484.2 kB

Adding undici would double the size of a downstream dependency like @google-cloud/storage.

However, there is discussion on adding the ProxyAgent within Node:

Workaround

Potential workaround/fallback for customers:

import {instance, request} from 'gaxios';
import {fetch, ProxyAgent} from 'undici';

instance.defaults.fetchImplementation = fetch;

await request({
  url: '', 
  dispatcher: new ProxyAgent();
})

Environment support

Additionally, it wasn't until recently that undici added env support for HTTP_PROXY, HTTPS_PROXY, & NO_PROXY and it will take time for it to propagate to Node.js releases:

Performance

The native fetch implementation is 26% - 28% slower than node-fetch according to undici's benchmarks:

Returning Streams

Native fetch returns a ReadableStream ('Web Streams') instead of stream.Readable. This may require a rewriting downstream for many applications that currently expects a Node stream.

However, here's a simple way to resolve:

import {Readable} from 'node:stream';

Readable.fromWeb(res.body);

Additional Notes

https://nodejs.org/en/about/previous-releases

Fixes: #625
🦕

@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Apr 19, 2024
Copy link

Warning: This pull request is touching the following templated files:

@ddelgrosso1
Copy link
Contributor

Really cool to see and thank you for digging into the numbers, both size wise and speed wise. Seems like there is a lot of work that still needs to happen on the node side for this to be competitive with node-fetch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat!: Migrate to fetch
3 participants