node-fetch should provide a polyfill entrypoint

[tarikjn]

This has been an issue that has came back to me several times, but the fact that v3 is ESM-only module makes this a pain point with node-fetch. Read more bellow:

When is this an issue?

• using a library that expects fetch available as a global
• working with an isomorphic codebase
• you want to run a module/some code originally designed for the browser in your node codebase

Why are the existing solutions inappropriate?

The doc suggests doing this:

import fetch from 'node-fetch';

if (!globalThis.fetch) {
  globalThis.fetch = fetch;
}

But this is actually problematic with ESM modules. If you use a library expecting fetch as a global (eg. react-relay-network-modern). There are typically three locations where you would import node-fetch:

1. right before you import the library (unless you also run that code in the browser)
2. in your server entry point
3. in your bundler entry point

In all three cases using ES imports the above code won't be guaranteed to have assigned fetch on the global object before something is done with it, and so the only way to address that is to use an import file specifically for polyfilling.

Solutions

The correct solution is to provide a polyfill entrypoint that can be used for this purpose:

import 'node-fetch/polyfill';

This can either be implemented on this project or with an external package. An external package would either include node-fetch as a peer dependency, requiring the user to use 2 packages, or worse, it may be using node-fetch as a dependency.

Either way, this is an issue that will keep coming back until a proper solution is documented. Perhaps a middle-ground solution is to link to a module that properly polyfill node-fetch in the documentation.

Prior art and discussion

#119 (by yours truly)
#521

Why does this belongs on this package?

1. This is a polyfill for node, not for any other environment. I agree with @bitinn that providing a browser.js file was out of scope:

   On Polyfill self #537

   Thx for the PR but I believe this is better left for third-party module, where the author knows exactly what they want.

   (we made mistake providing a browser.js and we regret it since, as people use it differently and sometimes in non-nodejs environment like react-native.)

2. There is pretty much only one way to do it, Adds polyfill support #119 and Provide global polyfills #521 both use the same approach.

Why is it an even bigger issue today than 5 years ago

• node-fetch v3 being an ESM-only module, it will often require creating a separate file to properly polyfill it
• browser code is now fairly safe to use fetch without any client polyfills and so it can be more frequently expected to rely on a global fetch

[jimmywarting]

I'm on the fence. Would like hear someones else opinion. (mostly from the pioner at @node-fetch/core team, but also from everyone else)

[gr2m]

I think we discussed this in the past in other issues?

With ESM and package.json we now have conditional exports, so we can export different code for different environments such as Node/Deno/Browsers/etc. As far as I know the tooling support for it is pretty good, and as it's now standardized in Node land, support for it will only grow.

I would suggest we move this into a separate package such as @node-fetch/universal or similar which only exports the fetch method where it's already available and node-fetch for Node

[tarikjn]

@gr2m this is unrelated to conditional exports though.

You would use conditional exports where you want an import present in multiple environments -- which on that subject, is arguably no a great approach IMO, as you may want to control how things are imported in userland, to e.g. rely on things like your browserlist.

Polyfilling as discussed here on the other hand is exclusively about exposing fetch in the global node environment. You only import it on a code path that you know is only used by node.

It can be a bit confusing because for example isomorphic-fetch does both of these. Perhaps polyfill is not the best term to use.

[gr2m]

Got it, sorry. Nevermind my comment, we can hide it as off topic.

I don't have an opinion on your use case but I agree that it will come up. The node-fetch/polyfill approach sounds good to me, it's simple enough to add and maintain to keep it in the same package, but others might have thought more about it so let's get some more opinions on the matter

[Richienb]

Can't it be done like this?

globalThis.fetch ||= (await import('node-fetch')).default

[jimmywarting]

what about Header, Request & Response?

[Richienb]

Can't it be done like this?

// lazy conditional import

if (!globalThis.fetch) {
  const {default: fetch, Headers, Request, Response} = await import('node-fetch');

  globalThis.fetch ||= fetch;
  globalThis.Headers ||= Headers;
  globalThis.Request ||= Request;
  globalThis.Response ||= Response;
}

[Richienb]

@jimmywarting By nesting it in an if statement, are we confident that the subclasses won't be missed out?

[jimmywarting]

don't know, it's a bit unsafe... but it would also be weird if they where not b/c they go so hand in hand

In #1314 i wrote about exposing FormData, File and Blob as well...
it's a bit annoying to install same dependencies also when node-fetch already has is and we could just as well use the same version to avoid dedup

[tarikjn]

@jimmywarting I may be wrong on this, but I think this may not need to go beyond fetch/Headers/Request/Response. In practice, if format objects or other things are used and need to be 'globaled' as well then they should probably be declared as separate dependencies?

I don't know of any lib that rely on fetch that also need those accessible globally, and if a path in your codebase does, then you would explicitly import, or polyfill those as required. if the same versions are used, the bundler will optimize bundle size. Besides, some of these things are making their way into node native. Perhaps this goes more into peerDependencies strategy land.

If it does come up as a real use case where we find it's actually needed, then maybe you have different level of polyfilling and you always use the lowest one needed? e.g.

import 'node-fetch/polyfill-level0' // => fetch
import 'node-fetch/polyfill-level1' // => fetch, Headers, Request, Response
import 'node-fetch/polyfill-level2' // => fetch, Headers, Request, Response, FormData, File, Blob

As for the import itself, I would make it static, the polyfill would typically be called just once, just above in the code tree before importing an isomorphic code path or the first lib that requires it, or in your build entry point. It's not meant to be used outside of node, so when you call it, nominally globalThis.fetch should not be defined. It could/should probably alert if the global was already defined. This follows the babel polyfill pattern. And if you are writing some node code that uses fetch, you should still use an explicit dependency and let the module system sort it out.

[goya]

https://github.com/node-fetch/node-fetch/blob/v2.6.6/browser.js

[goya]

it was removed in v3

[tarikjn]

@goya so the feature that you linked (browser.js), is unrelated, it's a conditional export to use node-fetch in the browser. which was a bad idea to start with IMO. node-fetch as it names indicates should only be used in the node environment.

On the other hand, offering an option to expose fetch on the global node environment to replicate the browser API should had been offered all along as an option vs. documented as code to copy-paste, as the method of doing so depend too much on node-fetch implementation and has changed with different releases.

This is a fairly simple feature, but I think the confusion around it shows how important it is to get that stuff right, especially for downstream packages that depend on fetch to not need to assume anything about their environment.

[goya]

yes, we used node-fetch in our sdk as it allowed our sdk users to package it for the browser or nodejs.

but we're happy to stick to < 3