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

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

Open
jsnajdr opened this issue Sep 3, 2019 · 16 comments · Fixed by #633

Comments

@jsnajdr
Copy link

jsnajdr commented Sep 3, 2019

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:

  • there are shared chunks that together form a chunk group. There's not reason why the last chunk should contain the updated moduleId
  • the last script tag is not owned by webpack, but loads something else. For example, analytics from a 3rd party source
  • the last script doesn't have a src attribute, but is an inline script. Then the HMR code will crash with TypeError: Cannot read property 'some' of null
@alexander-akait
Copy link
Member

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

@alexander-akait
Copy link
Member

And maybe won't fix

@alexander-akait
Copy link
Member

alexander-akait commented Sep 3, 2019

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

@jsnajdr
Copy link
Author

jsnajdr commented Sep 3, 2019

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
Copy link
Member

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

@jsnajdr
Copy link
Author

jsnajdr commented Sep 3, 2019

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
Copy link
Member

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

@jsnajdr
Copy link
Author

jsnajdr commented Sep 3, 2019

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
Copy link
Member

@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
Copy link
Author

jsnajdr commented Sep 3, 2019

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
Copy link
Member

No information about chunk

Rulexec added a commit to Rulexec/mini-css-extract-plugin that referenced this issue May 22, 2020
Rulexec added a commit to Rulexec/mini-css-extract-plugin that referenced this issue May 22, 2020
@Rulexec
Copy link

Rulexec commented May 22, 2020

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
Copy link
Member

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
Copy link
Member

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

@alexander-akait alexander-akait changed the title HMR: inferring script source from moduleId is not reliable HMR: inferring script source from moduleId is not reliable and don't work with [hash]/[fullhash] Apr 28, 2021
@hyf0
Copy link

hyf0 commented May 4, 2023

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

@hyf0
Copy link

hyf0 commented Jun 28, 2023

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

akambetov added a commit to akambetov/excel-course that referenced this issue Aug 13, 2023
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 a pull request may close this issue.

4 participants