Merge pull request webpack#6143 from ooflorent/module_id_bailout

sokra · web-flow · commit 5edc7efb1a60 · 2017-12-19T13:06:30.000+01:00
Bail out concatenation if module uses module.id or module.loaded
diff --git a/lib/NodeStuffPlugin.js b/lib/NodeStuffPlugin.js
@@ -70,8 +70,14 @@ class NodeStuffPlugin {
 					"expression require.extensions",
 					ParserHelpers.expressionIsUnsupported(parser, "require.extensions is not supported by webpack. Use a loader instead.")
 				);
-				parser.plugin("expression module.loaded", ParserHelpers.toConstantDependency(parser, "module.l"));
-				parser.plugin("expression module.id", ParserHelpers.toConstantDependency(parser, "module.i"));
+				parser.plugin("expression module.loaded", expr => {
+					parser.state.module.buildMeta.moduleConcatenationBailout = "module.loaded";
+					return ParserHelpers.toConstantDependency(parser, "module.l")(expr);
+				});
+				parser.plugin("expression module.id", expr => {
+					parser.state.module.buildMeta.moduleConcatenationBailout = "module.id";
+					return ParserHelpers.toConstantDependency(parser, "module.i")(expr);
+				});
 				parser.plugin("expression module.exports", () => {
 					const module = parser.state.module;
 					const isHarmony = module.buildMeta && module.buildMeta.harmonyModule;
diff --git a/lib/optimize/ModuleConcatenationPlugin.js b/lib/optimize/ModuleConcatenationPlugin.js
@@ -27,7 +27,8 @@ class ModuleConcatenationPlugin {
 		}) => {
 			const handler = (parser, parserOptions) => {
 				parser.plugin("call eval", () => {
-					parser.state.module.buildMeta.hasEval = true;
+					// Because of variable renaming we can't use modules with eval.
+					parser.state.module.buildMeta.moduleConcatenationBailout = "eval()";
 				});
 			};
 
@@ -58,9 +59,11 @@ class ModuleConcatenationPlugin {
 						continue;
 					}
 
-					// Because of variable renaming we can't use modules with eval
-					if(module.buildMeta && module.buildMeta.hasEval) {
-						setBailoutReason(module, "Module uses eval()");
+					// Some expressions are not compatible with module concatenation
+					// because they may produce unexpected results. The plugin bails out
+					// if some were detected upfront.
+					if(module.buildMeta && module.buildMeta.moduleConcatenationBailout) {
+						setBailoutReason(module, `Module uses ${module.buildMeta.moduleConcatenationBailout}`);
 						continue;
 					}
 
diff --git a/test/statsCases/scope-hoisting-bailouts/cjs.js b/test/statsCases/scope-hoisting-bailouts/cjs.js
@@ -0,0 +1,3 @@
+require("./ref-from-cjs");
+
+module.exports = "cjs module";
diff --git a/test/statsCases/scope-hoisting-bailouts/entry.js b/test/statsCases/scope-hoisting-bailouts/entry.js
@@ -0,0 +1 @@
+export default "another entry";
diff --git a/test/statsCases/scope-hoisting-bailouts/eval.js b/test/statsCases/scope-hoisting-bailouts/eval.js
@@ -0,0 +1 @@
+export default eval("using eval");
diff --git a/test/statsCases/scope-hoisting-bailouts/expected.txt b/test/statsCases/scope-hoisting-bailouts/expected.txt
@@ -0,0 +1,18 @@
+Hash: 841dd2f7e2da346f7bf1
+Time: Xms
+   [0] ./entry.js 32 bytes {0} {1} [built]
+       ModuleConcatenation bailout: Module is an entry point
+   [1] ./ref-from-cjs.js 45 bytes {0} [built]
+       ModuleConcatenation bailout: Module is referenced from these modules with unsupported syntax: ./cjs.js (referenced with cjs require)
+   [2] ./index.js 150 bytes {0} [built]
+       ModuleConcatenation bailout: Module is an entry point
+   [3] ./cjs.js 59 bytes {0} [built]
+       ModuleConcatenation bailout: Module is not an ECMAScript module
+   [4] ./eval.js 35 bytes {0} [built]
+       ModuleConcatenation bailout: Module uses eval()
+   [5] ./injected-vars.js 40 bytes {0} [built]
+       ModuleConcatenation bailout: Module uses injected variables (__dirname, __filename)
+   [6] ./module-id.js 26 bytes {0} [built]
+       ModuleConcatenation bailout: Module uses module.id
+   [7] ./module-loaded.js 30 bytes {0} [built]
+       ModuleConcatenation bailout: Module uses module.loaded
diff --git a/test/statsCases/scope-hoisting-bailouts/index.js b/test/statsCases/scope-hoisting-bailouts/index.js
@@ -0,0 +1,7 @@
+import "./cjs";
+import "./entry";
+import "./eval";
+import "./injected-vars";
+import "./module-id";
+import "./module-loaded";
+import "./ref-from-cjs";
diff --git a/test/statsCases/scope-hoisting-bailouts/injected-vars.js b/test/statsCases/scope-hoisting-bailouts/injected-vars.js
@@ -0,0 +1 @@
+export default [__dirname, __filename];
diff --git a/test/statsCases/scope-hoisting-bailouts/module-id.js b/test/statsCases/scope-hoisting-bailouts/module-id.js
@@ -0,0 +1 @@
+export default module.id;
diff --git a/test/statsCases/scope-hoisting-bailouts/module-loaded.js b/test/statsCases/scope-hoisting-bailouts/module-loaded.js
@@ -0,0 +1 @@
+export default module.loaded;
diff --git a/test/statsCases/scope-hoisting-bailouts/ref-from-cjs.js b/test/statsCases/scope-hoisting-bailouts/ref-from-cjs.js
@@ -0,0 +1 @@
+export default "referenced by a CJS module";
diff --git a/test/statsCases/scope-hoisting-bailouts/webpack.config.js b/test/statsCases/scope-hoisting-bailouts/webpack.config.js
@@ -0,0 +1,15 @@
+module.exports = {
+	mode: "production",
+	entry: {
+		index: "./index.js",
+		entry: "./entry.js"
+	},
+	target: "web",
+	output: {
+		filename: "[name].js"
+	},
+	stats: {
+		assets: false,
+		optimizationBailout: true
+	}
+};
diff --git a/test/statsCases/separate-css-bundle/expected.txt b/test/statsCases/separate-css-bundle/expected.txt
@@ -1,6 +1,6 @@
-Hash: aa1b352e571b0ba5e7f5d37c33df2ddf3d79c4f9
+Hash: 6324063243d1151e39ba483784dda778b9890e57
 Child
-    Hash: aa1b352e571b0ba5e7f5
+    Hash: 6324063243d1151e39ba
     Time: Xms
                                    Asset      Size  Chunks             Chunk Names
                  d6c1b876fc64139d8324.js  2.74 KiB       0  [emitted]  main
@@ -15,7 +15,7 @@ Child
            [0] (webpack)/node_modules/css-loader!./a/file.css 199 bytes {0} [built]
            [1] (webpack)/node_modules/css-loader/lib/css-base.js 2.21 KiB {0} [built]
 Child
-    Hash: d37c33df2ddf3d79c4f9
+    Hash: 483784dda778b9890e57
     Time: Xms
                                    Asset      Size  Chunks             Chunk Names
                  d6c1b876fc64139d8324.js  2.74 KiB       0  [emitted]  main

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+require("./ref-from-cjs");`
	`2`	`+`
	`3`	`+module.exports = "cjs module";`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+export default [__dirname, __filename];`