fix: use wrapper to allow patching global fetch

[dwightjack]

🔗 Linked issue

#295

❓ Type of change

• 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
• 🐞 Bug fix (a non-breaking change that fixes an issue)
• 👌 Enhancement (improving an existing functionality like performance)
• ✨ New feature (a non-breaking change that adds functionality)
• 🧹 Chore (updates to the build process or auxiliary tools and libraries)
• ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

I was investigating the usage of MSW on Nuxt 3 and encountered #295. Looking at the source code, I think that a possible solution is to define the local fetch constant as getter instead of a reference to globalThils.fetch. In this way, 3rd party library can patch and overwrite the global object without us referencing an outdated function:

// Before
export const fetch = _globalThis.fetch

// After
export const fetch = (...args: Parameters<typeof globalThis.fetch>) => _globalThis.fetch(...args)

At the moment, I cannot think of possible breaking changes or side effects caused by this change, but I might miss something.

📝 Checklist

• I have linked an issue or discussion.
• I have updated the documentation accordingly.

[rinux55]

nuxt/test-utils#775 just mentioning this one here, thanks for taking this up!

[pi0]

Thanks for PR. I was thinking of same solution too. I agree it is needed but (sadly) means 1) we allow patching behavior of a standard primitive 2) adds small call overhead.

@dwightjack Have you tried this solution btw? I guess we might need to patch the node entrypoint as well. (also in createFetch we persist the fetch variable)

[dwightjack]

Thanks for PR. I was thinking of same solution too. I agree it is needed but (sadly) means 1) we allow patching behavior of a standard primitive 2) adds small call overhead.

1. Not a fan as well, but I guess it's the only way for them to intercept fetch calls
2. I am not sure how much this could impact the performances 🤔 For sure, it's an unneeded change for many use cases.

@dwightjack Have you tried this solution btw? I guess we might need to patch the node entrypoint as well. (also in createFetch we persist the fetch variable)

Oh, I missed that part. I updated the PR (4026829).

I wasn't able to make a full reproduction of Nuxt + MSW on StackBlitz (I guess there are some problems with service workers), so I pushed it to a GitHub repository: https://github.com/dwightjack/ofetch-msw-reproduction

Some notes:

1. The repository includes the packed version of ofetch built from this PR (I am not sure if there's a more elegant way to do this) and I defined an override for every instance of ofetch.
2. I implemented MSW in a Nuxt plugin (server + client) and a Nitro plugin (just in case)
3. Mocking an API call with useFetch seems to work for client-only calls, but fails for server calls
4. In the node-app folder I ported the small node application linked in the original issue using both the un-modified and the patched versions of ofetch. In that case, the patched version seems to work correctly, so I am not sure why it is not working on Nuxt. 🤔 Could it be related to also in createFetchwe persist thefetch variable?

[codecov]

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 67.08%. Comparing base (27996d3) to head (63afebd).
Report is 35 commits behind head on main.

Files	Patch %	Lines
src/index.ts	0.00%	3 Missing ⚠️
src/node.ts	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #377       +/-   ##
===========================================
+ Coverage   56.86%   67.08%   +10.22%     
===========================================
  Files          16       16               
  Lines         728      474      -254     
  Branches      113      116        +3     
===========================================
- Hits          414      318       -96     
+ Misses        303      146      -157     
+ Partials       11       10        -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[vanstinator]

I'm working to add some CF Pages support to otel-cf-workers and auto-instrumentation of fetch has no effect there for folks using ofetch, because by the time globalThis.fetch is patched by otel-cf-workers ofetch has already initialized and doesn't pick up on the new instrumented fetch object assigned to globalThis.fetch.

I think the work being done here would unblock this use-case too!

evanderkoogh/otel-cf-workers#96

[MuhammadM1998]

Related nuxt/nuxt#24519

[pi0]

Thanks ❤️