Merge pull request webpack#7409 from ronanamsterdam/feature/webpack-options-compile-errors-warning

sokra · web-flow · commit 310cf6cb04dd · 2018-06-04T13:08:27.000+02:00
Issue webpack#7395: webpack import options comments warning propagation update
diff --git a/lib/CommentCompilationWarning.js b/lib/CommentCompilationWarning.js
@@ -0,0 +1,22 @@
+/*
+	MIT License http://www.opensource.org/licenses/mit-license.php
+	Author Tobias Koppers @sokra
+*/
+"use strict";
+
+const WebpackError = require("./WebpackError");
+
+class CommentCompilationWarning extends WebpackError {
+	constructor(message, module, loc) {
+		super(message);
+
+		this.name = "CommentCompilationWarning";
+
+		this.module = module;
+		this.loc = loc;
+
+		Error.captureStackTrace(this, this.constructor);
+	}
+}
+
+module.exports = CommentCompilationWarning;
diff --git a/lib/Parser.js b/lib/Parser.js
@@ -7,8 +7,8 @@
 // Syntax: https://developer.mozilla.org/en/SpiderMonkey/Parser_API
 
 const acorn = require("acorn-dynamic-import").default;
-const { Tapable, SyncBailHook } = require("tapable");
-const HookMap = require("tapable/lib/HookMap");
+const { Tapable, SyncBailHook, HookMap } = require("tapable");
+const util = require("util");
 const vm = require("vm");
 const BasicEvaluatedExpression = require("./BasicEvaluatedExpression");
 const StackedSetMap = require("./util/StackedSetMap");
@@ -31,6 +31,14 @@ const defaultParserOptions = {
 	}
 };
 
