[removed] transition.wait, use callbacks instead

This commit also removes our dependency on when.js and introduces a callback argument in transition hooks. Users are expected to call callback(error) when they're finished doing async stuff. If they don't have a callback in their argument list, we automatically call the callback for them for backwards compat. Fixes remix-run#273 Fixes remix-run#631
surajahmed · Dec 31, 2014 · c6ed6fa · c6ed6fa
1 parent a342d36
commit c6ed6fa
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 102 deletions.
diff --git a/docs/api/components/RouteHandler.md b/docs/api/components/RouteHandler.md
@@ -11,15 +11,16 @@ Static Lifecycle Methods
 You can define static methods on your route handlers that will be called
 during route transitions.
 
-### `willTransitionTo(transition, params, query)`
+### `willTransitionTo(transition, params, query, callback)`
 
 Called when a handler is about to render, giving you the opportunity to
 abort or redirect the transition. You can pause the transition while you
-do some asynchonous work with `transition.wait(promise)`.
+do some asynchonous work and call `callback(error)` when you're done, or
+omit the callback in your argument list and it will be called for you.
 
 See also: [transition](/docs/api/misc/transition.md)
 
-### `willTransitionFrom(transition, component)`
+### `willTransitionFrom(transition, component, callback)`
 
 Called when an active route is being transitioned out giving you an
 opportunity to abort the transition. The `component` is the current
