Comparison with node-fetch

[bitinn]

I think there are some inaccuracies:

• Stream API: node-fetch has res.body.pipe.
• Custom defaults: node-fetch does merge Headers and Request options correctly per spec.

Not butthurt of anything, just bump into this table and think the comparison can be improved.

[sindresorhus]

Stream API: node-fetch has res.body.pipe.

That's just the built-in Node.js http.request response body stream, right? Which is not that useful for most cases, as you'll lose the benefit of node-fetch, compatibility with Fetch. Got has a dedicated .stream() method that returns a stream that also proxies headers and other things. For node-fetch, the equivalent would be node-fetch/node-fetch#387.

Custom defaults: node-fetch does merge Headers and Request options correctly per spec.

That's just a small part of it. The other libs that support 'custom defaults' supports it for all options.

[bitinn]

@sindresorhus I see your take, But:

• we didn't switch to web stream because node-fetch are for nodejs and at the time getReader wasn't available. res.body.pipe isn't a standard API in Fetch spec but .pipe is exactly what people are looking for on nodejs. Note that we also support the standard .clone API which operates on stream.
• I just think it's unfair to categorize got as having stream api while node-fetch is categorized as doesn't simply because of spec compliance issue.
• As for the custom defaults, sure, if your argument is "only merging the main instance" counts.

[sindresorhus]

res.body.pipe isn't a standard API in Fetch spec but .pipe is exactly what people are looking for on nodejs.

But people use node-fetch to have a familiar API or most often identical API on the server-side as in the browser and this breaks that promise.

I just think it's unfair to categorize got as having stream api while node-fetch is categorized as doesn't simply because of spec compliance issue.

It would also be unfair to got and request if it was categorized as such though.

• node-fetch is isomorphic, but only supports a "stream" API for Node.js.
• The stream API is not a first-class citizen. It's just exposing the Node.js response stream. It doesn't have nice conveniences like forwarding headers, redirect handling, etc.

We can add a ✖ [1] and add a note that node-fetch somewhat supports streams.

[bitinn]

node-fetch is isomorphic, but only supports a "stream" API for Node.js.

• It's not isomorphic though, see https://github.com/bitinn/node-fetch#motivation
• We did try very hard to stay consistent with Fetch API, but it's not perfect yet: https://github.com/bitinn/node-fetch#difference-from-client-side-fetch
• And it makes sense to have things like res.body.pipe and timeout because at the time of release Fetch Spec doesn't even provide a way to abort a request and web stream isn't ready for use in any browsers.

The stream API is not a first-class citizen. It's just exposing the Node.js response stream. It doesn't have nice conveniences like forwarding headers, redirect handling, etc.

• I see another side on this: Fetch Spec already provide a compliant way to access these info, and they are available on fetch resolve, so why would you not use it?
• getReader isn't going to stream these info anyway: https://developer.mozilla.org/en-US/docs/Web/API/Streams_API/Using_readable_streams

[bitinn]

PS: I know all these are just technicality and we both have better things to do than correcting a comparison table, so consider this a suggestion for change not a request :)

[szmarczak]

Let's summarize: node-fetch aims to be compatible with the fetch, we should not expect any additional features. So if there are any, they're node-specific. No need to argue about node-specific things (proxying headers etc.).

I don't mind ticking node-fetch. But I think we should leave a note that it's a bit different than the window.fetch.

@bitinn It'd be very nice to have .getReader() :)

@sindresorhus thoughts?

[bitinn]

@szmarczak love to get .getReader() working in some way. We were expecting whatwg stream to be a part of nodejs core but the PR didn't land. (see nodejs/node#22352), so using a polyfill might be the only solution for now.