Skip to content

fix(node): allow fetch override on runtime plugin - #2603

Merged
ScriptedAlchemy merged 14 commits into
mainfrom
fix/allow-node-runtime-plugin-override
Jun 27, 2024
Merged

ScriptedAlchemy merged 14 commits into
mainfrom
fix/allow-node-runtime-plugin-override

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Jun 11, 2024 •

Copy link
Copy Markdown
Member

Description

Allow internal fetch method to be overriden

Related Issue

fixes: #2534

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation.

@changeset-bot

changeset-bot Bot commented Jun 11, 2024 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 71b0887

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 26 packages
Name Type
@module-federation/sdk Patch
@module-federation/node Minor
@module-federation/runtime Patch
@module-federation/devtools Patch
@module-federation/dts-plugin Patch
@module-federation/enhanced Patch
@module-federation/managers Patch
@module-federation/manifest Patch
@module-federation/nextjs-mf Patch
@module-federation/rspack Patch
@module-federation/utilities Patch
@module-federation/webpack-bundler-runtime Patch
@module-federation/bridge-react-webpack-plugin Patch
@module-federation/runtime-tools Patch
3008-runtime-remote Patch
host Patch
host-v5 Patch
host-vue3 Patch
remote1 Patch
remote2 Patch
remote3 Patch
@module-federation/modernjs Patch
@module-federation/third-party-dts-extractor Patch
@module-federation/bridge-react Patch
@module-federation/bridge-vue3 Patch
@module-federation/bridge-shared Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify

netlify Bot commented Jun 11, 2024 •

Copy link
Copy Markdown

✅ Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 71b0887
🔍 Latest deploy log https://app.netlify.com/sites/module-federation-docs/deploys/667d0a25f7c5030008ac3326
😎 Deploy Preview https://deploy-preview-2603--module-federation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment thread packages/node/src/runtimePlugin.ts Outdated
return callback(null, emptyChunk);
}
fetch(url)
const fetchFunction = globalThis.webpackChunkLoad || fetch;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch: new AsyncHook<

Maybe we can set fetch through fetch hook instead of using global variables

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhoushaw do you have a sample of how to use the fetch hook?
I do not think it is bound to script loader, only json manifest loader

This provides backward compat for user, but i am happy to look at better solution for future and deprecate this usage

@quantum-1986 quantum-1986 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! The fix works: #2534 (comment) 👏

Comment thread packages/node/src/runtimePlugin.ts Outdated
Comment thread packages/node/src/runtimePlugin.ts Outdated
@ScriptedAlchemy

Copy link
Copy Markdown
Member Author

@quantum-1986 - im working on integrating the chunk load cycle on the server with our runtime apis, so you can now do the following:

You can do this as a hook from the runtime plugins docs.

export default function (): FederationRuntimePlugin {
  return {
    name: 'node-federation-plugin',
    fetch(url, init) {
      console.log('HOOK', url);

      return fetch(url, init);
    },
    beforeInit(args) {
      
      //patch webpack chunk loading handlers etc (this is what i do) 
    ```

@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/allow-node-runtime-plugin-override branch from db0f648 to 3ed5ffb Compare June 27, 2024 04:21
@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/allow-node-runtime-plugin-override branch from 3ed5ffb to b121e85 Compare June 27, 2024 04:22
@2heal1

2heal1 commented Jun 27, 2024

Copy link
Copy Markdown
Member

I don't know how we get fetch now from this pr , because i see you will use __webpack_require__.federation.instance.loaderHook.lifecycle.fetch.emit , it will return undefined in the node-host demo

image

@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/allow-node-runtime-plugin-override branch from fb9f6f5 to 433985c Compare June 27, 2024 06:31
@ScriptedAlchemy

Copy link
Copy Markdown
Member Author

I don't know how we get fetch now from this pr , because i see you will use __webpack_require__.federation.instance.loaderHook.lifecycle.fetch.emit , it will return undefined in the node-host demo

image

yeah im working on a fix

@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/allow-node-runtime-plugin-override branch from 433985c to 48625ef Compare June 27, 2024 06:33
Comment thread packages/sdk/src/node.ts Outdated

// @ts-ignore
const hook = (url: RequestInfo | URL, init: RequestInit) => {
debugger; // Add debugger for hook

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep it ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol sorry!

@ScriptedAlchemy
ScriptedAlchemy force-pushed the fix/allow-node-runtime-plugin-override branch from 48625ef to 71b0887 Compare June 27, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to Override Default HTTP Chunk Fetch with globalThis.webpackChunkLoad

5 participants