@@ -34,13 +35,11 @@ See also: [transition](/docs/api/misc/transition.md)
 var Settings = React.createClass({
   statics: {
     willTransitionTo: function (transition, params) {
-      return auth.isLoggedIn().then(function (loggedIn) {
-        if (!loggedIn)
-          return;
+      if (!auth.isLoggedIn()) {
         transition.abort();
         auth.logIn({transition: transition});
         // in auth module call `transition.retry()` after being logged in
-      });
+      }
     },
 
     willTransitionFrom: function (transition, component) {

diff --git a/modules/__tests__/Router-test.js b/modules/__tests__/Router-test.js
@@ -35,8 +35,8 @@ describe('Router', function () {
       <Route path="/async" handler={Async}/>
     ];
 
-    describe('transition.wait', function () {
-      it('waits asynchronously in willTransitionTo', function (done) {
+    describe('asynchronous willTransitionTo', function () {
+      it('waits', function (done) {
         TestLocation.history = [ '/bar' ];
 
         var div = document.createElement('div');
@@ -70,7 +70,7 @@ describe('Router', function () {
         });
       });
 
-      it('stops waiting asynchronously in willTransitionTo on location.pop', function (done) {
+      it('stops waiting on location.pop', function (done) {
         TestLocation.history = [ '/bar' ];
 
         var div = document.createElement('div');
@@ -106,7 +106,7 @@ describe('Router', function () {
         });
       });
 
-      it('stops waiting asynchronously in willTransitionTo on router.transitionTo', function (done) {
+      it('stops waiting on router.transitionTo', function (done) {
         TestLocation.history = [ '/bar' ];
 
         var div = document.createElement('div');
@@ -149,7 +149,7 @@ describe('Router', function () {
         });
       });
 
-      it('stops waiting asynchronously in willTransitionTo on router.replaceWith', function (done) {
+      it('stops waiting on router.replaceWith', function (done) {
         TestLocation.history = [ '/bar' ];
 
         var div = document.createElement('div');
@@ -597,7 +597,7 @@ describe('Router', function () {
       var Bar = React.createClass({
         statics: {
           willTransitionFrom: function (transition, component) {
-            expect(div.querySelector('#bar')).toEqual(component.getDOMNode());
+            expect(div.querySelector('#bar')).toBe(component.getDOMNode());
             done();
           }
         },

diff --git a/modules/__tests__/TestHandlers.js b/modules/__tests__/TestHandlers.js
@@ -1,7 +1,6 @@
 var React = require('react');
 var RouteHandler = require('../components/RouteHandler');
 var State = require('../mixins/State');
-var delay = require('when/delay');
 
 exports.Nested = React.createClass({
   render: function () {
@@ -36,8 +35,8 @@ exports.Async = React.createClass({
   statics: {
     delay: 10,
 
-    willTransitionTo: function (transition) {
-      transition.wait(delay(this.delay));
+    willTransitionTo: function (transition, params, query, callback) {
+      setTimeout(callback, this.delay);
     }
   },
 
@@ -62,10 +61,11 @@ exports.RedirectToFooAsync = React.createClass({
   statics: {
     delay: 10,
 
-    willTransitionTo: function (transition) {
-      transition.wait(delay(this.delay).then(function () {
+    willTransitionTo: function (transition, params, query, callback) {
+      setTimeout(function () {
         transition.redirect('/foo');
-      }));
+        callback();
+      }, this.delay);
     }
   },
 
@@ -91,10 +91,11 @@ exports.AbortAsync = React.createClass({
   statics: {
     delay: 10,
 
-    willTransitionTo: function (transition) {
-      transition.wait(delay(this.delay).then(function () {
+    willTransitionTo: function (transition, params, query, callback) {
+      setTimeout(function () {
         transition.abort();
-      }));
+        callback();
+      }, this.delay);
     }
   },
 

diff --git a/modules/utils/Promise.js b/modules/utils/Promise.js
diff --git a/modules/utils/Transition.js b/modules/utils/Transition.js
@@ -1,37 +1,5 @@
 var assign = require('react/lib/Object.assign');
-var reversedArray = require('./reversedArray');
 var Redirect = require('./Redirect');
-var Promise = require('./Promise');
-
-/**
- * Runs all hook functions serially and calls callback(error) when finished.
- * A hook may return a promise if it needs to execute asynchronously.
- */
-function runHooks(hooks, callback) {
-  var promise;
-  try {
-    promise = hooks.reduce(function (promise, hook) {
-      // The first hook to use transition.wait makes the rest
-      // of the transition async from that point forward.
-      return promise ? promise.then(hook) : hook();
-    }, null);
-  } catch (error) {
-    return callback(error); // Sync error.
-  }
-
-  if (promise) {
-    // Use setTimeout to break the promise chain.
-    promise.then(function () {
-      setTimeout(callback);
-    }, function (error) {
-      setTimeout(function () {
-        callback(error);
-      });
-    });
-  } else {
-    callback();
-  }
-}
 
 /**
  * Calls the willTransitionFrom hook of all handlers in the given matches
@@ -40,23 +8,27 @@ function runHooks(hooks, callback) {
  * Calls callback(error) when finished.
  */
 function runTransitionFromHooks(transition, routes, components, callback) {
-  components = reversedArray(components);
-
-  var hooks = reversedArray(routes).map(function (route, index) {
-    return function () {
-      var handler = route.handler;
-
-      if (!transition.isAborted && handler.willTransitionFrom)
-        return handler.willTransitionFrom(transition, components[index]);
-
-      var promise = transition._promise;
-      transition._promise = null;
-
-      return promise;
+  var runHooks = routes.reduce(function (callback, route, index) {
+    return function (error) {
+      if (error || transition.isAborted) {
+        callback(error);
+      } else if (route.handler.willTransitionFrom) {
+        try {
+          route.handler.willTransitionFrom(transition, components[index], callback);
+
+          // If there is no callback in the argument list, call it automatically.
+          if (route.handler.willTransitionFrom.length < 3)
+            callback();
+        } catch (e) {
+          callback(e);
+        }
+      } else {
+        callback();
+      }
     };
-  });
+  }, callback);
 
-  runHooks(hooks, callback);
+  runHooks();
 }
 
 /**
@@ -65,21 +37,27 @@ function runTransitionFromHooks(transition, routes, components, callback) {
  * handler. Calls callback(error) when finished.
  */
 function runTransitionToHooks(transition, routes, params, query, callback) {
-  var hooks = routes.map(function (route) {
-    return function () {
-      var handler = route.handler;
-
-      if (!transition.isAborted && handler.willTransitionTo)
-        handler.willTransitionTo(transition, params, query);
-
-      var promise = transition._promise;
-      transition._promise = null;
-
-      return promise;
+  var runHooks = routes.reduceRight(function (callback, route) {
+    return function (error) {
+      if (error || transition.isAborted) {
+        callback(error);
+      } else if (route.handler.willTransitionTo) {
+        try {
+          route.handler.willTransitionTo(transition, params, query, callback);
+
+          // If there is no callback in the argument list, call it automatically.
+          if (route.handler.willTransitionTo.length < 4)
+            callback();
+        } catch (e) {
+          callback(e);
+        }
+      } else {
+        callback();
+      }
     };
-  });
+  }, callback);
 
-  runHooks(hooks, callback);
+  runHooks();
 }
 
 /**
@@ -93,7 +71,6 @@ function Transition(path, retry) {
   this.abortReason = null;
   this.isAborted = false;
   this.retry = retry.bind(this);
-  this._promise = null;
 }
 
 assign(Transition.prototype, {
@@ -112,10 +89,6 @@ assign(Transition.prototype, {
     this.abort(new Redirect(to, params, query));
   },
 
-  wait: function (value) {
-    this._promise = Promise.resolve(value);
-  },
-
   from: function (routes, components, callback) {
     return runTransitionFromHooks(this, routes, components, callback);
   },

diff --git a/modules/utils/createRouter.js b/modules/utils/createRouter.js
@@ -322,7 +322,7 @@ function createRouter(options) {
        * all route handlers we're transitioning to.
        *
        * Both willTransitionFrom and willTransitionTo hooks may either abort or redirect the
-       * transition. To resolve asynchronously, they may use transition.wait(promise). If no
+       * transition. To resolve asynchronously, they may use the callback argument. If no
        * hooks wait, the transition is fully synchronous.
        */
       dispatch: function (path, action, callback) {
@@ -374,7 +374,9 @@ function createRouter(options) {
         var transition = new Transition(path, this.replaceWith.bind(this, path));
         pendingTransition = transition;
 
-        transition.from(fromRoutes, components, function (error) {
+        var fromComponents = components.slice(prevRoutes.length - fromRoutes.length);
+
+        transition.from(fromRoutes, fromComponents, function (error) {
           if (error || transition.isAborted)
             return callback.call(router, error, transition);
 

diff --git a/modules/utils/reversedArray.js b/modules/utils/reversedArray.js
diff --git a/package.json b/package.json
@@ -47,8 +47,7 @@
     "react": "0.12.x"
   },
   "dependencies": {
-    "qs": "2.3.3",
-    "when": "3.4.6"
+    "qs": "2.3.3"
   },
   "tags": [
     "react",