CommonsChunkPlugin: Don't allow blocks to contain duplicate chunks

ymichael · ymichael · commit 7834e6cd5703 · 2017-05-30T21:11:42.000-07:00
- Add configCase to repro failure
diff --git a/lib/optimize/CommonsChunkPlugin.js b/lib/optimize/CommonsChunkPlugin.js
@@ -340,7 +340,9 @@ Take a look at the "name"/"names" or async/children option.`);
 	moveExtractedChunkBlocksToTargetChunk(chunks, targetChunk) {
 		for(let chunk of chunks) {
 			for(let block of chunk.blocks) {
-				block.chunks.unshift(targetChunk);
+				if(block.chunks.indexOf(targetChunk) === -1) {
+					block.chunks.unshift(targetChunk);
+				}
 				targetChunk.addBlock(block);
 			}
 		}
diff --git a/test/configCases/async-commons-chunk/existing-name/a.js b/test/configCases/async-commons-chunk/existing-name/a.js
@@ -0,0 +1 @@
+module.exports = "a";
diff --git a/test/configCases/async-commons-chunk/existing-name/b.js b/test/configCases/async-commons-chunk/existing-name/b.js
@@ -0,0 +1 @@
+module.exports = "b";
diff --git a/test/configCases/async-commons-chunk/existing-name/c.js b/test/configCases/async-commons-chunk/existing-name/c.js
@@ -0,0 +1 @@
+module.exports = "c";
diff --git a/test/configCases/async-commons-chunk/existing-name/index.js b/test/configCases/async-commons-chunk/existing-name/index.js
@@ -0,0 +1,33 @@
+require("should");
+const sinon = require("sinon");
+const chunkLoadingSpy = sinon.spy(__webpack_require__, 'e');
+
+it("should not have duplicate chunks in blocks", function(done) {
+    // This split point should contain: a
+	require.ensure([], function(require) {
+		require("./a").should.be.eql("a");
+	}, 'a');
+
+    // This split point should contain: a and b - we use CommonsChunksPlugin to
+    // have it only contain b and make chunk a be an async dependency.
+	require.ensure([], function(require) {
+		require("./a").should.be.eql("a");
+		require("./b").should.be.eql("b");
+	}, 'a+b');
+
+    // This split point should contain: a, b and c - we use CommonsChunksPlugin to
+    // have it only contain c and make chunks a and a+b be async dependencies.
+	require.ensure([], function(require) {
+		require("./a").should.be.eql("a");
+		require("./b").should.be.eql("b");
+		require("./c").should.be.eql("c");
+	}, 'a+b+c');
+
+    // Each of the require.ensures above should end up resolving chunks:
+    // - a
+    // - a, a+b
+    // - a, a+b, a+b+c
+	chunkLoadingSpy.callCount.should.be.eql(6);
+	chunkLoadingSpy.args.should.be.eql([[0], [0], [1], [0], [1], [2]]);
+	done();
+});
diff --git a/test/configCases/async-commons-chunk/existing-name/webpack.config.js b/test/configCases/async-commons-chunk/existing-name/webpack.config.js
@@ -0,0 +1,14 @@
+var webpack = require("../../../../");
+
+module.exports = {
+	plugins: [
+		new webpack.optimize.CommonsChunkPlugin({
+			chunks: ["a+b", "a+b+c"],
+			async: "a+b",
+		}),
+		new webpack.optimize.CommonsChunkPlugin({
+			chunks: ["a", "a+b"],
+			async: "a",
+		}),
+	]
+};

Original file line number	Diff line number	Diff line change
@@ -340,7 +340,9 @@ Take a look at the "name"/"names" or async/children option.`);
`340`	`340`	`moveExtractedChunkBlocksToTargetChunk(chunks, targetChunk) {`
`341`	`341`	`for(let chunk of chunks) {`
`342`	`342`	`for(let block of chunk.blocks) {`
`343`		`- block.chunks.unshift(targetChunk);`
	`343`	`+ if(block.chunks.indexOf(targetChunk) === -1) {`
	`344`	`+ block.chunks.unshift(targetChunk);`
	`345`	`+ }`
`344`	`346`	`targetChunk.addBlock(block);`
`345`	`347`	`}`
`346`	`348`	`}`