Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/do not inject runtime into build time chunk(#3225) #3229

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lnlfps
Copy link

@lnlfps lnlfps commented Nov 15, 2024

Description

When working with umijs or father, during the compilation of less modules, some 'build time chunks' are generated. These temporary chunks lack chunkGraph information, leading to errors when generating runtime code.

// enhanced/scr/lib/container/runtime/FederationRuntime.ts 49  
  
const conditionMap = this.compilation.chunkGraph.getChunkConditionMap(  
  this.chunk,  chunkHasJs,);  
  

Then The current chunk id is build time chunk, the variable this.compilation.chunkGraph is undefined,causing an exception to be thrown

Related Issue

#3225

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.

Copy link

netlify bot commented Nov 15, 2024

Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 07005c8
🔍 Latest deploy log https://app.netlify.com/sites/module-federation-docs/deploys/673b2351cceb520008bea34f
😎 Deploy Preview https://deploy-preview-3229--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.

@lnlfps lnlfps force-pushed the fix/do-not-inject-runtime-into-build-time-chunk(#3225) branch from fbb4034 to 16758b5 Compare November 15, 2024 06:09
@lnlfps
Copy link
Author

lnlfps commented Nov 15, 2024

The merge operation for this pull request has been blocked. This might be because I don't have the necessary permissions to perform the merge, or perhaps my workflow was incorrect. Could you please help me how to complete this merge?
thanks!

@ScriptedAlchemy
Copy link
Member

ScriptedAlchemy commented Nov 15, 2024

@lnlfps are you a bytedance employee?

If yes, I'll expedite the merge or ship a canary

If no, I'll review it in the morning and prepare release if tests pass.

@lnlfps
Copy link
Author

lnlfps commented Nov 15, 2024

@ScriptedAlchemy I am not an employee of ByteDance. Thanks very much for your response and assistance. I am planning to implement module federation in our UmiJS project.

I see that there are a few of test cases failing. The reasons for the failures are exactly the issues I am currently encountering.

image

When processing a style modules, the base mf runtime was not injected into the chunk context, but the share runtime was loaded, resulting in an undefined error.

My method might not be complete, and I hope you have better solutions to address similar problems.

@ScriptedAlchemy
Copy link
Member

@lnlfps please see CI errors

@lnlfps lnlfps force-pushed the fix/do-not-inject-runtime-into-build-time-chunk(#3225) branch from 0953476 to 07005c8 Compare November 18, 2024 11:21
Copy link

changeset-bot bot commented Nov 18, 2024

⚠️ No Changeset found

Latest commit: 07005c8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

@lnlfps
Copy link
Author

lnlfps commented Nov 18, 2024

I tried to find the cause, which might be an issue when using the mini-css-extract-plugin Webpack plugin to load LESS files. It fails during the compilation of loading modules when importing the federation runtime module, at this time the chunkGraph is undefined, leading to a failure in code generation.

Below is the debugging screenshot for the exception.

image

Here, an undefined error occurs, causing the federation runtime to fail to generate code correctly.

image

I have resubmitted the code, adding only the undefined check. It runs correctly on my local machine and fixes the issue I encountered previously. Please review the code, thank you very much. @ScriptedAlchemy

@ScriptedAlchemy
Copy link
Member

@lnlfps please add a new build test case under test/configCases in enhanced package so that in future we will have a build test for this kind of issue

@ScriptedAlchemy
Copy link
Member

@lnlfps please add test cases

@lnlfps
Copy link
Author

lnlfps commented Nov 27, 2024

@ScriptedAlchemy hi, I've been a bit busy.
I delved deeper into the cause of the issue. It seems that another runtime plugin of mine added the RuntimeGlobals.ensureChunkHandlers runtime dependency to the current build chunk before FederationRuntime, which caused this problem.
I delayed the registration of my plugin, which temporarily resolved the issue.

If need me to provide test cases to recurrence this issue, i will do it later.
If not, I will close this pull request.

@ScriptedAlchemy
Copy link
Member

Gotcha. You can use the afterPlugins hook if you want to tap others later. Just ammend your plugin. We do this in some of federation as well i think. That might work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants