Merge pull request #90 from ember-cli/stay-broken

Never return broken modules from require

lib/loader/loader.js

@@ -90,12 +90,21 @@ var loader, define, requireModule, require, requirejs;
     this.deps      = !deps.length && callback.length ? defaultDeps : deps;
     this.module    = { exports: {} };
     this.callback  = callback;
-    this.finalized = false;
     this.hasExportsAsDep = false;
     this.isAlias = alias;
     this.reified = new Array(deps.length);
-    this._foundDeps = false;
-    this.isPending = false;
+
+    /*
+       Each module normally passes through these states, in order:
+         new       : initial state
+         pending   : this module is scheduled to be executed
+         reifying  : this module's dependencies are being executed
+         reified   : this module's dependencies finished executing successfully
+         errored   : this module's dependencies failed to execute
+         finalized : this module executed successfully
+     */
+    this.state = 'new';
+
   }
 
   Module.prototype.makeDefaultExport = function() {
@@ -108,19 +117,19 @@ var loader, define, requireModule, require, requirejs;
   };
 
   Module.prototype.exports = function() {
-    if (this.finalized) { return this.module.exports; }
+    // if finalized, there is no work to do. If reifying, there is a
+    // circular dependency so we must return our (partial) exports.
+    if (this.state === 'finalized' || this.state === 'reifying') { return this.module.exports; }
     stats.exports++;
 
-    this.finalized = true;
-    this.isPending = false;
-
     if (loader.wrapModules) {
       this.callback = loader.wrapModules(this.name, this.callback);
     }
 
     this.reify();
 
     var result = this.callback.apply(this, this.reified);
+    this.state = 'finalized';
 
     if (!(this.hasExportsAsDep && result === undefined)) {
       this.module.exports = result;
@@ -130,29 +139,40 @@ var loader, define, requireModule, require, requirejs;
   };
 
   Module.prototype.unsee = function() {
-    this.finalized = false;
-    this._foundDeps = false;
-    this.isPending = false;
+    this.state = 'new';
     this.module = { exports: {} };
   };
 
   Module.prototype.reify = function() {
+    if (this.state === 'reified') { return; }
+    this.state = 'reifying';
+    try {
+      this.reified = this._reify();
+      this.state = 'reified';
+    } finally {
+      if (this.state === 'reifying') {
+        this.state = 'errored';
+      }
+    }
+  };
+
+  Module.prototype._reify = function() {
     stats.reify++;
-    var reified = this.reified;
+    var reified = this.reified.slice();
     for (var i = 0; i < reified.length; i++) {
       var mod = reified[i];
       reified[i] = mod.exports ? mod.exports : mod.module.exports();
     }
+    return reified;
   };
 
   Module.prototype.findDeps = function(pending) {
-    if (this._foundDeps) {
+    if (this.state !== 'new') {
       return;
     }
 
     stats.findDeps++;
-    this._foundDeps = true;
-    this.isPending = true;
+    this.state = 'pending';
 
     var deps = this.deps;
 
@@ -229,7 +249,7 @@ var loader, define, requireModule, require, requirejs;
 
     if (!mod) { missingModule(name, referrer); }
 
-    if (pending && !mod.finalized && !mod.isPending) {
+    if (pending && mod.state !== 'pending' && mod.state !== 'finalized') {
       mod.findDeps(pending);
       pending.push(mod);
       stats.pendingQueueLength++;

tests/all.js

@@ -1305,3 +1305,44 @@ test('require has a has method', function () {
     pendingQueueLength: 2
   });
 });
+
+test('broken modules are never returned', function() {
+  define('foo', function() {
+    throw new Error('I am a broken module');
+  });
+
+  throws(function() {
+    require('foo');
+  }, /I am a broken module/, 'The first time');
+
+  throws(function() {
+    require('foo');
+  }, /I am a broken module/, 'The second time');
+});
+
+test('modules with broken dependencies are never returned', function() {
+  define('foo', ['other'], function() {
+    throw new Error('I am a broken module');
+  });
+
+  define('valid-dep-before', function() {
+  });
+
+  define('valid-dep-after', function() {
+  });
+  define('other', function() {
+  });
+
+
+  define('bar', ['valid-dep-before', 'foo', 'valid-dep-after'], function() {
+  });
+
+
+  throws(function() {
+    require('bar');
+  }, /I am a broken module/, 'The first time');
+
+  throws(function() {
+    require('bar');
+  }, /I am a broken module/, 'The second time');
+});