move pathinfo comment into Template helper function

sokra · sokra · commit 91bfd4559ac1 · 2017-08-08T09:42:43.000+02:00
change toIdentifier to generate less underscores
diff --git a/lib/AmdMainTemplatePlugin.js b/lib/AmdMainTemplatePlugin.js
@@ -22,7 +22,7 @@ class AmdMainTemplatePlugin {
 				typeof m.request === "object" ? m.request.amd : m.request
 			));
 			const externalsArguments = externals.map((m) =>
-				Template.toIdentifier(`__WEBPACK_EXTERNAL_MODULE_${m.id}__`)
+				`__WEBPACK_EXTERNAL_MODULE_${Template.toIdentifier(`${m.id}`)}__`
 			).join(", ");
 
 			if(this.name) {
diff --git a/lib/BannerPlugin.js b/lib/BannerPlugin.js
@@ -7,10 +7,11 @@
 
 const ConcatSource = require("webpack-sources").ConcatSource;
 const ModuleFilenameHelpers = require("./ModuleFilenameHelpers");
+const Template = require("./Template");
 
 const wrapComment = (str) => {
-	if(!str.includes("\n")) return `/*! ${str} */`;
-	return `/*!\n * ${str.split("\n").join("\n * ")}\n */`;
+	if(!str.includes("\n")) return Template.toComment(str);
+	return `/*!\n * ${str.replace(/\*\//g, "* /").split("\n").join("\n * ")}\n */`;
 };
 
 class BannerPlugin {
diff --git a/lib/ExternalModule.js b/lib/ExternalModule.js
@@ -70,7 +70,7 @@ class ExternalModule extends Module {
 	}
 
 	getSourceForAmdOrUmdExternal(id, optional, request) {
-		const externalVariable = Template.toIdentifier(`__WEBPACK_EXTERNAL_MODULE_${id}__`);
+		const externalVariable = `__WEBPACK_EXTERNAL_MODULE_${Template.toIdentifier(`${id}`)}__`;
 		const missingModuleError = optional ? this.checkExternalVariable(externalVariable, request) : "";
 		return `${missingModuleError}module.exports = ${externalVariable};`;
 	}
diff --git a/lib/FunctionModuleTemplatePlugin.js b/lib/FunctionModuleTemplatePlugin.js
@@ -5,6 +5,7 @@
 "use strict";
 
 const ConcatSource = require("webpack-sources").ConcatSource;
+const Template = require("./Template");
 
 class FunctionModuleTemplatePlugin {
 	apply(moduleTemplate) {
@@ -29,17 +30,17 @@ class FunctionModuleTemplatePlugin {
 				source.add("  !*** " + req.replace(/\*\//g, "*_/") + " ***!\n");
 				source.add("  \\****" + req.replace(/./g, "*") + "****/\n");
 				if(Array.isArray(module.providedExports))
-					source.add("/*! exports provided: " + module.providedExports.join(", ") + " */\n");
+					source.add(Template.toComment("exports provided: " + module.providedExports.join(", ")) + "\n");
 				else if(module.providedExports)
-					source.add("/*! no static exports found */\n");
+					source.add(Template.toComment("no static exports found") + "\n");
 				if(Array.isArray(module.usedExports))
-					source.add("/*! exports used: " + module.usedExports.join(", ") + " */\n");
+					source.add(Template.toComment("exports used: " + module.usedExports.join(", ")) + "\n");
 				else if(module.usedExports)
-					source.add("/*! all exports used */\n");
+					source.add(Template.toComment("all exports used") + "\n");
 				if(module.optimizationBailout) {
 					module.optimizationBailout.forEach(text => {
 						if(typeof text === "function") text = text(this.requestShortener);
-						source.add(`/*! ${text} */\n`);
+						source.add(Template.toComment(`${text}`) + "\n");
 					});
 				}
 				source.add(moduleSource);
diff --git a/lib/MultiModule.js b/lib/MultiModule.js
@@ -5,6 +5,7 @@
 "use strict";
 
 const Module = require("./Module");
+const Template = require("./Template");
 const RawSource = require("webpack-sources").RawSource;
 
 class MultiModule extends Module {
@@ -58,7 +59,7 @@ class MultiModule extends Module {
 					str.push("module.exports = ");
 				str.push("__webpack_require__(");
 				if(outputOptions.pathinfo)
-					str.push(`/*! ${dep.request} */`);
+					str.push(Template.toComment(dep.request));
 				str.push(`${JSON.stringify(dep.module.id)}`);
 				str.push(")");
 			} else {
diff --git a/lib/Template.js b/lib/Template.js
@@ -23,7 +23,17 @@ module.exports = class Template extends Tapable {
 
 	static toIdentifier(str) {
 		if(typeof str !== "string") return "";
-		return str.replace(/^[^a-zA-Z$_]/, "_").replace(/[^a-zA-Z0-9$_]/g, "_");
+		return str.replace(/^([^a-zA-Z$_])/, "_$1").replace(/[^a-zA-Z0-9$]+/g, "_");
+	}
+
+	static toComment(str) {
+		if(!str) return "";
+		return `/*! ${str.replace(/\*\//g, "* /")} */`;
+	}
+
+	static toNormalComment(str) {
+		if(!str) return "";
+		return `/* ${str.replace(/\*\//g, "* /")} */`;
 	}
 
 	static toPath(str) {
diff --git a/lib/UmdMainTemplatePlugin.js b/lib/UmdMainTemplatePlugin.js
@@ -95,7 +95,7 @@ class UmdMainTemplatePlugin {
 			}
 
 			function externalsArguments(modules) {
-				return modules.map(m => Template.toIdentifier(`__WEBPACK_EXTERNAL_MODULE_${m.id}__`)).join(", ");
+				return modules.map(m => `__WEBPACK_EXTERNAL_MODULE_${Template.toIdentifier(`${m.id}`)}__`).join(", ");
 			}
 
 			function libraryName(library) {
diff --git a/lib/dependencies/AMDRequireArrayDependency.js b/lib/dependencies/AMDRequireArrayDependency.js
@@ -4,6 +4,7 @@
 */
 "use strict";
 const Dependency = require("../Dependency");
+const Template = require("../Template");
 const webpackMissingModuleModule = require("./WebpackMissingModule").module;
 
 class AMDRequireArrayDependency extends Dependency {
@@ -26,19 +27,12 @@ AMDRequireArrayDependency.Template = class AMDRequireArrayDependencyTemplate {
 
 	getContent(dep, outputOptions, requestShortener) {
 		const requires = dep.depsArray.map((dependency) => {
-			const optionalComment = this.optionalComment(outputOptions.pathinfo, requestShortener.shorten(dependency.request));
+			const optionalComment = outputOptions.pathinfo ? Template.toComment(requestShortener.shorten(dependency.request)) : "";
 			return this.contentForDependency(dependency, optionalComment);
 		});
 		return `[${requires.join(", ")}]`;
 	}
 
-	optionalComment(pathInfo, shortenedRequest) {
-		if(!pathInfo) {
-			return "";
-		}
-		return `/*! ${shortenedRequest} */ `;
-	}
-
 	contentForDependency(dep, comment) {
 		if(typeof dep === "string") {
 			return dep;
diff --git a/lib/dependencies/ContextDependencyTemplateAsId.js b/lib/dependencies/ContextDependencyTemplateAsId.js
@@ -4,11 +4,12 @@
 */
 "use strict";
 
+const Template = require("../Template");
+
 class ContextDependencyTemplateAsId {
 
 	apply(dep, source, outputOptions, requestShortener) {
-		const comment = outputOptions.pathinfo ?
-			"/*! " + requestShortener.shorten(dep.request) + " */ " : "";
+		const comment = outputOptions.pathinfo ? Template.toComment(requestShortener.shorten(dep.request)) + " " : "";
 
 		if(dep.module && dep.module.dependencies && dep.module.dependencies.length > 0) {
 			if(dep.valueRange) {
diff --git a/lib/dependencies/ContextDependencyTemplateAsRequireCall.js b/lib/dependencies/ContextDependencyTemplateAsRequireCall.js
@@ -4,11 +4,12 @@
 */
 "use strict";
 
+const Template = require("../Template");
+
 class ContextDependencyTemplateAsRequireCall {
 
 	apply(dep, source, outputOptions, requestShortener) {
-		const comment = outputOptions.pathinfo ?
-			"/*! " + requestShortener.shorten(dep.request) + " */ " : "";
+		const comment = outputOptions.pathinfo ? Template.toComment(requestShortener.shorten(dep.request)) + " " : "";
 
 		const containsDeps = dep.module && dep.module.dependencies && dep.module.dependencies.length > 0;
 		const isAsync = dep.module && dep.module.async;
diff --git a/lib/dependencies/DepBlockHelpers.js b/lib/dependencies/DepBlockHelpers.js
@@ -4,6 +4,8 @@
 */
 "use strict";
 
+const Template = require("../Template");
+
 const DepBlockHelpers = exports;
 
 DepBlockHelpers.getLoadDepBlockWrapper = (depBlock, outputOptions, requestShortener, name) => {
@@ -25,9 +27,9 @@ DepBlockHelpers.getDepBlockPromise = (depBlock, outputOptions, requestShortener,
 		name = asComment(name);
 		if(chunks.length === 1) {
 			const chunkId = JSON.stringify(chunks[0].id);
-			return `__webpack_require__.e${name}(${chunkId}${pathChunkCheck ? "/*! " + shortChunkName + " */" : ""}${chunkReason})`;
+			return `__webpack_require__.e${name}(${chunkId}${pathChunkCheck ? Template.toComment(shortChunkName) : ""}${chunkReason})`;
 		} else if(chunks.length > 0) {
-			return `Promise.all${name}(${pathChunkCheck ? "/*! " + shortChunkName + " */" : ""}[${chunks.map(requireChunkId).join(", ")}])`;
+			return `Promise.all${name}(${pathChunkCheck ? Template.toComment(shortChunkName) : ""}[${chunks.map(requireChunkId).join(", ")}])`;
 		}
 	}
 	return "new Promise(function(resolve) { resolve(); })";
diff --git a/lib/dependencies/ImportDependency.js b/lib/dependencies/ImportDependency.js
@@ -4,6 +4,7 @@
 */
 "use strict";
 const ModuleDependency = require("./ModuleDependency");
+const Template = require("../Template");
 const DepBlockHelpers = require("./DepBlockHelpers");
 const webpackMissingPromiseModule = require("./WebpackMissingModule").promise;
 
@@ -22,20 +23,12 @@ ImportDependency.Template = class ImportDependencyTemplate {
 	apply(dep, source, outputOptions, requestShortener) {
 		const depBlock = dep.block;
 		const promise = DepBlockHelpers.getDepBlockPromise(depBlock, outputOptions, requestShortener, "import()");
-		const comment = this.getOptionalComment(outputOptions.pathinfo, requestShortener.shorten(dep.request));
+		const comment = outputOptions.pathinfo ? Template.toComment(requestShortener.shorten(dep.request)) : "";
 
 		const content = this.getContent(promise, dep, comment);
 		source.replace(depBlock.range[0], depBlock.range[1] - 1, content);
 	}
 
-	getOptionalComment(pathinfo, shortenedRequest) {
-		if(!pathinfo) {
-			return "";
-		}
-
-		return `/*! ${shortenedRequest} */ `;
-	}
-
 	getContent(promise, dep, comment) {
 		if(promise && dep.module) {
 			const stringifiedId = JSON.stringify(dep.module.id);
diff --git a/lib/dependencies/ImportEagerDependency.js b/lib/dependencies/ImportEagerDependency.js
@@ -4,6 +4,7 @@
 */
 "use strict";
 const ModuleDependency = require("./ModuleDependency");
+const Template = require("../Template");
 const webpackMissingPromiseModule = require("./WebpackMissingModule").promise;
 
 class ImportEagerDependency extends ModuleDependency {
@@ -19,20 +20,12 @@ class ImportEagerDependency extends ModuleDependency {
 
 ImportEagerDependency.Template = class ImportEagerDependencyTemplate {
 	apply(dep, source, outputOptions, requestShortener) {
-		const comment = this.getOptionalComment(outputOptions.pathinfo, requestShortener.shorten(dep.request));
+		const comment = outputOptions.pathinfo ? Template.toComment(requestShortener.shorten(dep.request)) : "";
 
 		const content = this.getContent(dep, comment);
 		source.replace(dep.range[0], dep.range[1] - 1, content);
 	}
 
-	getOptionalComment(pathinfo, shortenedRequest) {
-		if(!pathinfo) {
-			return "";
-		}
-
-		return `/*! ${shortenedRequest} */ `;
-	}
-
 	getContent(dep, comment) {
 		if(dep.module) {
 			const stringifiedId = JSON.stringify(dep.module.id);
diff --git a/lib/dependencies/ImportWeakDependency.js b/lib/dependencies/ImportWeakDependency.js
@@ -4,6 +4,7 @@
 */
 "use strict";
 const ModuleDependency = require("./ModuleDependency");
+const Template = require("../Template");
 const webpackMissingPromiseModule = require("./WebpackMissingModule").promise;
 
 class ImportWeakDependency extends ModuleDependency {
@@ -20,20 +21,12 @@ class ImportWeakDependency extends ModuleDependency {
 
 ImportWeakDependency.Template = class ImportDependencyTemplate {
 	apply(dep, source, outputOptions, requestShortener) {
-		const comment = this.getOptionalComment(outputOptions.pathinfo, requestShortener.shorten(dep.request));
+		const comment = outputOptions.pathinfo ? Template.toComment(requestShortener.shorten(dep.request)) : "";
 
 		const content = this.getContent(dep, comment);
 		source.replace(dep.range[0], dep.range[1] - 1, content);
 	}
 
-	getOptionalComment(pathinfo, shortenedRequest) {
-		if(!pathinfo) {
-			return "";
-		}
-
-		return `/*! ${shortenedRequest} */ `;
-	}
-
 	getContent(dep, comment) {
 		if(dep.module) {
 			const stringifiedId = JSON.stringify(dep.module.id);
diff --git a/lib/dependencies/ModuleDependencyTemplateAsId.js b/lib/dependencies/ModuleDependencyTemplateAsId.js
@@ -4,12 +4,14 @@
 */
 "use strict";
 
+const Template = require("../Template");
+
 class ModuleDependencyTemplateAsId {
 
 	apply(dep, source, outputOptions, requestShortener) {
 		if(!dep.range) return;
 		const comment = outputOptions.pathinfo ?
-			`/*! ${requestShortener.shorten(dep.request)} */ ` : "";
+			Template.toComment(requestShortener.shorten(dep.request)) + " " : "";
 		let content;
 		if(dep.module)
 			content = comment + JSON.stringify(dep.module.id);
diff --git a/lib/dependencies/ModuleDependencyTemplateAsRequireId.js b/lib/dependencies/ModuleDependencyTemplateAsRequireId.js
@@ -4,12 +4,14 @@
 */
 "use strict";
 
+const Template = require("../Template");
+
 class ModuleDependencyTemplateAsRequireId {
 
 	apply(dep, source, outputOptions, requestShortener) {
 		if(!dep.range) return;
 		const comment = outputOptions.pathinfo ?
-			`/*! ${requestShortener.shorten(dep.request)} */ ` : "";
+			Template.toComment(requestShortener.shorten(dep.request)) + " " : "";
 		let content;
 		if(dep.module)
 			content = `__webpack_require__(${comment}${JSON.stringify(dep.module.id)})`;
diff --git a/lib/dependencies/RequireIncludeDependency.js b/lib/dependencies/RequireIncludeDependency.js
@@ -3,7 +3,9 @@
 	Author Tobias Koppers @sokra
 */
 "use strict";
+
 const ModuleDependency = require("./ModuleDependency");
+const Template = require("../Template");
 
 class RequireIncludeDependency extends ModuleDependency {
 	constructor(request, range) {
@@ -18,16 +20,9 @@ class RequireIncludeDependency extends ModuleDependency {
 
 RequireIncludeDependency.Template = class RequireIncludeDependencyTemplate {
 	apply(dep, source, outputOptions, requestShortener) {
-		const comment = this.getOptionalComment(outputOptions.pathinfo && dep.module, requestShortener.shorten(dep.request));
+		const comment = outputOptions.pathinfo ? Template.toComment(`require.include ${requestShortener.shorten(dep.request)}`) : "";
 		source.replace(dep.range[0], dep.range[1] - 1, `undefined${comment}`);
 	}
-
-	getOptionalComment(shouldHaveComment, shortenedRequest) {
-		if(shouldHaveComment) {
-			return "";
-		}
-		return `/*! require.include ${shortenedRequest} */`;
-	}
 };
 
 module.exports = RequireIncludeDependency;
diff --git a/lib/optimize/ConcatenatedModule.js b/lib/optimize/ConcatenatedModule.js
@@ -40,8 +40,7 @@ function getExternalImport(importedModule, importedVar, exportName, asCall) {
 	if(!isHarmonyModule && exportName === "default") {
 		return asCall ? `${importedVar}_default()` : `${importedVar}_default.a`;
 	}
-	// TODO use Template.toNormalComment when merging with pure-module
-	const comment = used !== exportName ? ` /* ${exportName} */` : "";
+	const comment = used !== exportName ? ` ${Template.toNormalComment(exportName)}` : "";
 	const reference = `${importedVar}[${JSON.stringify(used)}${comment}]`;
 	if(asCall)
 		return `Object(${reference})`;
@@ -78,8 +77,7 @@ function getFinalName(info, exportName, moduleToInfoMap, requestShortener, asCal
 				const problem = `Cannot get final name for export "${exportName}" in "${info.module.readableIdentifier(requestShortener)}"` +
 					` (known exports: ${Array.from(info.exportMap.keys()).filter(name => name !== true).join(" ")}, ` +
 					`known reexports: ${Array.from(info.reexportMap.keys()).join(" ")})`;
-				// TODO use Template.toNormalComment when merging with pure-module
-				return `/* ${problem} */ undefined`;
+				return `${Template.toNormalComment(problem)} undefined`;
 			}
 		case "external":
 			{
diff --git a/test/Template.test.js b/test/Template.test.js
@@ -6,7 +6,7 @@ const template = require("../lib/Template");
 
 describe("Template", () => {
 	it("should generate valid identifiers", () =>
-		template.toIdentifier("0abc-def9").should.equal("_abc_def9"));
+		template.toIdentifier("0abc-def9").should.equal("_0abc_def9"));
 	it("should generate valid number identifiers", () => {
 		const items = [];
 		let item;

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ class ExternalModule extends Module {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`getSourceForAmdOrUmdExternal(id, optional, request) {`
`73`		- const externalVariable = Template.toIdentifier(`__WEBPACK_EXTERNAL_MODULE_${id}__`);
	`73`	+ const externalVariable = `__WEBPACK_EXTERNAL_MODULE_${Template.toIdentifier(`${id}`)}__`;
`74`	`74`	`const missingModuleError = optional ? this.checkExternalVariable(externalVariable, request) : "";`
`75`	`75`	return `${missingModuleError}module.exports = ${externalVariable};`;
`76`	`76`	`}`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ class UmdMainTemplatePlugin {`
`95`	`95`	`}`
`96`	`96`
`97`	`97`	`function externalsArguments(modules) {`
`98`		- return modules.map(m => Template.toIdentifier(`__WEBPACK_EXTERNAL_MODULE_${m.id}__`)).join(", ");
	`98`	+ return modules.map(m => `__WEBPACK_EXTERNAL_MODULE_${Template.toIdentifier(`${m.id}`)}__`).join(", ");
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`function libraryName(library) {`