Merge pull request #169 from chuckdumont/work2

Fix memory leak issues

Author: Chuck Dumont

lib/DojoAMDModuleFactoryPlugin.js

@@ -1,4 +1,5 @@
 /*
+/* (C) Copyright HCL Technologies Ltd. 2018
  * (C) Copyright IBM Corp. 2012, 2016 All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -43,14 +44,13 @@ module.exports = class DojoAMDModuleFactoryPlugin {
 				"resolver"                 : this.resolver,
 				"create-module"            : this.createModule
 			}, context);
-			tap(compiler, {"compilation" : (compilation, params) => {
-				tap(factory, "module", this.module, Object.create(context, {
-					compilation: {value: compilation},
-					params: {value: params}
-				}));
-			}});
-		}, this);
-	}
+		});
+		tap(compiler, {"compilation" : (compilation, params) => {
+			tap(params.normalModuleFactory, {
+				"module": this.module.bind(this, compilation, params.normalModuleFactory)
+			});
+		}});
+}
 
 	toAbsMid(request, issuerAbsMid, dojoRequire) {
 		var result = request;
@@ -210,29 +210,29 @@ module.exports = class DojoAMDModuleFactoryPlugin {
 		return module;
 	}
 
-	module(module) {
+	module(compilation, moduleFactory, module) {
 		if (module.originalRequest instanceof Module) {
 			// Copy absMids from original request to the new request
-			callSync(this.factory, "filter absMids", module.originalRequest, absMid => {
-				callSync(this.factory, "add absMid", module, absMid);
+			callSync(moduleFactory, "filter absMids", module.originalRequest, absMid => {
+				callSync(moduleFactory, "add absMid", module, absMid);
 				return true;
 			});
 		}
 		// If the module already exists in the compilation, then copy the absMid data from
 		// this module to the existing module since this module will be discarded
-		const existing = this.compilation.findModule(module.request);
+		const existing = compilation.findModule(module.request);
 		if (existing) {
-			callSync(this.factory, "filter absMids", module, absMid => {
-				callSync(this.factory, "add absMid", existing, absMid);
+			callSync(moduleFactory, "filter absMids", module, absMid => {
+				callSync(moduleFactory, "add absMid", existing, absMid);
 				return true;
 			});
 		}
 		// Add functions to the module object for adding/filtering absMids (for use by loaders)
 		module.addAbsMid = absMid => {
-			callSync(this.factory, "add absMid", module, absMid);
+			callSync(moduleFactory, "add absMid", module, absMid);
 		};
 		module.filterAbsMids = callback => {
-			callSync(this.factory, "filter absMids", module, callback);
+			callSync(moduleFactory, "filter absMids", module, callback);
 		};
 		return module;
 	}

lib/DojoAMDPlugin.js

@@ -1,4 +1,5 @@
 /*
+ * (C) Copyright HCL Technologies Ltd. 2018
  * (C) Copyright IBM Corp. 2012, 2016 All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -28,6 +29,7 @@ const DojoAMDMainTemplatePlugin = require("./DojoAMDMainTemplatePlugin");
 const DojoAMDChunkTemplatePlugin = require("./DojoAMDChunkTemplatePlugin");
 const DojoAMDModuleFactoryPlugin = require("./DojoAMDModuleFactoryPlugin");
 const DojoLoaderPlugin = require("./DojoLoaderPlugin");
+const DojoLoaderValidatorPlugin = require("./DojoLoaderValidatorPlugin");
 const DojoAMDRequireArrayDependency =  require("./DojoAMDRequireArrayDependency");
 const ScopedRequirePlugin = require("./ScopedRequirePlugin");
 const {applyResolverPlugin} = require("./compat");
@@ -45,6 +47,7 @@ module.exports = class DojoAMDPlugin {
 		tap(compiler, "dojo-webpack-plugin-options", () => this.options);
 		applyResolverPlugin(this.options, compiler, this.newResolverPlugin);
 		this.newDojoLoaderPlugin(this.options).apply(compiler);
+		this.newDojoLoaderValidatorPlugin(this.options).apply(compiler);
 		this.newMainTemplatePlugin(this.options).apply(compiler);
 		this.newChunkTemplatePlugin(this.options).apply(compiler);
 		this.newModuleFactoryPlugin(this.options).apply(compiler);
@@ -114,6 +117,9 @@ module.exports = class DojoAMDPlugin {
 	newDojoLoaderPlugin(options) {
 		return new DojoLoaderPlugin(options);
 	}
+	newDojoLoaderValidatorPlugin(options) {
+		return new DojoLoaderValidatorPlugin(options);
+	}
 	newMainTemplatePlugin(options) {
 		return new DojoAMDMainTemplatePlugin(options);
 	}

lib/DojoAMDResolverPluginV4.js

@@ -1,4 +1,5 @@
 /*
+ * (C) Copyright HCL Technologies Ltd. 2018
  * (C) Copyright IBM Corp. 2012, 2016 All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,12 +25,10 @@ module.exports = class DojoAMDResolverPluginV4 extends DojoAMDResolverPluginBase
 
 	apply(compiler) {
 		this.compiler = compiler;
-		tap(compiler, "compilation", () => {
-			tap(compiler.resolverFactory, "resolver normal", resolver => {
-				tap(resolver, "module", this.module, Object.create(this, {
-					resolver: {value: resolver}
-				}), {stage:-1});
-			});
+		tap(compiler.resolverFactory, "resolver normal", resolver => {
+			tap(resolver, "module", this.module, Object.create(this, {
+				resolver: {value: resolver}
+			}), {stage:-1});
 		});
 	}
 

lib/DojoLoaderPlugin.js

@@ -1,4 +1,5 @@
 /*
+ * (C) Copyright HCL Technologies Ltd. 2018
  * (C) Copyright IBM Corp. 2017 All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -25,7 +26,6 @@ const BasicEvaluatedExpression = require("webpack/lib/BasicEvaluatedExpression")
 
 const embeddedLoaderFilenameExpression = "__embedded_dojo_loader__";
 
-
 module.exports = class DojoLoaderPlugin {
 	constructor(options) {
 		this.options = options;
@@ -54,11 +54,6 @@ module.exports = class DojoLoaderPlugin {
 				compilation:{value: compilation},
 				params:{value: params}
 			});
-			if (compilation.compiler === compiler) {
-				// Don't do this for child compilations
-				//  https://github.com/OpenNTF/dojo-webpack-plugin/issues/115
-				tap(compiler, "make", this.validateEmbeddedLoader, context);
-			}
 			tap(compilation, [
 				["succeed-module",        this.succeedModule],
 				["after-optimize-chunks", this.afterOptimizeChunks]
@@ -171,28 +166,6 @@ module.exports = class DojoLoaderPlugin {
 		return loaderScope;
 	}
 
-	validateEmbeddedLoader(compilation__, callback) {
-		// Vefiry that embedded loader version and dojo version are the same
-		this.params.normalModuleFactory.create({
-			dependencies: [{request: "dojo/package.json"}]
-		}, (err, pkgModule) => {
-			if (!err) {
-				 const dojoVersion = require(pkgModule.request).version;
-				 const scope = callSyncBail(this.compiler, "create-embedded-loader-scope", {}, this.embeddedLoader, this.filename);
-				 if (dojoVersion !== scope.loaderVersion) {
-					 err = new Error(
-`Dojo loader version does not match the version of Dojo.
-Loader version = ${scope.loaderVersion}.
-Dojo version = ${dojoVersion}.
-You may need to rebuild the Dojo loader.
-Refer to https://github.com/OpenNTF/dojo-webpack-plugin/blob/master/README.md#building-the-dojo-loader`);
-				 }
-				 return callback(err, scope);
-			}
-			callback(err);
-		});
-	}
-
 	getBuildLoaderConfig() {
 		var loaderConfig = this.options.loaderConfig;
 		if (util.isString(loaderConfig)) {
@@ -339,4 +312,4 @@ Refer to https://github.com/OpenNTF/dojo-webpack-plugin/blob/master/README.md#bu
 	getDefaultFeaturesForEmbeddedLoader() {
 		return require("../buildDojo/loaderDefaultFeatures");
 	}
-};
+};

lib/DojoLoaderValidatorPlugin.js

@@ -0,0 +1,63 @@
+/*
+ * (C) Copyright HCL Technologies Ltd. 2018
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+const {tap, callSyncBail} = require("./pluginCompat").for("dojo-webpack-plugin");
+
+module.exports = class DojoLoaderValidatorPlugin {
+	constructor(options) {
+		this.options = options;
+	}
+
+	apply(compiler) {
+		this.compiler = compiler;
+		tap(compiler, {
+			"compile":               params => this.params = params,
+			"embedded-dojo-loader":  this.embeddedDojoLoader,
+			"make":                  this.validateEmbeddedLoader
+		}, this);
+	}
+
+	embeddedDojoLoader(content, filename) {
+		this.embeddedLoader = content;
+		this.embeddedLoaderFilename = filename;
+	}
+
+	validateEmbeddedLoader(compilation, callback) {
+		if (compilation.compiler !== this.compiler) {
+			// Don't do this for child compilations
+			//  https://github.com/OpenNTF/dojo-webpack-plugin/issues/115
+			return callback();
+		}
+		// Vefiry that embedded loader version and dojo version are the same
+		this.params.normalModuleFactory.create({
+			dependencies: [{request: "dojo/package.json"}]
+		}, (err, pkgModule) => {
+			if (!err) {
+				 const dojoVersion = require(pkgModule.request).version;
+				 const scope = callSyncBail(this.compiler, "create-embedded-loader-scope", {}, this.embeddedLoader, this.filename);
+				 if (dojoVersion !== scope.loaderVersion) {
+					 err = new Error(
+`Dojo loader version does not match the version of Dojo.
+Loader version = ${scope.loaderVersion}.
+Dojo version = ${dojoVersion}.
+You may need to rebuild the Dojo loader.
+Refer to https://github.com/OpenNTF/dojo-webpack-plugin/blob/master/README.md#building-the-dojo-loader`);
+				 }
+				 return callback(err, scope);
+			}
+			callback(err);
+		});
+	}
+};

test/DojoAMDModuleFactoryPlugin.test.js

@@ -41,7 +41,7 @@ describe("DojoAMDModuleFactoryPlugin tests", function() {
 		plugin.apply(compiler);
 		plugin.factory = factory;
 		callSync(compiler, "normal-module-factory", factory);
-		callSync(compiler, "compilation", compilation, {});
+		callSync(compiler, "compilation", compilation, {normalModuleFactory: factory});
 	});
 	describe("addAbsMid tests", function() {
 		it("should add the absMid", function() {
@@ -210,7 +210,6 @@ describe("DojoAMDModuleFactoryPlugin tests", function() {
 			const delegated = {originalRequest: originalModule};
 			callSync(factory, "add absMid", originalModule, "a");
 			compilation.findModule = function() { return null; };
-			debugger; // eslint-disable-line
 			const result = callSyncWaterfall(factory, "module", delegated);
 			result.should.be.eql(delegated);
 			result.absMid.should.eql('a');

test/DojoLoaderPlugin.test.js

@@ -60,23 +60,6 @@ describe("DojoLoaderPlugin tests", function() {
 			});
 		});
 	});
-	describe("validateEmbeddedLoader edge cases", function() {
-		it("Should invoke callback with error if nomralModuleFactory.create returns an error", function(done) {
-			var error = new Error("Failed to create module");
-			plugin.params = {
-				normalModuleFactory: {
-					create: function(params__, callback) {
-						callback(error);
-					}
-				}
-			};
-			plugin.embeddedLoader = plugin.filename = "";
-			plugin.validateEmbeddedLoader(null, err => {
-				err.should.be.eql(error);
-				done();
-			});
-		});
-	});
 	describe("compiler run1 edge cases", function() {
 		afterEach(function() {
 			delete plugin.getDojoPath;

test/DojoLoaderValidatorPlugin.test.js

@@ -0,0 +1,51 @@
+/*
+ * Tests to provide complete test coverage for DojoLoaderValidatorPlugin.  These
+ * tests exercise code paths that are difficult or impossible to invoke from
+ * within webpack.  As such, they provide only enough scafoliding to support
+ * execution of the targeted paths.  Code changes to the module under
+ * test may require additional scafolding in this file, even if the code
+ * changes are not related to the paths being tested.
+ */
+const {Tapable} = require("../lib/pluginCompat").for("DojoLoaderPlugin.test");
+
+const DojoLoaderValidatorPlugin = require("../lib/DojoLoaderValidatorPlugin");
+var plugin;
+describe("DojoLoaderValidatorPlugin tests", function() {
+	describe("validateEmbeddedLoader edge cases", function() {
+		var compiler;
+		beforeEach(function() {
+			plugin = new DojoLoaderValidatorPlugin({loaderConfig:{}, noConsole:true});
+			compiler = new Tapable();
+			compiler.context = '.';
+			plugin.compiler = compiler;
+		});
+		it("Should invoke callback with error if nomralModuleFactory.create returns an error", function(done) {
+			var error = new Error("Failed to create module");
+			plugin.compilation = {
+				compiler:plugin.compiler
+			};
+			plugin.params = {
+				normalModuleFactory: {
+					create: function(params__, callback) {
+						callback(error);
+					}
+				}
+			};
+			plugin.embeddedLoader = plugin.filename = "";
+			plugin.validateEmbeddedLoader(plugin.compilation, err => {
+				err.should.be.eql(error);
+				done();
+			});
+		});
+
+		it("Should invoke callback with no error for child compiler", function(done) {
+			plugin.compilation = {
+				compiler:{}
+			};
+			plugin.validateEmbeddedLoader(plugin.compilation, err => {
+				(typeof err === 'undefined').should.be.true;
+				done();
+			});
+		});
+	});
+});