feat: hmr error recovery

src/loader.js

@@ -16,26 +16,44 @@ const MODULE_TYPE = 'css/mini-extract';
 const pluginName = 'mini-css-extract-plugin';
 
 function hotLoader(content, context) {
-  const accept = context.locals
-    ? ''
-    : 'module.hot.accept(undefined, cssReload);';
+  const cssReload = loaderUtils.stringifyRequest(
+    context.context,
+    path.join(__dirname, 'hmr/hotModuleReplacement.js')
+  );
+
+  const cssReloadArgs = JSON.stringify({
+    ...context.options,
+    locals: !!context.locals,
+  });
+
+  // The module should *always* self-accept and have an error handler
+  // present to ensure a faulting module does not bubble further out.
+  // The error handler itself does not actually need to do anything.
+  //
+  // When there are no locals, then the module should also accept
+  // changes on an empty set of dependencies and execute the css
+  // reloader.
+  let accept = 'module.hot.accept(function(){});';
+  if (!context.locals) {
+    accept += '\n      module.hot.accept(undefined, cssReload);';
+  }
 
   return `${content}
     if(module.hot) {
       // ${Date.now()}
-      var cssReload = require(${loaderUtils.stringifyRequest(
-        context.context,
-        path.join(__dirname, 'hmr/hotModuleReplacement.js')
-      )})(module.id, ${JSON.stringify({
-    ...context.options,
-    locals: !!context.locals,
-  })});
+      var cssReload = require(${cssReload})(module.id, ${cssReloadArgs});
       module.hot.dispose(cssReload);
       ${accept}
     }
   `;
 }
 
+function interceptError(callback, interceptor) {
+  return (err, source) => {
+    return callback(null, err ? interceptor(err) : source);
+  };
+}
+
 const exec = (loaderContext, code, filename) => {
   const module = new NativeModule(filename, loaderContext);
 
@@ -133,17 +151,24 @@ export function pitch(request) {
     });
   });
 
