mini-css-extract-plugin icon indicating copy to clipboard operation
mini-css-extract-plugin copied to clipboard

HMR stops updating after first error, reports as unaccepted module

Open rjgotten opened this issue 5 years ago • 17 comments

  • Operating System: Windows 10
  • Node Version: 10.x
  • NPM Version: 9.6.0
  • webpack Version: 4.35.3
  • mini-css-extract-plugin Version: 0.8.0

Expected Behavior

Given is a Less file styles.less, in which we accidentally introduce an error and then save. HMR attempts an update and it shows the errored compilation.

After fixing the error, HMR should correctly load the fixed compiled CSS.

Actual Behavior

Instead of loading the fixed compiled CSS, HMR reports Ignored an update to unaccepted module ./styles.less and fails to update.

After having received a failed compilation, the module will continue to not accept hot updates until having manually reloaded the web page.

rjgotten avatar Jul 19 '19 15:07 rjgotten

Please create minimum reproducible test repo

alexander-akait avatar Jul 19 '19 15:07 alexander-akait

Please create minimum reproducible test repo

You can't. This relies on interactive behavior reloading in developer mode after a live edit in the .less source file.

Or do you actually want just the base webpack.config.js with just less-loader and a module that imports the style sheet?

rjgotten avatar Jul 19 '19 15:07 rjgotten

@rjgotten without reproducible test repo issue was closed because we can't help you with example

alexander-akait avatar Jul 19 '19 15:07 alexander-akait

Already figured out the problem.

MiniExtract use a pitching loader that starts a child compilation to grab a module produced by css-loader and then executes that module to obtain the CSS source.

If the .less file being compiled contains a syntax error it will cause that child compilation to fail.

Your loader will try/catch that, and will at that point propagate the error by passing it to its callback. That will lead Webpack itself to produce a module that has a throw statement in it containing the build error.

The whole problem then becomes that the hot module replacement will dispose the old registration, but the new module source that is incoming to replacing it, only throws and doesn't contain any of the hot module registration code needed for a new module.hot.accept()


Here's what I would propose to resolve that problem:

Inside the try-catch that guards the execution of the module yielded by the child complation, you should first check if hmr is enabled. If it is; then you should emit your own module source, mimicing the error module that Webpack would generate -- which is literally just throw new Error("<message>") -- but before the throw register a new instance of the hot loader.

That way, the hot module replacer will register for a future accept and the error becomes recoverable.

rjgotten avatar Jul 19 '19 15:07 rjgotten

Can you don't ignore my message about reproducible test repo? A lot of developers use less and HMR and no problem was reported, so i want to ensure problem in plugin not in your configuration so i ask you create simple reproducible test repo

alexander-akait avatar Jul 19 '19 15:07 alexander-akait

I know the plugin is to blame. Because I've just fixed it.

And yes; I'll work on a repro case. Two actually. One which also demonstrates the fixed plugin.

That'll be during office hours next week. I'm already an hour into overtime today.

rjgotten avatar Jul 19 '19 16:07 rjgotten

Glad to hear, don't forget we need add test for a PR, so reproducible test repo help me solve this fastly with test

alexander-akait avatar Jul 19 '19 16:07 alexander-akait

reproducible test repo

Ofcourse. Note though, as mentioned before: triggering the problem requires editing CSS/Less/Sass code while the webpack project is up and running with HMR. So an automated test repo demonstrating it will be kind of hard to pull off.

So I hope you'll settle for a pre-baked project along with some manual instructions to trigger the problem.


On a sidenote, something I really feel I have to say, having now stepped into the source-code for MiniCssExtract and coming to grips with the techniques with which it 'does its magic':

The ideas behind how it works and how it integrates its hot-reloading is nothing short of genius. Massive kudos for that.

rjgotten avatar Jul 19 '19 17:07 rjgotten

@evilebottnawi I think I may have run across one particular reason why this particular problem with HMR halting at the first error, may be kind of rare to occur.

webpack-dev-server is hard-wired to reload when it comes across an unaccepted module, which is the code path that is triggered once the hot.accept binding is dropped and not re-established for a stylesheet module that doesn't compile.

webpack-hot-middleware integrating with Express (but also with more black-boxy stuff like Microsoft's Webpack support for ASP.NET Core) has automatic full-reloading as an option, and defaults it to disabled. If you don't (or can't) enable it then you run into the particular problem described in this issue.

I have a repro-case of it all just about finished, along with a separate branch showing hardened HMR support in the loader part of MiniCssExtract, so that it can handle recovery even without a full reload. And yes; it passes unit tests and linting.

(Kind of cool really. One of the boons of this plugin using a child compiler is that such a flow specifically allows you to intercept the child compilation's errors and handle them yourself in a non-fatal way.)


Ah... and that reminds me: Apologies for allowing the (CR)LF linting issue to run out of hand earlier today. I think tension ran just a tad too high there. :smile:

rjgotten avatar Jul 22 '19 15:07 rjgotten

@rjgotten don't worry, please create reproducible test repo, it is help to me catch problem place and think about solution, hmr is not easy, maybe problem in webpack-hot-middleware, maybe not, need look on full setup, thanks

alexander-akait avatar Jul 22 '19 15:07 alexander-akait

Looks like Github is currently experiencing some difficulties. I have everything ready on my end, but it seems I'll have to wait until tomorrow to be able to push it.

Just letting you know.

rjgotten avatar Jul 22 '19 16:07 rjgotten

@rjgotten yep, feel free to ping me

alexander-akait avatar Jul 22 '19 16:07 alexander-akait

@evilebottnawi

The testcase is available now as mini-css-extract-plugin-hmr-testcase.

The readme contains a few manual instructions. (Sorry, couldn't get that to work automated.) It also contains some npm tasks to swap between the package version of mini-css-extract-plugin and the fork that I patched. And it contains a short rundown of what/how/why changes are applied to the forked plugin to make error-recovery/resilience work.

If you want I can also open a PR from the fork.

rjgotten avatar Jul 23 '19 09:07 rjgotten

Thanks for reproducible test repo, feel free to send a PR anytime

alexander-akait avatar Jul 23 '19 09:07 alexander-akait

@evilebottnawi

PR is also up, but there seems to be an issue with the CLA-signing. Bot gets confused when dealing with commits from separate emails which map to the same GitHub user, as far as I can see.

rjgotten avatar Jul 23 '19 10:07 rjgotten

Sorry for big delay, I will look at this tomorrow

alexander-akait avatar Nov 13 '20 17:11 alexander-akait

Sorry for big delay, I will look at this tomorrow

No problem. Stuff like this happens. Pretty sure that the like of Webpack 5 puts a lot of work on your plate.

rjgotten avatar Nov 13 '20 17:11 rjgotten