search

nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.19k stars 540 forks source link

Support fetch() in Node v14 (and maybe v12) #1018

Closed mcollina closed 3 years ago

mcollina commented 3 years ago

I think it would significantly benefit adoption of undici and standard fetch in Node.js.

Also node-fetch moved to ESM breaking everybody, so there is definitely a way to encourage migration.

ronag commented 3 years ago

Do you imagine we do that by excluding web streams? i.e. we just work on async iterables?

ronag commented 3 years ago

Also node-fetch moved to ESM breaking everybody

Ref?

mcollina commented 3 years ago

From their readme https://github.com/node-fetch/node-fetch/blob/main/README.md:

node-fetch is an ESM-only module - you are not able to import it with require. We recommend you stay on v2 which is built with CommonJS unless you use ESM yourself. We will continue to publish critical bug fixes for it.

I think just support async iterators for v12 and v14 would be fine. Unless we can see to backport web streams to v14 at least and not support v12 for fetch.

sholladay commented 3 years ago

ky-universal would really benefit from fetch() in core supporting web streams back to Node 14.

https://github.com/sindresorhus/ky-universal/issues/25

szmarczak commented 3 years ago

To be honest I have a feeling that supporting lower versions than 16.x can end up being a blocker. If we encounter a bug that is Node.js v14-specific, patches will keep accumulating and it will be just harder to maintain. Once we reach stable I'm okay with supporting lower versions. Just my two cents.

mcollina commented 3 years ago

This makes sense @szmarczak! I'll close this for now - we might reevaluate when/if Node.js v14 ships Web Streams.

KnorpelSenf commented 2 years ago

I don't know much about undici and I'm interested in this. Are the availability of web streams the only thing that prevents fetch from working under Node 14? If so, can I just shim them and use it on 14 anyway?

ronag commented 2 years ago

You can either shim web streams and/or add an option where node streams are used instead of web streams.

ronag commented 2 years ago

Maybe you/we could use https://www.npmjs.com/package/web-streams-polyfill?

KnorpelSenf commented 2 years ago

and/or add an option where node streams are used instead of web streams

You mean PR against undici? Does this have a chance of getting merged?

ronag commented 2 years ago

and/or add an option where node streams are used instead of web streams

You mean PR against undici? Does this have a chance of getting merged?

I'd say 50/50

KnorpelSenf commented 2 years ago

At random? If I should contribute this, it helps to know what a successful merge depends on.