Fixed everything

[removed] <Routes onTransitionError>
[removed] <Routes onAbortedTransition>

Author: mjackson

modules/actions/LocationActions.js

@@ -21,12 +21,7 @@ var LocationActions = {
   /**
    * Indicates the most recent entry should be removed from the history stack.
    */
-  POP: 'pop',
-
-  /**
-   * Indicates that a route transition is finished.
-   */
-  FINISHED_TRANSITION: 'finished-transition'
+  POP: 'pop'
 
 };
 

modules/components/Routes.js

@@ -2,10 +2,7 @@ var React = require('react');
 var warning = require('react/lib/warning');
 var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM;
 var copyProperties = require('react/lib/copyProperties');
-var LocationActions = require('../actions/LocationActions');
-var LocationDispatcher = require('../dispatchers/LocationDispatcher');
 var PathStore = require('../stores/PathStore');
-var ScrollStore = require('../stores/ScrollStore');
 var reversedArray = require('../utils/reversedArray');
 var Transition = require('../utils/Transition');
 var Redirect = require('../utils/Redirect');
@@ -265,13 +262,6 @@ function computeHandlerProps(matches, query) {
 
 var BrowserTransitionHandling = {
 
-  handleTransition: function (transition) {
-    LocationDispatcher.handleViewAction({
-      type: LocationActions.FINISHED_TRANSITION,
-      path: transition.path
-    });
-  },
-
   handleTransitionError: function (error) {
     throw error; // This error probably originated in a transition hook.
   },
@@ -290,10 +280,6 @@ var BrowserTransitionHandling = {
 
 var ServerTransitionHandling = {
 
-  handleTransition: function (transition) {
-    // TODO
-  },
-
   handleTransitionError: function (error) {
     // TODO
   },
@@ -330,20 +316,9 @@ var Routes = React.createClass({
   mixins: [ ActiveContext, LocationContext, RouteContext, ScrollContext ],
 
   propTypes: {
-    onTransition: React.PropTypes.func.isRequired,
-    onTransitionError: React.PropTypes.func.isRequired,
-    onAbortedTransition: React.PropTypes.func.isRequired,
     initialPath: React.PropTypes.string
   },
 
-  getDefaultProps: function () {
-    return {
-      onTransition: TransitionHandling.handleTransition,
-      onTransitionError: TransitionHandling.handleTransitionError,
-      onAbortedTransition: TransitionHandling.handleAbortedTransition
-    };
-  },
-
   getInitialState: function () {
     return {
       matches: []
@@ -356,41 +331,35 @@ var Routes = React.createClass({
 
   componentDidMount: function () {
     PathStore.addChangeListener(this.handlePathChange);
-    ScrollStore.addChangeListener(this.handleScrollChange);
   },
 
   componentWillUnmount: function () {
-    ScrollStore.removeChangeListener(this.handleScrollChange);
     PathStore.removeChangeListener(this.handlePathChange);
   },
 
   handlePathChange: function (_path) {
     var path = _path || PathStore.getCurrentPath();
+    var actionType = PathStore.getCurrentActionType();
 
     if (this.state.path === path)
       return; // Nothing to do!
 
+    if (this.state.path)
+      this.recordScroll(this.state.path);
+
     var self = this;
 
     this.dispatch(path, function (error, transition) {
       if (error) {
-        self.props.onTransitionError.call(self, error);
+        TransitionHandling.handleTransitionError.call(self, error);
       } else if (transition.isAborted) {
-        self.props.onAbortedTransition.call(self, transition);
+        TransitionHandling.handleAbortedTransition.call(self, transition);
       } else {
-        self.props.onTransition.call(self, transition);
+        self.updateScroll(path, actionType);
       }
     });
   },
 
-  handleScrollChange: function () {
-    var behavior = this.getScrollBehavior();
-    var position = ScrollStore.getCurrentScrollPosition();
-
-    if (behavior && position)
-      behavior.updateScrollPosition(position, PathStore.getCurrentActionType());
-  },
-
   /**
    * Performs a depth-first search for the first route in the tree that matches on
    * the given path. Returns an array of all routes in the tree leading to the one

modules/components/__tests__/Routes-test.js

@@ -69,58 +69,4 @@ describe('A Routes', function () {
     });
   });
 
-
-  describe('when a transition is aborted', function () {
-    it('triggers onAbortedTransition', function (done) {
-      var AbortHandler = React.createClass({
-        statics: {
-          willTransitionTo: function (transition) {
-            transition.abort();
-          }
-        },
-        render: function () {
-          return null;
-        }
-      });
-
-      function handleAbortedTransition(transition) {
-        assert(transition);
-        done();
-      }
-
-      ReactTestUtils.renderIntoDocument(
-        Routes({ onAbortedTransition: handleAbortedTransition },
-          Route({ handler: AbortHandler })
-        )
-      );
-    });
-  });
-
-  describe('when there is an error in a transition hook', function () {
-    it('triggers onTransitionError', function (done) {
-      var ErrorHandler = React.createClass({
-        statics: {
-          willTransitionTo: function (transition) {
-            throw new Error('boom!');
-          }
-        },
-        render: function () {
-          return null;
-        }
-      });
-
-      function handleTransitionError(error) {
-        assert(error);
-        expect(error.message).toEqual('boom!');
-        done();
-      }
-
-      ReactTestUtils.renderIntoDocument(
-        Routes({ onTransitionError: handleTransitionError },
-          Route({ handler: ErrorHandler })
-        )
-      );
-    });
-  });
-
 });

modules/mixins/ScrollContext.js

@@ -4,6 +4,18 @@ var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM;
 var ImitateBrowserBehavior = require('../behaviors/ImitateBrowserBehavior');
 var ScrollToTopBehavior = require('../behaviors/ScrollToTopBehavior');
 
+function getWindowScrollPosition() {
+  invariant(
+    canUseDOM,
+    'Cannot get current scroll position without a DOM'
+  );
+
+  return {
+    x: window.scrollX,
+    y: window.scrollY
+  };
+}
+
 /**
  * A hash of { name: scrollBehavior } pairs.
  */
@@ -52,6 +64,22 @@ var ScrollContext = {
       behavior == null || canUseDOM,
       'Cannot use scroll behavior without a DOM'
     );
+
+    if (behavior != null)
+      this._scrollPositions = {};
+  },
+
+  recordScroll: function (path) {
+    if (this._scrollPositions)
+      this._scrollPositions[path] = getWindowScrollPosition();
+  },
+
+  updateScroll: function (path, actionType) {
+    var behavior = this.getScrollBehavior();
+    var position = this._scrollPositions[path];
+
+    if (behavior && position)
+      behavior.updateScrollPosition(position, actionType);
   },
 
   /**

modules/stores/PathStore.js

@@ -1,7 +1,6 @@
 var EventEmitter = require('events').EventEmitter;
 var LocationActions = require('../actions/LocationActions');
 var LocationDispatcher = require('../dispatchers/LocationDispatcher');
-var ScrollStore = require('./ScrollStore');
 
 var CHANGE_EVENT = 'change';
 var _events = new EventEmitter;
@@ -51,8 +50,6 @@ var PathStore = {
       case LocationActions.PUSH:
       case LocationActions.REPLACE:
       case LocationActions.POP:
-        LocationDispatcher.waitFor([ ScrollStore.dispatchToken ]);
-
         if (_currentPath !== action.path) {
           _currentPath = action.path;
           _currentActionType = action.type;

modules/stores/ScrollStore.js

@@ -16,10 +16,8 @@ require('./modules/utils/__tests__/Path-test');
 
 
 var PathStore = require('./modules/stores/PathStore');
-var ScrollStore = require('./modules/stores/ScrollStore');
 
 afterEach(function () {
   // For some reason unmountComponentAtNode doesn't call componentWillUnmount :/
   PathStore.removeAllChangeListeners();
-  ScrollStore.removeAllChangeListeners();
 });