webpack-contrib / mini-css-extract-plugin

Lightweight CSS extraction plugin
MIT License
4.66k stars 389 forks source link

HMR: inferring script source from moduleId is not reliable and don't work with `[hash]`/`[fullhash]` #444

Open jsnajdr opened 5 years ago

jsnajdr commented 5 years ago

The HMR code needs to do some magic to convert the moduleId of the updated module to the URLs of the CSS chunks that contain the module. These URLs then need to be reloaded.

Trying to enable it in Calypso revealed at least three bugs:

Doesn't work with async-loaded chunks getCurrentScriptUrl relies on document.currentScript, which works when the chunk is loaded with script tag, but not when a chunk is loaded asynchronously. Then the module is evaluated in a Promise.then handler and document.currentScript is null. No hot reload.

Is not reliable with shared chunks With shared async chunks optimized by SplitChunksPlugin, it commonly happens that a module code is duplicated in multiple chunks. To reliably hot-reload their CSS, we need to reload all the CSS chunks where the module is bundled. That doesn't happen at this moment. Only the chunk that was used to actually execute the module is reloaded.

Fallback to "last script tag" almost never works When getCurrentScriptUrl can't find document.currentScript, it falls back to using the src attribute of the last script tag in the document. That works for very simple apps that load a single JS bundle, but fails in cases like:

alexander-akait commented 5 years ago

We recommend disable SplitChunksPlugin when you want to use HMR, it is very hard to implement

alexander-akait commented 5 years ago

And maybe won't fix

alexander-akait commented 5 years ago

Also please provide minimum reproducible test cases (some of them can be fixed like Fallback to "last script tag" almost never works)

jsnajdr commented 5 years ago

We recommend disable SplitChunksPlugin when you want to use HMR, it is very hard to implement

That still leaves two of three bugs: async loading and Promise.then, and unreliable last script tag.

Could you shed some light on why the "last script tag" code is there at all? It's very easy to break -- why not just look at document.currentScript and reject the hot update if it's null?

alexander-akait commented 5 years ago

Maybe it is bug, sometimes bugs happen, you can send a PR with fixes

jsnajdr commented 5 years ago

Pinging @ScriptedAlchemy who seems to be the original author of the HMR code and might know the rationale for the "last script tag" code.

alexander-akait commented 5 years ago

@jsnajdr anyway if you provide reproducible examples it help to use fix problem faster

jsnajdr commented 5 years ago

anyway if you provide reproducible examples it help to use fix problem faster

Last time I did that in #253, after a very similar conversation, it didn't help. I'd be very happy to spend my time creating and testing a patch though. Are there any suggestions on how to figure out the chunk IDs from module ID inside the webpack runtime? That would help implement the hot reloading reliably.

alexander-akait commented 5 years ago

@jsnajdr #253 will be solved in webpack@5, we can't do something in webpack@4, it is breaking change.

Are there any suggestions on how to figure out the chunk IDs from module ID inside the webpack runtime?

module.id

jsnajdr commented 5 years ago

module.id

Can you expand on that? The HMR code already knows the module ID. It needs to figure out the chunk URLs to reload.

alexander-akait commented 5 years ago

No information about chunk

Rulexec commented 4 years ago

Are there any suggestions on how to figure out the chunk IDs from module ID inside the webpack runtime? That would help implement the hot reloading reliably.

It is possible to collect moduleId => chunkIds map at compile time, pass it to runtime chunk and use from HMR.

alexander-akait commented 3 years ago

Other example:

git clone https://gist.github.com/andersk/2d45d4363478b22e998e177836ebce12 css-hmr-test
cd css-hmr-test
npm install
npx webpack serve --hot
alexander-akait commented 3 years ago

Also need test with [hash]/[fullhash]/[contenthash]

hyf0 commented 1 year ago

Does this means experiment: { css: true } would have this problem too?

hyf0 commented 1 year ago

I reproduce the problem in Webpack5 with experiment: { css: true }. I will dig into it to see how to fix this.