fix: use the native fetch API in browsers and a patched node-fetch otherwise

[steveluscher]

Problem

1. Presently, web3.js does not work in a Service Worker. cross-fetch is incompatible in that environment.
2. We're including a fetch ponyfill when in reality web3.js is already incompatible with browsers that don't support fetch natively.

Summary of Changes

• Augment the build system so that you can fork modules
• Fork the browser version of the fetch implementation to very simply be globalThis.fetch which works in browser and Service Worker environments.

Notes on the new fork system

The fork system (which should really have been in a separate PR) lets you override modules depending on the build environment. Given that you have a module called foo, the build system will check if there's a module at that directory level called __forks__/CONFIG/foo.ts. If there is, that will be built into the bundle.

src/
L __forks__/
  L browser/
    L foo.ts  <-- this will be used in browser builds
L foo.ts  <-- this will be used in non-browser builds
L utils/
  L __forks__/
    L node/
      L assert.ts  <-- this will be used in node builds
  L assert.ts  <-- this will be used in non-node builds

Test plan

Ran the test plan at https://github.com/wentokay/unfetch-test. Thanks @wentokay!

Noted that this saves 2.62K gzipped (tested in Solana Explorer build).

---

This diff was inspired by the phenomenal PR #25500.

[steveluscher]

globalThis will resolve to self in workers, and window in browsers. Both have fetch available on them.

[jstarry]

Aside.. @steveluscher what version of yarn do you use? I'm on 1.22.18 and I think we keep thrashing the lock file and creating huge diffs. Can we try to get on the same version?

[codecov]

Codecov Report

Merging #25576 (e552067) into master (f9f6b94) will decrease coverage by 6.6%.
The diff coverage is n/a.

❗ Current head e552067 differs from pull request most recent head e7ac7d3. Consider uploading reports for the commit e7ac7d3 to get more accurate results

@@             Coverage Diff             @@
##           master   #25576       +/-   ##
===========================================
- Coverage    82.0%    75.3%     -6.7%     
===========================================
  Files         655       40      -615     
  Lines      171822     2342   -169480     
  Branches      335      336        +1     
===========================================
- Hits       140972     1765   -139207     
+ Misses      30734      459    -30275     
- Partials      116      118        +2     

[steveluscher]

I'm on 1.22.17. 🤔

[jstarry]

Maybe it's dependabot then 🤷🏻

[steveluscher]

That little point version made all the difference! I just upgraded to 1.22.19 and now the diff is many times smaller.

[jstarry]

lgtm but have a few questions.. and I think the typing magic you used is cool but hard to understand and maintain. I'd prefer if things were simpler.

[jstarry]

when would the input be missing a protocol and just start with //?

[steveluscher]

This is a standard patch for the broken implementation of node-fetch that doesn't support protocol-relative URLs like the fetch in browsers does. https://github.com/node-fetch/node-fetch/blob/main/docs/v2-LIMITS.md

Folks can and will write apps that make use of protocol-relative URLs, and we don't want them to break in Node when they do.

This replicates what cross-fetch was already doing.

https://github.com/lquixada/cross-fetch/blob/24dedb4c8a16a33cb9b4d4682fb731438a6a9e2d/src/node-ponyfill.js#L4-L11

[jstarry]

nit: how about importing ./fetch-iml as fetchImpl and keeping this as fetch?

[jstarry]

I can't really make sense of this syntax.. what is being evaluated by the ? operator?

[steveluscher]

Fair enough. I was hoping the names of the utility types would make the reader incurious as to how it's implemented. These types should really just be TypeScript builtins, but alas.

I'll kill this stuff and just write the types manually.

[jstarry]

What is the benefit of expressing the type of FetchMiddleware like this over the original type signature which is more explicit?

[steveluscher]

Well, the original signature is just straight up wrong. The first parameter to fetch is RequestInfo not string. The second RequestInit not any. I'm just trying to make everything properly line up with the type of the fetch-impl shim.