The system behaves better when the modules don't exist or are invalid.

YuzuJS · Aug 8, 2011 · e985adc · e985adc
1 parent bce12f4
commit e985adc
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 27 deletions.
diff --git a/nobleModules.js b/nobleModules.js
@@ -236,6 +236,10 @@
         // An array of DOM elements for the <script /> tags we insert, so that when we reset the module loader, we can remove them.
         var scriptTagEls = [];
 
+        function isLoaded(uri) {
+            return document.head.querySelector('script[src^="' + uri + '"]') !== null;
+        }
+
         return {
             reset: function () {
                 loadingTracker.empty();
@@ -250,40 +254,40 @@
                 return loadingTracker.containsKey(uri);
             },
             load: function (uri, onLoaded, onError) {
-                if (document.head.querySelector('script[src^="' + uri + '"]') === null) {
-                    loadingTracker.set(uri, {});
+                if (isLoaded(uri)) {
+                    throw new Error("Tried to load script at " + uri + "; however, the script was already in the DOM.");
+                }
 
-                    var el = document.createElement("script");
+                loadingTracker.set(uri, {});
 
-                    function onCommon(callback) {
-                        loadingTracker.remove(uri);
+                var el = document.createElement("script");
 
-                        el.removeEventListener("load", onScriptLoad, false);
-                        el.removeEventListener("error", onScriptError, false);
+                function onCommon(callback) {
+                    loadingTracker.remove(uri);
 
-                        if (typeof callback === "function") {
-                            callback();
-                        }
-                    }
-                    function onScriptLoad() {
-                        onCommon(onLoaded);
-                    }
-                    function onScriptError() {
-                        onCommon(onError);
+                    el.removeEventListener("load", onScriptLoad, false);
+                    el.removeEventListener("error", onScriptError, false);
+
+                    if (typeof callback === "function") {
+                        callback();
                     }
+                }
+                function onScriptLoad() {
+                    onCommon(onLoaded);
+                }
+                function onScriptError() {
+                    onCommon(onError);
+                }
 
-                    el.addEventListener("load", onScriptLoad, false);
-                    el.addEventListener("error", onScriptError, false);
+                el.addEventListener("load", onScriptLoad, false);
+                el.addEventListener("error", onScriptError, false);
 
-                    // If specified, we want to prevent caching, so append a timestamp to the URI. Separate it with underscores so that when
-                    // debugging you can visually separate the filename (which you care about) from the timestamp (which you don't care about).
-                    el.src = debugOptions.disableCaching ? uri + "?___________________________________" + Date.now() : uri;
+                // If specified, we want to prevent caching, so append a timestamp to the URI. Separate it with underscores so that when
+                // debugging you can visually separate the filename (which you care about) from the timestamp (which you don't care about).
+                el.src = debugOptions.disableCaching ? uri + "?___________________________________" + Date.now() : uri;
 
-                    document.head.appendChild(el);
-                    scriptTagEls.push(el);
-                } else {
-                    throw new Error("Tried to load script at " + uri + "; however, the script was already in the DOM.");
-                }
+                document.head.appendChild(el);
+                scriptTagEls.push(el);
             }
         };
     } ());
@@ -512,6 +516,7 @@
                 // The module isn't provided though, so for users of the module loader, an error will be encountered
                 // when they try to require it.
                 onModuleLoaded();
+                loadListeners.trigger(id);
             }
         );
     }
@@ -553,7 +558,7 @@
                     // (b) other module-related things happened after module.declare inside the file.
                     // In both cases: BAD module author! BAD!
                     callOnDependencyProvided();
-                    throw new Error('module.declare was not used inside the module with ID "' + id + '", or was not the sole statement.');
+                    warn('Requested module with "' + id + '", but it was not a valid module file.');
                 }
             }
 

diff --git a/test/moduleNamespace.js b/test/moduleNamespace.js
@@ -63,6 +63,28 @@ asyncModuleTest("load: when called twice in a row for the same module, both call
     });
 });
 
+asyncModuleTest("load: when called twice in a row for the same nonextant module, both callbacks fire", function (require, exports, module) {
+    var numberOfLoadsSoFar = 0;
+
+    module.load("asdf", function onLoad1() {
+        ok(true, "First callback");
+
+        ++numberOfLoadsSoFar;
+        if (numberOfLoadsSoFar === 2) {
+            start();
+        }
+    });
+
+    module.load("asdf", function onLoad2() {
+        ok(true, "Second callback");
+
+        ++numberOfLoadsSoFar;
+        if (numberOfLoadsSoFar === 2) {
+            start();
+        }
+    });
+});
+
 asyncModuleTest("load: does not memoize the loaded module", function (require, exports, module) {
     module.load("demos/vader", function onModuleLoaded() {
         strictEqual(require.isMemoized("demos/vader"), false, "demos/vader was not memoized");
@@ -106,6 +128,28 @@ asyncModuleTest("provide: still calls the callback even if one of the modules in
     });
 });
 
+asyncModuleTest("provide: two calls in a row for a nonextant module still results in both callbacks being called", function (require, exports, module) {
+    var numberOfLoadsSoFar = 0;
+
+    module.provide(["asdf"], function onLoad1() {
+        ok(true, "First callback");
+
+        ++numberOfLoadsSoFar;
+        if (numberOfLoadsSoFar === 2) {
+            start();
+        }
+    });
+
+    module.provide(["asdf"], function onLoad2() {
+        ok(true, "Second callback");
+
+        ++numberOfLoadsSoFar;
+        if (numberOfLoadsSoFar === 2) {
+            start();
+        }
+    });
+});
+
 // See http://groups.google.com/group/commonjs/browse_thread/thread/50d4565bd07e03cb
 asyncModuleTest("provide: does not modify module.dependencies", function (require, exports, module) {
     module.provide(["demos/restaurants"], function onModulesProvided() {