remove the need to rewrite chunk in reasons

sokra · ooflorent · commit f24e389f2dbc · 2018-03-06T15:27:08.000+01:00
diff --git a/lib/Chunk.js b/lib/Chunk.js
@@ -200,7 +200,6 @@ class Chunk {
 	moveModule(module, otherChunk) {
 		GraphHelpers.disconnectChunkAndModule(this, module);
 		GraphHelpers.connectChunkAndModule(otherChunk, module);
-		module.rewriteChunkInReasons(this, [otherChunk]);
 	}
 
 	integrate(otherChunk, reason) {
diff --git a/lib/Module.js b/lib/Module.js
@@ -64,9 +64,6 @@ class Module extends DependenciesBlock {
 		this.used = null;
 		this.usedExports = null;
 		this.optimizationBailout = [];
-
-		// delayed operations
-		this._rewriteChunkInReasons = undefined;
 	}
 
 	get exportsArgument() {
@@ -82,7 +79,6 @@ class Module extends DependenciesBlock {
 		this.renderedHash = undefined;
 
 		this.reasons.length = 0;
-		this._rewriteChunkInReasons = undefined;
 		this._chunks.clear();
 
 		this.id = null;
@@ -187,36 +183,48 @@ class Module extends DependenciesBlock {
 		return false;
 	}
 
-	hasReasonForChunk(chunk) {
-		if (this._rewriteChunkInReasons) {
-			for (const operation of this._rewriteChunkInReasons)
-				this._doRewriteChunkInReasons(operation.oldChunk, operation.newChunks);
-			this._rewriteChunkInReasons = undefined;
-		}
-		for (let i = 0; i < this.reasons.length; i++) {
-			if (this.reasons[i].hasChunk(chunk)) return true;
+	isAccessibleInChunk(chunk, ignoreChunk) {
+		// Check if module is accessible in ALL chunk groups
+		for (const chunkGroup of chunk.groupsIterable) {
+			if (!this.isAccessibleInChunkGroup(chunkGroup)) return false;
 		}
-		return false;
+		return true;
 	}
 
-	hasReasons() {
-		return this.reasons.length > 0;
-	}
+	isAccessibleInChunkGroup(chunkGroup, ignoreChunk) {
+		const queue = new Set([chunkGroup]);
 
-	rewriteChunkInReasons(oldChunk, newChunks) {
-		// This is expensive. Delay operation until we really need the data
-		if (this._rewriteChunkInReasons === undefined)
-			this._rewriteChunkInReasons = [];
-		this._rewriteChunkInReasons.push({
-			oldChunk,
-			newChunks
-		});
+		// Check if module is accessible from all items of the queue
+		queueFor: for (const cg of queue) {
+			// 1. If module is in one of the chunks of the group we can continue checking the next items
+			//    because it's accessible.
+			for (const chunk of cg.chunks) {
+				if (chunk !== ignoreChunk && chunk.containsModule(this))
+					continue queueFor;
+			}
+			// 2. If the chunk group is initial, we can break here because it's not accessible.
+			if (chunkGroup.isInitial()) return false;
+			// 3. Enqueue all parents because it must be accessible from ALL parents
+			for (const parent of chunkGroup.parentsIterable) queue.add(parent);
+		}
+		// When we processed through the whole list and we didn't bailout, the module is accessible
+		return true;
 	}
 
-	_doRewriteChunkInReasons(oldChunk, newChunks) {
-		for (let i = 0; i < this.reasons.length; i++) {
-			this.reasons[i].rewriteChunks(oldChunk, newChunks);
+	hasReasonForChunk(chunk) {
+		// check for each reason if we need the chunk
+		for (const reason of this.reasons) {
+			const fromModule = reason.module;
+			for (const originChunk of fromModule.chunksIterable) {
+				// return true if module this is not reachable from originChunk when ignoring cunk
+				if (!this.isAccessibleInChunk(originChunk, chunk)) return true;
+			}
 		}
+		return false;
+	}
+
+	hasReasons() {
+		return this.reasons.length > 0;
 	}
 
 	isUsed(exportName) {
diff --git a/lib/ModuleReason.js b/lib/ModuleReason.js
@@ -9,32 +9,21 @@ class ModuleReason {
 		this.module = module;
 		this.dependency = dependency;
 		this.explanation = explanation;
-		this._chunks = null;
-	}
-
-	hasChunk(chunk) {
-		if (this._chunks) {
-			if (this._chunks.has(chunk)) return true;
-		} else if (this.module && this.module._chunks.has(chunk)) return true;
-		return false;
 	}
+}
 
-	rewriteChunks(oldChunk, newChunks) {
-		if (!this._chunks) {
-			if (this.module) {
-				if (!this.module._chunks.has(oldChunk)) return;
-				this._chunks = new Set(this.module._chunks);
-			} else {
-				this._chunks = new Set();
-			}
-		}
-		if (this._chunks.has(oldChunk)) {
-			this._chunks.delete(oldChunk);
-			for (let i = 0; i < newChunks.length; i++) {
-				this._chunks.add(newChunks[i]);
-			}
-		}
+Object.defineProperty(ModuleReason.prototype, "chunks", {
+	configurable: false,
+	get() {
+		throw new Error(
+			"ModuleReason.chunks: This was removed without replacement"
+		);
+	},
+	set() {
+		throw new Error(
+			"ModuleReason.chunks: This was removed without replacement"
+		);
 	}
-}
+});
 
 module.exports = ModuleReason;
diff --git a/lib/optimize/RemoveParentModulesPlugin.js b/lib/optimize/RemoveParentModulesPlugin.js
@@ -7,23 +7,6 @@
 const Queue = require("../util/Queue");
 const intersect = require("../util/SetHelpers").intersect;
 
-const getParentChunksWithModule = (currentChunk, module) => {
-	const chunks = [];
-	const stack = new Set(currentChunk.parentsIterable);
-
-	for (const chunk of stack) {
-		if (chunk.containsModule(module)) {
-			chunks.push(chunk);
-		} else {
-			for (const parent of chunk.parentsIterable) {
-				stack.add(parent);
-			}
-		}
-	}
-
-	return chunks;
-};
-
 class RemoveParentModulesPlugin {
 	apply(compiler) {
 		compiler.hooks.compilation.tap("RemoveParentModulesPlugin", compilation => {
@@ -92,10 +75,6 @@ class RemoveParentModulesPlugin {
 							if (chunk.containsModule(m)) toRemove.add(m);
 					}
 					for (const module of toRemove) {
-						module.rewriteChunkInReasons(
-							chunk,
-							getParentChunksWithModule(chunk, module)
-						);
 						chunk.removeModule(module);
 					}
 				}
diff --git a/lib/optimize/SplitChunksPlugin.js b/lib/optimize/SplitChunksPlugin.js
@@ -421,7 +421,6 @@ module.exports = class SplitChunksPlugin {
 							// Remove all selected modules from the chunk
 							for (const module of item.modules) {
 								chunk.removeModule(module);
-								module.rewriteChunkInReasons(chunk, [newChunk]);
 							}
 						}
 						// If we successfully created a new chunk or reused one
diff --git a/test/ModuleReason.unittest.js b/test/ModuleReason.unittest.js

Original file line number	Diff line number	Diff line change
`@@ -200,7 +200,6 @@ class Chunk {`
`200`	`200`	`moveModule(module, otherChunk) {`
`201`	`201`	`GraphHelpers.disconnectChunkAndModule(this, module);`
`202`	`202`	`GraphHelpers.connectChunkAndModule(otherChunk, module);`
`203`		`- module.rewriteChunkInReasons(this, [otherChunk]);`
`204`	`203`	`}`
`205`	`204`
`206`	`205`	`integrate(otherChunk, reason) {`
Original file line number	Diff line number	Diff line change
`@@ -421,7 +421,6 @@ module.exports = class SplitChunksPlugin {`
`421`	`421`	`// Remove all selected modules from the chunk`
`422`	`422`	`for (const module of item.modules) {`
`423`	`423`	`chunk.removeModule(module);`
`424`		`- module.rewriteChunkInReasons(chunk, [newChunk]);`
`425`	`424`	`}`
`426`	`425`	`}`
`427`	`426`	`// If we successfully created a new chunk or reused one`