+// regexp to match at lease one "magic comment"
+const webpackCommentRegExp = new RegExp(/(^|\W)webpack[A-Z]{1,}[A-Za-z]{1,}:/);
+
+const EMPTY_COMMENT_OPTIONS = {
+	options: null,
+	errors: null
+};
+
 class Parser extends Tapable {
 	constructor(options, sourceType = "auto") {
 		super();
@@ -2060,20 +2068,27 @@ class Parser extends Tapable {
 		);
 	}
 
-	getCommentOptions(range) {
+	parseCommentOptions(range) {
 		const comments = this.getComments(range);
-		if (comments.length === 0) return null;
-		const options = comments.map(comment => {
-			try {
-				let val = vm.runInNewContext(
-					`(function(){return {${comment.value}};})()`
-				);
-				return val;
-			} catch (e) {
-				return {};
+		if (comments.length === 0) {
+			return EMPTY_COMMENT_OPTIONS;
+		}
+		let options = {};
+		let errors = [];
+		for (const comment of comments) {
+			const { value } = comment;
+			if (value && webpackCommentRegExp.test(value)) {
+				// try compile only if webpack options comment is present
+				try {
+					const val = vm.runInNewContext(`(function(){return {${value}};})()`);
+					Object.assign(options, val);
+				} catch (e) {
+					e.comment = comment;
+					errors.push(e);
+				}
 			}
-		});
-		return options.reduce((o, i) => Object.assign(o, i), {});
+		}
+		return { options, errors };
 	}
 
 	getNameForExpression(expression) {
@@ -2161,4 +2176,12 @@ class Parser extends Tapable {
 	}
 }
 
+// TODO remove in webpack 5
+Object.defineProperty(Parser.prototype, "getCommentOptions", {
+	configurable: false,
+	value: util.deprecate(function(range) {
+		return this.parseCommentOptions(range).options;
+	}, "Parser.getCommentOptions: Use Parser.parseCommentOptions(range) instead")
+});
+
 module.exports = Parser;
diff --git a/lib/dependencies/ImportParserPlugin.js b/lib/dependencies/ImportParserPlugin.js
@@ -10,6 +10,7 @@ const ImportDependenciesBlock = require("./ImportDependenciesBlock");
 const ImportEagerDependency = require("./ImportEagerDependency");
 const ContextDependencyHelpers = require("./ContextDependencyHelpers");
 const UnsupportedFeatureWarning = require("../UnsupportedFeatureWarning");
+const CommentCompilationWarning = require("../CommentCompilationWarning");
 
 class ImportParserPlugin {
 	constructor(options) {
@@ -32,7 +33,26 @@ class ImportParserPlugin {
 			let exclude = null;
 			const groupOptions = {};
 
-			const importOptions = parser.getCommentOptions(expr.range);
+			const {
+				options: importOptions,
+				errors: commentErrors
+			} = parser.parseCommentOptions(expr.range);
+
+			if (commentErrors) {
+				for (const e of commentErrors) {
+					const { comment } = e;
+					parser.state.module.warnings.push(
+						new CommentCompilationWarning(
+							`Compilation error while processing magic comment(-s): /*${
+								comment.value
+							}*/: ${e.message}`,
+							parser.state.module,
+							comment.loc
+						)
+					);
+				}
+			}
+
 			if (importOptions) {
 				if (typeof importOptions.webpackIgnore !== "undefined") {
 					if (typeof importOptions.webpackIgnore !== "boolean") {
@@ -239,4 +259,5 @@ class ImportParserPlugin {
 		});
 	}
 }
+
 module.exports = ImportParserPlugin;
diff --git a/test/__snapshots__/StatsTestCases.test.js.snap b/test/__snapshots__/StatsTestCases.test.js.snap
@@ -1007,6 +1007,28 @@ Entrypoint entry = entry.js
 [2] ./modules/a.js 37 bytes [built]"
 `;
 
+exports[`StatsTestCases should print correct stats for import-with-invalid-options-comments 1`] = `
+"Built at: Thu Jan 01 1970 00:00:00 GMT
+[0] ./chunk-a.js 27 bytes {2} [built]
+[1] ./chunk-b.js 27 bytes {3} [built]
+[2] ./chunk-c.js 27 bytes {4} [built]
+[3] ./chunk-d.js 27 bytes {5} [built]
+[4] ./chunk.js 401 bytes {0} [built] [3 warnings]
+[5] ./index.js 50 bytes {1} [built]
+
+WARNING in ./chunk.js 4:11-77
+Compilation error while processing magic comment(-s): /* webpack Prefetch: 0, webpackChunkName: \\"notGoingToCompile-c\\" */: Unexpected identifier
+ @ ./index.js 1:0-49
+
+WARNING in ./chunk.js 5:11-38
+Compilation error while processing magic comment(-s): /* webpackPrefetch: nope */: nope is not defined
+ @ ./index.js 1:0-49
+
+WARNING in ./chunk.js 2:11-84
+Compilation error while processing magic comment(-s): /* webpackPrefetch: true, webpackChunkName: notGoingToCompileChunkName */: notGoingToCompileChunkName is not defined
+ @ ./index.js 1:0-49"
+`;
+
 exports[`StatsTestCases should print correct stats for limit-chunk-count-plugin 1`] = `
 "Hash: 3c127dca42ce1c13b8eae6c0ceac1aa3be512946a5969fc94d792cce5364503bf42779f704747e2d
 Child 1 chunks:
diff --git a/test/statsCases/import-with-invalid-options-comments/chunk-a.js b/test/statsCases/import-with-invalid-options-comments/chunk-a.js
@@ -0,0 +1 @@
+module.exports = "chunk-a";
diff --git a/test/statsCases/import-with-invalid-options-comments/chunk-b.js b/test/statsCases/import-with-invalid-options-comments/chunk-b.js
@@ -0,0 +1 @@
+module.exports = "chunk-b";
diff --git a/test/statsCases/import-with-invalid-options-comments/chunk-c.js b/test/statsCases/import-with-invalid-options-comments/chunk-c.js
@@ -0,0 +1 @@
+module.exports = "chunk-c";
diff --git a/test/statsCases/import-with-invalid-options-comments/chunk-d.js b/test/statsCases/import-with-invalid-options-comments/chunk-d.js
@@ -0,0 +1 @@
+module.exports = "chunk-d";
diff --git a/test/statsCases/import-with-invalid-options-comments/chunk.js b/test/statsCases/import-with-invalid-options-comments/chunk.js
@@ -0,0 +1,6 @@
+export default function() {
+    import(/* webpackPrefetch: true, webpackChunkName: notGoingToCompileChunkName */ "./chunk-a");
+    import(/* webpackPrefetch: 0, webpackChunkName: "goingToCompileChunkName-b" */ "./chunk-b");
+    import(/* webpack Prefetch: 0, webpackChunkName: "notGoingToCompile-c" */ "./chunk-c");
+    import(/* webpackPrefetch: nope */ /* webpackChunkName: "yep" */ "./chunk-d");
+}
diff --git a/test/statsCases/import-with-invalid-options-comments/index.js b/test/statsCases/import-with-invalid-options-comments/index.js
@@ -0,0 +1 @@
+import(/* webpackChunkName: "chunk" */ "./chunk");
diff --git a/test/statsCases/import-with-invalid-options-comments/webpack.config.js b/test/statsCases/import-with-invalid-options-comments/webpack.config.js
@@ -0,0 +1,15 @@
+module.exports = {
+	mode: "production",
+	entry: "./index",
+	output: {
+		chunkFilename: "[name].js"
+	},
+	stats: {
+		timings: false,
+		hash: false,
+		entrypoints: false,
+		assets: false,
+		errorDetails: true,
+		moduleTrace: true
+	}
+};

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+import(/* webpackChunkName: "chunk" */ "./chunk");`