Refactor addModule to keep Module instances between rebuilds

sokra · sokra · commit 2bb95a3b93d1 · 2017-12-12T16:57:50.000+01:00
diff --git a/lib/Compilation.js b/lib/Compilation.js
@@ -226,8 +226,14 @@ class Compilation extends Tapable {
 
 	addModule(module, cacheGroup) {
 		const identifier = module.identifier();
-		if(this._modules.get(identifier)) {
-			return false;
+		const alreadyAddedModule = this._modules.get(identifier);
+		if(alreadyAddedModule) {
+			return {
+				module: alreadyAddedModule,
+				issuer: false,
+				build: false,
+				dependencies: false
+			};
 		}
 		const cacheName = (cacheGroup || "m") + identifier;
 		if(this.cache && this.cache[cacheName]) {
@@ -244,16 +250,27 @@ class Compilation extends Tapable {
 				this.modules.push(cacheModule);
 				cacheModule.errors.forEach(err => this.errors.push(err));
 				cacheModule.warnings.forEach(err => this.warnings.push(err));
-				return cacheModule;
+				return {
+					module: cacheModule,
+					issuer: true,
+					build: false,
+					dependencies: true
+				};
 			}
-			module.unbuild();
+			cacheModule.unbuild();
+			module = cacheModule;
 		}
 		this._modules.set(identifier, module);
 		if(this.cache) {
 			this.cache[cacheName] = module;
 		}
 		this.modules.push(module);
-		return true;
+		return {
+			module: module,
+			issuer: true,
+			build: true,
+			dependencies: true
+		};
 	}
 
 	getModule(module) {
@@ -371,6 +388,7 @@ class Compilation extends Tapable {
 	addModuleDependencies(module, dependencies, bail, cacheGroup, recursive, callback) {
 		let _this = this;
 		const start = _this.profile && Date.now();
+		const currentProfile = _this.profile && {};
 
 		asyncLib.forEach(dependencies, (item, callback) => {
 			const dependencies = item.dependencies;
@@ -420,14 +438,6 @@ class Compilation extends Tapable {
 						}
 					};
 
-					const iterationDependencies = depend => {
-						for(let index = 0; index < depend.length; index++) {
-							const dep = depend[index];
-							dep.module = dependentModule;
-							dependentModule.addReason(module, dep);
-						}
-					};
-
 					if(err) {
 						semaphore.release();
 						return errorOrWarningAndCallback(new ModuleNotFoundError(module, err, dependencies));
@@ -436,94 +446,83 @@ class Compilation extends Tapable {
 						semaphore.release();
 						return process.nextTick(callback);
 					}
-					if(_this.profile) {
-						if(!dependentModule.profile) {
-							dependentModule.profile = {};
-						}
+					if(currentProfile) {
 						afterFactory = Date.now();
-						dependentModule.profile.factory = afterFactory - start;
+						currentProfile.factory = afterFactory - start;
 					}
 
-					const newModule = _this.addModule(dependentModule, cacheGroup);
-
-					if(!newModule) { // from cache
-						dependentModule = _this.getModule(dependentModule);
-
-						iterationDependencies(dependencies);
-
-						if(_this.profile) {
-							if(!module.profile) {
-								module.profile = {};
-							}
-							const time = Date.now() - start;
-							if(!module.profile.dependencies || time > module.profile.dependencies) {
-								module.profile.dependencies = time;
-							}
+					const iterationDependencies = depend => {
+						for(let index = 0; index < depend.length; index++) {
+							const dep = depend[index];
+							dep.module = dependentModule;
+							dependentModule.addReason(module, dep);
 						}
+					};
 
-						semaphore.release();
-						_this.waitForBuildingFinished(dependentModule, callback);
-						return;
-					}
+					const addModuleResult = _this.addModule(dependentModule, cacheGroup);
+					dependentModule = addModuleResult.module;
+					iterationDependencies(dependencies);
 
-					if(newModule instanceof Module) {
-						if(_this.profile) {
-							newModule.profile = dependentModule.profile;
+					const afterBuild = () => {
+						if(currentProfile) {
+							const afterBuilding = Date.now();
+							currentProfile.building = afterBuilding - afterFactory;
 						}
 
-						newModule.issuer = module;
-						dependentModule = newModule;
-
-						iterationDependencies(dependencies);
+						if(recursive && addModuleResult.dependencies) {
+							_this.processModuleDependencies(dependentModule, callback);
+						} else {
+							return callback();
+						}
+					};
 
-						if(_this.profile) {
-							const afterBuilding = Date.now();
-							module.profile.building = afterBuilding - afterFactory;
+					if(addModuleResult.issuer) {
+						if(currentProfile) {
+							dependentModule.profile = currentProfile;
 						}
 
-						semaphore.release();
-						if(recursive) {
-							return process.nextTick(_this.processModuleDependencies.bind(_this, dependentModule, callback));
-						} else {
-							return process.nextTick(callback);
+						dependentModule.issuer = module;
+					} else {
+						if(_this.profile) {
+							if(module.profile) {
+								const time = Date.now() - start;
+								if(!module.profile.dependencies || time > module.profile.dependencies) {
+									module.profile.dependencies = time;
+								}
+							}
 						}
 					}
 
-					dependentModule.issuer = module;
+					if(addModuleResult.build) {
+						_this.buildModule(dependentModule, isOptional(), module, dependencies, err => {
+							if(_this === null) return semaphore.release();
 
-					iterationDependencies(dependencies);
+							if(err) {
+								semaphore.release();
+								return errorOrWarningAndCallback(err);
+							}
 
-					_this.buildModule(dependentModule, isOptional(), module, dependencies, err => {
-						if(_this === null) return semaphore.release();
+							if(currentProfile) {
+								const afterBuilding = Date.now();
+								currentProfile.building = afterBuilding - afterFactory;
+							}
 
-						if(err) {
 							semaphore.release();
-							return errorOrWarningAndCallback(err);
-						}
-
-						if(_this.profile) {
-							const afterBuilding = Date.now();
-							dependentModule.profile.building = afterBuilding - afterFactory;
-						}
-
+							afterBuild();
+						});
+					} else {
 						semaphore.release();
-						if(recursive) {
-							_this.processModuleDependencies(dependentModule, callback);
-						} else {
-							return callback();
-						}
-					});
-
+						_this.waitForBuildingFinished(dependentModule, afterBuild);
+					}
 				});
 			});
 		}, err => {
 			// In V8, the Error objects keep a reference to the functions on the stack. These warnings &
 			// errors are created inside closures that keep a reference to the Compilation, so errors are
-			// leaking the Compilation object. Setting _this to null workarounds the following issue in V8.
-			// https://bugs.chromium.org/p/chromium/issues/detail?id=612191
-			_this = null;
+			// leaking the Compilation object.
 
 			if(err) {
+				err.stack = err.stack;
 				return callback(err);
 			}
 
@@ -533,6 +532,7 @@ class Compilation extends Tapable {
 
 	_addModuleChain(context, dependency, onModule, callback) {
 		const start = this.profile && Date.now();
+		const currentProfile = this.profile && {};
 
 		const errorAndCallback = this.bail ? (err) => {
 			callback(err);
@@ -567,81 +567,60 @@ class Compilation extends Tapable {
 
 				let afterFactory;
 
-				if(this.profile) {
-					if(!module.profile) {
-						module.profile = {};
-					}
+				if(currentProfile) {
 					afterFactory = Date.now();
-					module.profile.factory = afterFactory - start;
+					currentProfile.factory = afterFactory - start;
 				}
 
-				const result = this.addModule(module);
-				if(!result) {
-					module = this.getModule(module);
+				const addModuleResult = this.addModule(module);
+				module = addModuleResult.module;
 
-					onModule(module);
+				onModule(module);
 
-					dependency.module = module;
-					module.addReason(null, dependency);
+				dependency.module = module;
+				module.addReason(null, dependency);
 
-					if(this.profile) {
+				const afterBuild = () => {
+					if(currentProfile) {
 						const afterBuilding = Date.now();
-						module.profile.building = afterBuilding - afterFactory;
+						currentProfile.building = afterBuilding - afterFactory;
 					}
 
-					this.semaphore.release();
-					this.waitForBuildingFinished(module, err => {
-						if(err) return callback(err);
-						callback(null, module);
-					});
-					return;
-				}
-
-				const moduleReady = () => {
-					this.semaphore.release();
-					this.processModuleDependencies(module, err => {
-						if(err) {
-							return callback(err);
-						}
-
+					if(addModuleResult.dependencies) {
+						this.processModuleDependencies(module, err => {
+							if(err) return callback(err);
+							callback(null, module);
+						});
+					} else {
 						return callback(null, module);
-					});
+					}
 				};
 
-				if(result instanceof Module) {
-					if(this.profile) {
-						result.profile = module.profile;
+				if(addModuleResult.issuer) {
+					if(currentProfile) {
+						module.profile = currentProfile;
 					}
-
-					module = result;
-
-					onModule(module);
-
-					dependency.module = module;
-					module.addReason(null, dependency);
-
-					moduleReady();
-					return;
 				}
 
-				onModule(module);
+				if(addModuleResult.build) {
+					this.buildModule(module, false, null, null, err => {
+						if(err) {
+							this.semaphore.release();
+							return errorAndCallback(err);
+						}
 
-				dependency.module = module;
-				module.addReason(null, dependency);
+						if(currentProfile) {
+							const afterBuilding = Date.now();
+							currentProfile.building = afterBuilding - afterFactory;
+						}
 
-				this.buildModule(module, false, null, null, err => {
-					if(err) {
 						this.semaphore.release();
-						return errorAndCallback(err);
-					}
-
-					if(this.profile) {
-						const afterBuilding = Date.now();
-						module.profile.building = afterBuilding - afterFactory;
-					}
-
-					moduleReady();
-				});
+						afterBuild();
+					});
+				} else {
+					this.semaphore.release();
+					this.waitForBuildingFinished(module, afterBuild);
+				}
 			});
 		});
 	}
