Merge pull request #5506 from webpack/bugfix/concat-hmr

Disallow Scope Hoisting when using HMR
webpack · Aug 12, 2017 · 2475c6a · 2475c6a
2 parents 287d587 + 0ea37a5
commit 2475c6a
Show file tree

Hide file tree

Showing 7 changed files with 78 additions and 29 deletions.
diff --git a/lib/optimize/ModuleConcatenationPlugin.js b/lib/optimize/ModuleConcatenationPlugin.js
@@ -5,6 +5,8 @@
 "use strict";
 
 const HarmonyImportDependency = require("../dependencies/HarmonyImportDependency");
+const ModuleHotAcceptDependency = require("../dependencies/ModuleHotAcceptDependency");
+const ModuleHotDeclineDependency = require("../dependencies/ModuleHotDeclineDependency");
 const ConcatenatedModule = require("./ConcatenatedModule");
 const HarmonyExportImportedSpecifierDependency = require("../dependencies/HarmonyExportImportedSpecifierDependency");
 const HarmonyCompatibilityDependency = require("../dependencies/HarmonyCompatibilityDependency");
@@ -67,6 +69,12 @@ class ModuleConcatenationPlugin {
 						continue;
 					}
 
+					// Hot Module Replacement need it's own module to work correctly
+					if(module.dependencies.some(dep => dep instanceof ModuleHotAcceptDependency || dep instanceof ModuleHotDeclineDependency)) {
+						setBailoutReason(module, "Module uses Hot Module Replacement");
+						continue;
+					}
+
 					relevantModules.push(module);
 
 					// Module must not be the entry points

diff --git a/test/HotTestCases.test.js b/test/HotTestCases.test.js
@@ -37,35 +37,38 @@ describe("HotTestCases", () => {
 							updateIndex: 0
 						}
 					};
-					const options = {
-						context: testDirectory,
-						entry: "./index.js",
-						output: {
-							path: outputDirectory,
-							filename: "bundle.js"
-						},
-						module: {
-							loaders: [{
-								test: /\.js$/,
-								loader: path.join(__dirname, "hotCases", "fake-update-loader.js"),
-								enforce: "pre"
-							}, {
-								test: /\.css$/,
-								use: ExtractTextPlugin.extract({
-									fallback: "style-loader",
-									use: "css-loader"
-								})
-							}]
-						},
-						target: "async-node",
-						plugins: [
-							new webpack.HotModuleReplacementPlugin(),
-							new webpack.NamedModulesPlugin(),
-							new webpack.LoaderOptionsPlugin(fakeUpdateLoaderOptions),
-							new ExtractTextPlugin("bundle.css")
-						],
-						recordsPath: recordsPath
-					};
+					const configPath = path.join(testDirectory, "webpack.config.js");
+					let options = {};
+					if(fs.existsSync(configPath))
+						options = require(configPath);
+					if(!options.context) options.context = testDirectory;
+					if(!options.entry) options.entry = "./index.js";
+					if(!options.output) options.output = {};
+					if(!options.output.path) options.output.path = outputDirectory;
+					if(!options.output.filename) options.output.filename = "bundle.js";
+					if(options.output.pathinfo === undefined) options.output.pathinfo = true;
+					if(!options.module) options.module = {};
+					if(!options.module.rules) options.module.rules = [];
+					options.module.rules.push({
+						test: /\.js$/,
+						loader: path.join(__dirname, "hotCases", "fake-update-loader.js"),
+						enforce: "pre"
+					}, {
+						test: /\.css$/,
+						use: ExtractTextPlugin.extract({
+							fallback: "style-loader",
+							use: "css-loader"
+						})
+					});
+					if(!options.target) options.target = "async-node";
+					if(!options.plugins) options.plugins = [];
+					options.plugins.push(
+						new webpack.HotModuleReplacementPlugin(),
+						new webpack.NamedModulesPlugin(),
+						new webpack.LoaderOptionsPlugin(fakeUpdateLoaderOptions),
+						new ExtractTextPlugin("bundle.css")
+					);
+					if(!options.recordsPath) options.recordsPath = recordsPath;
 					const compiler = webpack(options);
 					compiler.run((err, stats) => {
 						if(err) return done(err);

diff --git a/test/hotCases/concat/reload-external/a.js b/test/hotCases/concat/reload-external/a.js
@@ -0,0 +1,6 @@
+export default 1;
+---
+export default 2;
+---
+export default 2;
+---
diff --git a/test/hotCases/concat/reload-external/b.js b/test/hotCases/concat/reload-external/b.js
@@ -0,0 +1,6 @@
+export default 10;
+---
+export default 10;
+---
+export default 20;
+---
diff --git a/test/hotCases/concat/reload-external/index.js b/test/hotCases/concat/reload-external/index.js
@@ -0,0 +1 @@
+import "./module";
diff --git a/test/hotCases/concat/reload-external/module.js b/test/hotCases/concat/reload-external/module.js
@@ -0,0 +1,16 @@
+import value1 from "./a";
+import value2 from "./b";
+
+it("should allow to hot replace modules in a ConcatenatedModule", function(done) {
+	value1.should.be.eql(1);
+	value2.should.be.eql(10);
+	module.hot.accept("./a", function() {
+		value1.should.be.eql(2);
+		NEXT(require("../../update")(done));
+	});
+	module.hot.accept("./b", function() {
+		value2.should.be.eql(20);
+		done();
+	});
+	NEXT(require("../../update")(done));
+});
diff --git a/test/hotCases/concat/reload-external/webpack.config.js b/test/hotCases/concat/reload-external/webpack.config.js
@@ -0,0 +1,9 @@
+"use strict";
+
+const webpack = require("../../../../");
+
+module.exports = {
+	plugins: [
+		new webpack.optimize.ModuleConcatenationPlugin()
+	]
+};