zkat/make-fetch-happen

Clarify make-fetch-happen is using a fork of node-fetch

bitinn opened this issue · 4 comments

Hi,

I am sorry to be a bit pedantic here, as node-fetch-npm and node-fetch have identical API.

But I am seeing a few comments that confuses node-fetch-npm with node-fetch, and the description here states "make-fetch-happen is a Node.js library that wraps node-fetch with additional features it doesn't intend to include" is less than ideal. As node-fetch is not a direct dependency.

I would say linking to node-fetch-npm instead would be much more accurate.

zkat commented

Oh, I think that was probably written before we forked off node-fetch-npm. I would gladly take a PR to reword this in a way you find satisfactory, if you would be interested in that?

As far as other concerns go: it's very important to me that node-fetch get its fair share of credit. I might not have time to do it at the moment but it's very much the intention to go ahead and sync up with mainline node-fetch at some point. We just needed a few quick patches available ASAP and it's useful for us to have a fork handy that we can patch at our own pace, because of how critical that particular library is. So, it would be good for any edits to refer back to node-fetch as well.

Thanks for bringing this up!

No worries about credits, I am not here for that. Nor am I saying node-fetch-npm needs to stay in sync with upstream.

My suggestion is simply: because make-fetch-happen has caching support and says it wraps around node-fetch, people will likely use this package and not realise it's actually wrapping a fork of node-fetch.

I am opening this ticket as a precaution, because I started to see some developers making that assumption.

Open #45 as a suggestion.

zkat commented

#45 was merged. Closing this.