Merge pull request #1273 from stealjs/loads-twice-warning

Warn if a module is loaded at the same path twice

Gruntfile.js

@@ -92,6 +92,7 @@ module.exports = function (grunt) {
 					"src/system-extension-contextual.js",
 					"src/system-extension-script-module.js",
 					"src/system-extension-steal.js",
+					"src/system-extension-module-loaded-twice.js",
 					"src/trace/trace.js",
 					"src/json/json.js",
 					"src/cache-bust/cache-bust.js",
@@ -117,6 +118,7 @@ module.exports = function (grunt) {
 					"src/system-extension-contextual.js",
 					"src/system-extension-script-module.js",
 					"src/system-extension-steal.js",
+					"src/system-extension-module-loaded-twice.js",
 					"src/trace/trace.js",
 					"src/json/json.js",
 					"src/cache-bust/cache-bust.js",

src/system-extension-module-loaded-twice.js

@@ -0,0 +1,48 @@
+/**
+ * Extension to warn users when a module is instantiated twice
+ *
+ * Multiple module instantiation might cause unexpected side effects
+ */
+addStealExtension(function(loader) {
+	var superInstantiate = loader.instantiate;
+
+	var warn = typeof console === "object" ?
+		Function.prototype.bind.call(console.warn, console) :
+		null;
+
+	loader._instantiatedModules = loader._instantiatedModules || {};
+
+	loader.instantiate = function(load) {
+		var instantiated = loader._instantiatedModules;
+
+		if (warn && instantiated[load.address]) {
+			var loads = (loader._traceData && loader._traceData.loads) || {};
+			var map = (loader._traceData && loader._traceData.parentMap) || {};
+
+			var parents = (map[load.name] ? Object.keys(map[load.name]) : [])
+				.map(function(parent) {
+					// module names might confuse people
+					return "\t " + loads[parent].address;
+				})
+				.join("\n");
+
+			warn(
+				[
+					"The module with address " + load.address +
+						" is being instantiated twice",
+					"This happens when module identifiers normalize to different module names.\n",
+					"HINT: Import the module using the ~/[modulePath] identifier" +
+						(parents ? " in " : ""),
+					(parents || "") + "\n",
+					"Learn more at https://stealjs.com/docs/moduleName.html and " +
+						"https://stealjs.com/docs/tilde.html"
+				].join("\n")
+			);
+		} else {
+			instantiated[load.address] = load;
+		}
+
+		return superInstantiate.apply(loader, arguments);
+	};
+});
+

steal-sans-promises.js

@@ -4982,6 +4982,55 @@ addStealExtension(function (loader) {
     return loaderInstantiate.call(loader, load);
   };
 });
+/**
+ * Extension to warn users when a module is instantiated twice
+ *
+ * Multiple module instantiation might cause unexpected side effects
+ */
+addStealExtension(function(loader) {
+	var superInstantiate = loader.instantiate;
+
+	var warn = typeof console === "object" ?
+		Function.prototype.bind.call(console.warn, console) :
+		null;
+
+	loader._instantiatedModules = loader._instantiatedModules || {};
+
+	loader.instantiate = function(load) {
+		var instantiated = loader._instantiatedModules;
+
+		if (warn && instantiated[load.address]) {
+			var loads = (loader._traceData && loader._traceData.loads) || {};
+			var map = (loader._traceData && loader._traceData.parentMap) || {};
+
+			var parents = (map[load.name] ? Object.keys(map[load.name]) : [])
+				.map(function(parent) {
+					// module names might confuse people
+					return "\t " + loads[parent].address;
+				})
+				.join("\n");
+
+			warn(
+				[
+					"The module with address " + load.address +
+						" is being instantiated twice",
+					"This happens when module identifiers normalize to different module names.\n",
+					"HINT: Import the module using the ~/[modulePath] identifier" +
+						(parents ? " in " : ""),
+					(parents || "") + "\n",
+					"Learn more at https://stealjs.com/docs/moduleName.html and " +
+						"https://stealjs.com/docs/tilde.html"
+				].join("\n")
+			);
+		} else {
+			instantiated[load.address] = load;
+		}
+
+		return superInstantiate.apply(loader, arguments);
+	};
+});
+
+
 addStealExtension(function applyTraceExtension(loader) {
 	if(loader._extensions) {
 		loader._extensions.push(applyTraceExtension);

steal.js

@@ -6252,6 +6252,55 @@ addStealExtension(function (loader) {
     return loaderInstantiate.call(loader, load);
   };
 });
+/**
+ * Extension to warn users when a module is instantiated twice
+ *
+ * Multiple module instantiation might cause unexpected side effects
+ */
+addStealExtension(function(loader) {
+	var superInstantiate = loader.instantiate;
+
+	var warn = typeof console === "object" ?
+		Function.prototype.bind.call(console.warn, console) :
+		null;
+
+	loader._instantiatedModules = loader._instantiatedModules || {};
+
+	loader.instantiate = function(load) {
+		var instantiated = loader._instantiatedModules;
+
+		if (warn && instantiated[load.address]) {
+			var loads = (loader._traceData && loader._traceData.loads) || {};
+			var map = (loader._traceData && loader._traceData.parentMap) || {};
+
+			var parents = (map[load.name] ? Object.keys(map[load.name]) : [])
+				.map(function(parent) {
+					// module names might confuse people
+					return "\t " + loads[parent].address;
+				})
+				.join("\n");
+
+			warn(
+				[
+					"The module with address " + load.address +
+						" is being instantiated twice",
+					"This happens when module identifiers normalize to different module names.\n",
+					"HINT: Import the module using the ~/[modulePath] identifier" +
+						(parents ? " in " : ""),
+					(parents || "") + "\n",
+					"Learn more at https://stealjs.com/docs/moduleName.html and " +
+						"https://stealjs.com/docs/tilde.html"
+				].join("\n")
+			);
+		} else {
+			instantiated[load.address] = load;
+		}
+
+		return superInstantiate.apply(loader, arguments);
+	};
+});
+
+
 addStealExtension(function applyTraceExtension(loader) {
 	if(loader._extensions) {
 		loader._extensions.push(applyTraceExtension);

test/load_module_twice/dev.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+	<meta charset="UTF-8">
+	<title>module is loaded twice with different paths</title>
+</head>
+<body>
+	<script>
+		window.assert = window.parent.assert;
+		window.done = window.parent.done;
+
+		window.WARN = Function.prototype.bind.call(
+			window.console.warn,
+			window.console
+		);
+
+		window.console.warn = function(msg) {
+			window.assert.ok(
+				/is being instantiated twice/.test(msg),
+				"steal should warn users when module is instantiated twice"
+			);
+			window.WARN(msg);
+			window.done();
+		};
+	</script>
+	<script src="../../../steal-sans-promises.js" base-url="." config="package.json!npm">
+
+	</script>
+</body>
+</html>

test/load_module_twice/foo.js

@@ -0,0 +1 @@
+module.exports = {};

test/load_module_twice/main.js

@@ -0,0 +1,7 @@
+var foo = require("./foo");
+var foo2 = require("foo");
+
+if (!window || !window.assert) {
+	console.log("foo: ", foo);
+	console.log("foo2: ", foo2);
+}

test/load_module_twice/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "load_module_twice",
+  "version": "0.0.1",
+  "main": "main.js"
+}

test/test.js

@@ -242,6 +242,10 @@ if (hasConsole) {
 	QUnit.test("warn in production when main is not set (#537)", function(assert) {
 		makeIframe("basics/no_main_warning.html", assert);
 	});
+
+	QUnit.test("warns when module is loaded twice with different paths", function(assert) {
+		makeIframe("load_module_twice/dev.html", assert);
+	});
 }
 
 QUnit.test("can load a bundle with an amd module depending on a global", function(assert) {