-  const callback = this.async();
+  const callback = !options.hmr
+    ? this.async()
+    : interceptError(this.async(), (err) => {
+        let resultSource = `// extracted by ${pluginName}`;
+        resultSource += hotLoader('', {
+          context: this.context,
+          locals: null,
+          options,
+        });
+        resultSource += `\nthrow new Error(${JSON.stringify(String(err))});`;
+        return resultSource;
+      });
 
   childCompiler.runAsChild((err, entries, compilation) => {
     if (err) {
       return callback(err);
     }
 
-    if (compilation.errors.length > 0) {
-      return callback(compilation.errors[0]);
-    }
-
     compilation.fileDependencies.forEach((dep) => {
       this.addDependency(dep);
     }, this);
@@ -152,6 +177,10 @@ export function pitch(request) {
       this.addContextDependency(dep);
     }, this);
 
+    if (compilation.errors.length > 0) {
+      return callback(compilation.errors[0]);
+    }
+
     if (!source) {
       return callback(new Error("Didn't get a result from child compiler"));
     }

test/TestCases.test.js

@@ -29,8 +29,46 @@ describe('TestCases', () => {
   const casesDirectory = path.resolve(__dirname, 'cases');
   const outputDirectory = path.resolve(__dirname, 'js');
 
+  // The ~hmr-resilience testcase has a few variable components in its
+  // output. To resolve these and make the output predictible and comparable:
+  // - it needs Date.now to be mocked to a constant value.
+  // - it needs JSON.stringify to be mocked to strip source location path
+  //   out of a stringified error.
+  let dateNowMock = null;
+  let jsonStringifyMock = null;
+  beforeEach(() => {
+    dateNowMock = jest
+      .spyOn(Date, 'now')
+      .mockImplementation(() => 1479427200000);
+
+    const stringify = JSON.stringify.bind(JSON);
+    jsonStringifyMock = jest
+      .spyOn(JSON, 'stringify')
+      .mockImplementation((value) => {
+        // ~hmr-resilience testcase. Need to erase stack trace location,
+        // which varies by system and cannot be compared.
+        if (typeof value === 'string' && value.includes('error-loader.js')) {
+          return stringify(
+            value.replace(
+              /\([^(]+error-loader\.js:\d+:\d+\)$/,
+              '(error-loader.js:1:1)'
+            )
+          );
+        }
+
+        return stringify(value);
+      });
+  });
+
+  afterEach(() => {
+    dateNowMock.mockRestore();
+    jsonStringifyMock.mockRestore();
+  });
+
   for (const directory of fs.readdirSync(casesDirectory)) {
     if (!/^(\.|_)/.test(directory)) {
+      const expectsError = /-fail$/.test(directory);
+
       // eslint-disable-next-line no-loop-func
       it(`${directory} should compile to the expected result`, (done) => {
         const directoryForCase = path.resolve(casesDirectory, directory);
@@ -59,7 +97,7 @@ describe('TestCases', () => {
         }
 
         webpack(webpackConfig, (err, stats) => {
-          if (err) {
+          if (err && !expectsError) {
             done(err);
             return;
           }
@@ -76,7 +114,7 @@ describe('TestCases', () => {
             })
           );
 
-          if (stats.hasErrors()) {
+          if (stats.hasErrors() && !expectsError) {
             done(
               new Error(
                 stats.toString({

test/cases/hmr-resilience-fail/error-loader.js

@@ -0,0 +1,4 @@
+module.exports = function loader() {
+  const callback = this.async();
+  callback(new Error('I am error'));
+};

test/cases/hmr-resilience-fail/expected/check.js

@@ -0,0 +1,19 @@
+(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["check"],{
+
+/***/ "./index.css":
+/***/ (function(module, exports, __webpack_require__) {
+
+// extracted by mini-css-extract-plugin
+    if(true) {
+      // 1479427200000
+      var cssReload = __webpack_require__("../../../src/hmr/hotModuleReplacement.js")(module.i, {"hmr":true,"locals":false});
+      module.hot.dispose(cssReload);
+      module.hot.accept(function(){});
+      module.hot.accept(undefined, cssReload);
+    }
+
+throw new Error("ModuleBuildError: Module build failed (from ./error-loader.js):\nError: I am error\n    at Object.loader (error-loader.js:1:1)");
+
+/***/ })
+
+}]);

test/cases/hmr-resilience-fail/index.css

@@ -0,0 +1,3 @@
+.a {
+  background: red;
+}

test/cases/hmr-resilience-fail/webpack.config.js

@@ -0,0 +1,64 @@
+import { HotModuleReplacementPlugin } from 'webpack';
+
+import Self from '../../../src';
+
+module.exports = {
+  entry: './index.css',
+  mode: 'development',
+  devtool: false,
+  // NOTE:
+  // Using optimization settings to shunt everything
+  // except the generated module code itself into
+  // discarded chunks that won't be compared for
+  // expected output.
+  optimization: {
+    runtimeChunk: 'single',
+    namedModules: true,
+    namedChunks: true,
+    splitChunks: {
+      chunks: 'all',
+      cacheGroups: {
+        vendors: {
+          test: /[\\/]index\.css$/,
+          name: 'check',
+          enforce: true,
+        },
+      },
+    },
+  },
+  module: {
+    rules: [
+      {
+        test: /\.css$/,
+        use: [
+          {
+            loader: Self.loader,
+            options: {
+              hmr: true,
+            },
+          },
+          require.resolve('./error-loader'),
+        ],
+      },
+    ],
+  },
+  plugins: [
+    new HotModuleReplacementPlugin(),
+    new Self({
+      filename: '[name].css',
+    }),
+    {
+      apply(compiler) {
+        compiler.hooks.emit.tapAsync('no-emit', (compilation, callback) => {
+          const { assets } = compilation;
+
+          // Not interested in comparing output for these.
+          delete assets['runtime.js'];
+          delete assets['main.js'];
+
+          callback();
+        });
+      },
+    },
+  ],
+};