webpack
webpack copied to clipboard
webpack
fix: Single Runtime Chunk and Federation eager module hoisting.
Hoists Eager Referenced modules into the runtime chunks
https://github.com/webpack/webpack/issues/18810
What kind of change does this PR introduce?
bugfix Did you add tests for your changes? no
Does this PR introduce a breaking change? no
What needs to be documented once your changes are merged? Nothing
For maintainers only:
- [ ] This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
- [ ] This needs to be backported to webpack 4 (issue will be created when merged)
@ScriptedAlchemy Let's fix test, looks like a typo
Let's also add a couple of tests to show that we've solved the problem
Sure. This case is believe this plugin will only solve some aspects. I'll put test and see if it can pass. But ideally I want to get the parts approved first then send a pr that utilizes them. Since there's several files that need to be updated to bring some features back. Initially I wanted review on it. Which is why I don't automatically apply the plugin yet 😅
@alexander-akait the tests fail saying getComplationHooks is not a function but it is static method on the plugin class. This fails the basictest configCases and crashed the build, can you help me understand why.
@ScriptedAlchemy Fixed your problem, but it looks like our tests are failed and your test doesn't work either, we need to fix it, I think you can do it faster than me, since you understand the logic better
To avoid:
ConfigTestCases › chunk-graph › rewalk-chunk › rewalk-chunk should compile
just make rebase to main
@ScriptedAlchemy Fixed your problem, but it looks like our tests are failed and your test doesn't work either, we need to fix it, I think you can do it faster than me, since you understand the logic better
Yeah, no problem. The main blocker that I had was, I could not import the plug in to even start looking at the test. Now that that is resolved, I will actually get the test working and create something that confirms it works
@alexander-akait the plugin is now hoisting modules correctly as you can see in the runtime chunk, it is now not empty.
But i get a test runtime error, why?
Okay i found the problem i have updated the test.
There is an issue / race condition with how these test work. Since the "host" app consumes its own remoteEntry - when runtiemChunk: single is set in ESM mode, the runtime ends up externally importing ./container.mjs -> import webpack runtime. So it fails in this case, however this pattern is hardly ever used as people are not consuming their own containers, its usually another build who is.
I have added a test case to check the modules in the module cache, when there is 2 entrypoints - the external container modules will fail to init that are in the entry who is not loaded and the cache will only include the modules who happen to be loaded at startup, with the hoist plugin enabled the warning goes away and the module cache shows all remote modules regardless of which entry uses them
@ScriptedAlchemy We still have two problems:
chunk-graph › rewalk-chunk › rewalk-chunk should compile(let's rebase)
This looks like a slight regression, can you look at it and understand why it happened:
container › reference-hoisting › exported tests › should have the hoisted container references- container › reference-hoisting › exported tests › should have the hoisted container reference`
@alexander-akait i found the problem, walk chunk test has been corrected.
It seems somehow, i removed the application of the plugin. I have re-applied it again and now the hoist ref test case passes too
@ScriptedAlchemy I think we can go further, we should add documentation, but we can do it later, when we finish the work, what do you think about it?
@ScriptedAlchemy I think we can go further, we should add documentation, but we can do it later, when we finish the work, what do you think about it?
Yeah when we finish the work. Then we can document. Some PR have no need for doc or are just mechanical