-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: fix live reload for child compiler scenario #3972
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3972 +/- ##
==========================================
- Coverage 92.58% 92.28% -0.30%
==========================================
Files 14 14
Lines 1362 1413 +51
Branches 474 521 +47
==========================================
+ Hits 1261 1304 +43
- Misses 93 101 +8
Partials 8 8
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is valid solution, if something changed in child compilations, hash of main compilation should be changed. Adding applyEntryOption
in processAssets
hooks is too late, can you show me use case for this?
What if child compilation depends on the chunk hash? For example service worker, SSR html page generation and so on. My use case is the child compilation will do something similar to DefinePlugin to replace the token with the chunk hash. The compilation is using a different entry and different target. I remember it was working before webpack-dev-server major update. |
You have this problem due invalid usage of hooks, because it is too late for this... |
If we merge we will always got reloading due changes in mini-css-extract-plugin and other plugins with child compilations, check it |
Thanks, Let me read it first |
Here are my use cases and requirements
if you are just talking about reload, I know how to do it. Again my child compiler needs parent compilation hash value result. How can I get the chunk finalized hash value before the seal? |
I am not sure if you are suggesting me I should use Currently, I have several javascript parser hooks at the
I also take a look how In my case, I cannot because of using javascript parser hook in the child compilation |
Again, if something was changed hash the main compilation should be changed, it is not only about live reloading, it is about caches and other mechanisms inside webpack, will break not only live reload, hot is broken, cache can be broken, emitting can be broken, please provide code of the plugin and I will show how to fix it |
You can configure loader inside plugin (and pass options to loaders too), we do the same in many places |
https://github.com/wood1986/demo. Here is my sample code.
|
Why you not emit files using webpack API? |
Firstly as you can see you use |
Secondary, try There are no problems with dev server, webpack just can't understand your changes |
Oh, I was wrong about The main idea - if something was changed in child compilation it should change the main hash, otherwise you will faced with a lot of bugs and problems with cached, not only dev server rely on |
Thanks!! Your review is very useful. Let me get back to you after 8 hours. |
Do you have sample code because I do not know how to use.
I do not know this is a hack. I remember there was a service work plugin library was doing this. And I also expect for all files which has
Are you saying I can explicitly update the parent compilation hash manually? When is the right stage to update hash? If I update at |
But you are have them inside plugin... https://github.com/wood1986/demo/blob/main/index.js#L111, just do it earlier
Yes, possible solutions:
|
It does not work. Take a look the commit this branch https://github.com/wood1986/demo/commit/389e5914cfb0b2845e5307fe28b3d6197926045c You can see the execute order of each hook. I cannot register the |
To be honest, I do not think it is fixable. To get it work properly, it will be either webpack add another stage in processAssets hooks with a hash parameter in the callback for people to update hash and or webpack-dev-server take my fix |
There is not problems with webpack here, you should manually update hash |
You can do it early, what is the problem do it? |
Again, it is invalid fix and break a lot of non official plugins |
Just create variable - |
Child compilation can change something in the main compilation and do nothing, so hash can be the same or can be changed, you should manually update hash when you change something. All these problems arise because you use hacky approach, ideally - you need multi compiler mode with https://webpack.js.org/configuration/other-options/#dependencies, so you can get web content of client and inject it in node target, but you chose the way through child compilation, it has some limitations where you have to solve the problem using low API and take care of hash/cache/dependecies/etc. I will look at this today/tomorrow (late) and show how to solve it using your approach. |
I have not tried. Where and how to configure devServer? I do not think I should put two devServer configs in each target. So if I need to put one devServer in either web or node target, I can foresee it will be no different from my plugin approach. I mean if you make a change to the entry js file, it will not have auto reload. Other than that, as the stats of each target are completely separated, disconnected and no relationship, how can node compilation get web compilation stats? You can say I can write the manifest to the disk and then let node compilation read this from the disk. Yes, it is doable but I do not like it. It is not dev-friendly because consumers have to specify to two different plugins for web and node target. I would say this is NO. FYI: I have tried these 3 approaches
parentCompilation.fullHash = parentCompilation.fullHash + childCompilation.fullHash
parentCompilation.hash = parentCompilation.fullHash
I have a feeling if you ask me to use some low api, you put the hacky code on my side. Even if you think it is not hacky, I still do not want to maintain these codes on my side. This PR does not break existing test cases. I strongly believe webpack-dev-server is missing this case because webpack's watch can re-compile on child entry change. I do not understand why you think this is not a valid solution. Meanwhile I thought webpack-dev-server relies on invalid hooks to trigger a reload. I want to know why it does not relies invalid hooks. I totally understand your expectation child compiler's change should update parent compiler hash. But it does not support. Although you keep saying I do it in a hacky way after processAssets stage, I do not think so because the final hash value is available at this stage based on this comment. https://github.com/webpack/webpack/blob/f0298fe46fc22eebf42eb034a9435d7c19aeddd9/lib/Compilation.js#L5180-L5184 |
Again, because a lot of plugins use child compilation, some of them just generate external files for other usages, we can't reload app only based on this, want to reload - update hash.
Is it? Then which one should he use? It was discussed a lot of time, API solves the problems of 99%. If you don't want to do something using the official approach and accordingly designed for this I do not know how to help you, unfortunately I cannot solve the problems of everyone and everything. And I don't want to break logic for many developers. If you do not believe me, I am very sad about this. If we had the opportunity to experiment (alpha/beta/rc), then I would show you how many issues will appear after this was merged. And eventually you would do revert. |
Anyway I will look at this deeply today (have small tasks) and help you |
Hey @alexander-akait, may I know if you are still looking at this issue? |
@wood1986 Yes, sorry for delay, tomorrow I will look, a little busy |
Thanks for letting me know!! FYI, I can confirm that it was working before webpack-dev-server 4.0.0 |
@wood1986 Can you provide example for v3? Try to set |
Here is https://github.com/wood1986/demo/tree/wds3 I have just found. It always has auto reload when it is a new change. However when you undo the previous saved change, it will not have auto reload. I cannot reproduce in own repository but I can reproduce it in this repository
|
I see, this related to #3794, due Generally we have two problems here:
|
Oh, I see running
Should fix the problem, feel free to feedback |
I did mention this fix See parentCompilation.fullHash = parentCompilation.fullHash + childCompilation.fullHash
parentCompilation.hash = parentCompilation.fullHash but it is not a native webpack hash. And do you think if other 3rd parties will have some logic related to this field Thanks for looking it up. |
@wood1986 It is valid solution, for multi compiler mode we do the same, just concat two hashes in string, we have this logic https://github.com/webpack/webpack-dev-server/blob/master/client-src/utils/reloadApp.js#L12, i.e.
Hash is just string and can have any length (types), so even a plugin will have logic for |
It works. Thanks!!! I am curious the multi compiler mode implementation. Is it possible to show me the which files have hash concat? |
@wood1986 All used modules, you can track it in |
@wood1986 Can we close? |
fix #3971
For Bugs and Features; did you add new tests?
Currently, webpack-dev-server only cares
stats.hash
but this hash does not count child compilation when you run the childCompiler after parent'sseal
stage.Motivation / Use-Case
Breaking Changes
Additional Info