lquixada / cross-fetch

Universal WHATWG Fetch API for Node, Browsers and React Native.
MIT License
1.66k stars 102 forks source link

Document Enhancement Suggestion: No Experimental Native Fetch in Node, Not an Issue Per Se #155

Closed lgants closed 1 year ago

lgants commented 1 year ago

I recently migrated from isomorphic-unfetch to cross-fetch because following my upgrade to Node v18, which introduced a native fetch API, the isomorphic-unfetch library (v3.1.0) began dispatching server requests via the new/experimental fetch API. Unfortunately, that can be very problematic (as I discovered) because: 1) The Node runtime message clearly states: ExperimentalWarning: The Fetch API is an experimental feature. This feature could change at any time 2) The fetch implementation uses undici, which doesn't document certain forbidden headers - see GitHub issue here 3) Even if the forbidden headers were documented, certain state management libraries automatically inject (potentially forbidden) headers. That includes mine, which injects Connection and cannot be overrriden

So, this is just a suggestion to document that additional benefit of cross-fetch and request not to change that behavior

Thanks for maintaining the library and supporting open-source!

joebowbeer commented 1 year ago

@lgants Thanks for the heads up.

I just ran across an issue with the native fetch that was reported in the underlying node-fetch package:

https://github.com/node-fetch/node-fetch/issues/1566

The workaround was to explicitly disable native fetch via --no-experimental-fetch

It would be great to identify if/when cross-fetch needs this flag. Do you think polyfill is more likely than ponyfill to need this flag?

By my reading of node-polyfill.js, the native fetch will be used instead of node-fetch.

lquixada commented 1 year ago

@lgants thanks for taking the time to write this report. Currently, there is no plan to detect native node fetch implementation but I'll keep these issues in mind if something changes. Closing this for now.