Merge pull request webpack#7493 from webpack/bugfix/issue-7492

sokra · web-flow · commit 551384a26e77 · 2018-06-07T08:25:17.000+02:00
fixes webpack#7492
diff --git a/lib/CaseSensitiveModulesWarning.js b/lib/CaseSensitiveModulesWarning.js
@@ -14,12 +14,12 @@ const WebpackError = require("./WebpackError");
  */
 const sortModules = modules => {
 	return modules.slice().sort((a, b) => {
-		a = a.identifier();
-		b = b.identifier();
+		const aIdent = a.identifier();
+		const bIdent = b.identifier();
 		/* istanbul ignore next */
-		if (a < b) return -1;
+		if (aIdent < bIdent) return -1;
 		/* istanbul ignore next */
-		if (a > b) return 1;
+		if (aIdent > bIdent) return 1;
 		/* istanbul ignore next */
 		return 0;
 	});
diff --git a/lib/Module.js b/lib/Module.js
@@ -370,8 +370,12 @@ Object.defineProperty(Module.prototype, "meta", {
 	}, "Module.meta was renamed to Module.buildMeta")
 });
 
+/** @type {function(): string} */
 Module.prototype.identifier = null;
+
+/** @type {function(RequestShortener): string} */
 Module.prototype.readableIdentifier = null;
+
 Module.prototype.build = null;
 Module.prototype.source = null;
 Module.prototype.size = null;
diff --git a/lib/RecordIdsPlugin.js b/lib/RecordIdsPlugin.js
@@ -6,16 +6,53 @@
 
 const identifierUtils = require("./util/identifier");
 
+/** @typedef {import("./Compiler")} Compiler */
+/** @typedef {import("./Chunk")} Chunk */
+/** @typedef {import("./Module")} Module */
+
+/**
+ * @typedef {Object} RecordsChunks
+ * @property {Record<string, number>=} byName
+ * @property {Record<string, number>=} bySource
+ * @property {number[]=} usedIds
+ */
+
+/**
+ * @typedef {Object} RecordsModules
+ * @property {Record<string, number>=} byIdentifier
+ * @property {Record<string, number>=} bySource
+ * @property {Record<number, number>=} usedIds
+ */
+
+/**
+ * @typedef {Object} Records
+ * @property {RecordsChunks=} chunks
+ * @property {RecordsModules=} modules
+ */
+
 class RecordIdsPlugin {
+	/**
+	 * @param {Object} options Options object
+	 * @param {boolean=} options.portableIds true, when ids need to be portable
+	 */
 	constructor(options) {
 		this.options = options || {};
 	}
 
+	/**
+	 * @param {Compiler} compiler the Compiler
+	 * @returns {void}
+	 */
 	apply(compiler) {
 		const portableIds = this.options.portableIds;
 		compiler.hooks.compilation.tap("RecordIdsPlugin", compilation => {
 			compilation.hooks.recordModules.tap(
 				"RecordIdsPlugin",
+				/**
+				 * @param {Module[]} modules the modules array
+				 * @param {Records} records the records object
+				 * @returns {void}
+				 */
 				(modules, records) => {
 					if (!records.modules) records.modules = {};
 					if (!records.modules.byIdentifier) records.modules.byIdentifier = {};
@@ -36,9 +73,15 @@ class RecordIdsPlugin {
 			);
 			compilation.hooks.reviveModules.tap(
 				"RecordIdsPlugin",
+				/**
+				 * @param {Module[]} modules the modules array
+				 * @param {Records} records the records object
+				 * @returns {void}
+				 */
 				(modules, records) => {
 					if (!records.modules) return;
 					if (records.modules.byIdentifier) {
+						/** @type {Set<number>} */
 						const usedIds = new Set();
 						for (const module of modules) {
 							if (module.id !== null) continue;
@@ -62,6 +105,10 @@ class RecordIdsPlugin {
 				}
 			);
 
+			/**
+			 * @param {Module} module the module
+			 * @returns {string} the (portable) identifier
+			 */
 			const getModuleIdentifier = module => {
 				if (portableIds) {
 					return identifierUtils.makePathsRelative(
@@ -73,7 +120,12 @@ class RecordIdsPlugin {
 				return module.identifier();
 			};
 
+			/**
+			 * @param {Chunk} chunk the chunk
+			 * @returns {string[]} sources of the chunk
+			 */
 			const getChunkSources = chunk => {
+				/** @type {string[]} */
 				const sources = [];
 				for (const chunkGroup of chunk.groupsIterable) {
 					const index = chunkGroup.chunks.indexOf(chunk);
@@ -108,10 +160,16 @@ class RecordIdsPlugin {
 
 			compilation.hooks.recordChunks.tap(
 				"RecordIdsPlugin",
+				/**
+				 * @param {Chunk[]} chunks the chunks array
+				 * @param {Records} records the records object
+				 * @returns {void}
+				 */
 				(chunks, records) => {
 					if (!records.chunks) records.chunks = {};
 					if (!records.chunks.byName) records.chunks.byName = {};
 					if (!records.chunks.bySource) records.chunks.bySource = {};
+					/** @type {Set<number>} */
 					const usedIds = new Set();
 					for (const chunk of chunks) {
 						if (typeof chunk.id !== "number") continue;
@@ -128,8 +186,14 @@ class RecordIdsPlugin {
 			);
 			compilation.hooks.reviveChunks.tap(
 				"RecordIdsPlugin",
+				/**
+				 * @param {Chunk[]} chunks the chunks array
+				 * @param {Records} records the records object
+				 * @returns {void}
+				 */
 				(chunks, records) => {
 					if (!records.chunks) return;
+					/** @type {Set<number>} */
 					const usedIds = new Set();
 					if (records.chunks.byName) {
 						for (const chunk of chunks) {
@@ -148,8 +212,8 @@ class RecordIdsPlugin {
 							for (const source of sources) {
 								const id = records.chunks.bySource[source];
 								if (id === undefined) continue;
-								if (usedIds[id]) continue;
-								usedIds[id] = true;
+								if (usedIds.has(id)) continue;
+								usedIds.add(id);
 								chunk.id = id;
 								break;
 							}
diff --git a/test/configCases/records/issue-7492/async.js b/test/configCases/records/issue-7492/async.js
@@ -0,0 +1 @@
+import "vendor";
diff --git a/test/configCases/records/issue-7492/index.js b/test/configCases/records/issue-7492/index.js
@@ -0,0 +1,3 @@
+it("should load fine", () => {
+	return import(/* webpackChunkName: "async" */"./async");
+});
diff --git a/test/configCases/records/issue-7492/node_modules/vendor.js b/test/configCases/records/issue-7492/node_modules/vendor.js
diff --git a/test/configCases/records/issue-7492/records.json b/test/configCases/records/issue-7492/records.json
@@ -0,0 +1,10 @@
+{
+  "chunks": {
+    "byName": {
+      "vendors~async": 123
+    },
+    "bySource": {
+      "1 index.js ./async": 123
+    }
+  }
+}
diff --git a/test/configCases/records/issue-7492/webpack.config.js b/test/configCases/records/issue-7492/webpack.config.js
@@ -0,0 +1,14 @@
+var path = require("path");
+
+module.exports = {
+	entry: "./index",
+	recordsInputPath: path.resolve(__dirname, "records.json"),
+	output: {
+		chunkFilename: "[name]-[chunkhash].js"
+	},
+	optimization: {
+		splitChunks: {
+			minSize: 0
+		}
+	}
+};

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+it("should load fine", () => {`
	`2`	`+ return import(/* webpackChunkName: "async" */"./async");`
	`3`	`+});`