Merge pull request #103 from yahoo/navigateTransaction

Add transactionIds to ensure only the latest navigation state is used

lib/RouteStore.js

@@ -27,27 +27,43 @@ var RouteStore = createStore({
         this._isNavigateComplete = null;
         this._router = null;
     },
-    _handleNavigateStart: function (payload) {
-        var matchedRoute = this._matchRoute(payload.url, {
-            navigate: payload,
-            method: payload.method
+    _handleNavigateStart: function (navigate) {
+        var matchedRoute = this._matchRoute(navigate.url, {
+            navigate: navigate,
+            method: navigate.method
         });
 
         if (!this._areEqual(matchedRoute, this._currentRoute)) {
             this._currentRoute = matchedRoute;
-            this._currentNavigate = payload;
+            this._currentNavigate = navigate;
         }
 
         this._currentNavigateError = null;
         this._isNavigateComplete = false;
-        this._currentUrl = payload.url;
+        this._currentUrl = navigate.url;
         this.emitChange();
     },
     _handleNavigateSuccess: function (route) {
+        // Ignore successes from past navigations that have been superceded
+        if (
+            this._isNavigateComplete ||
+            (this._currentNavigate && route.navigate && route.navigate.transactionId !== this._currentNavigate.transactionId)
+        ) {
+            return;
+        }
+
         this._isNavigateComplete = true;
         this.emitChange();
     },
     _handleNavigateFailure: function (error) {
+        // Ignore failures from past navigations that have been superceded
+        if (
+            this._isNavigateComplete ||
+            (this._currentNavigate && error.transactionId !== this._currentNavigate.transactionId)
+        ) {
+            return;
+        }
+
         this._isNavigateComplete = true;
         this._currentNavigateError = error;
         this.emitChange();

lib/navigateAction.js

@@ -7,23 +7,28 @@ var debug = require('debug')('navigateAction');
 
 module.exports = function navigateAction (context, payload, done) {
     var routeStore = context.getStore('RouteStore');
+
+    var navigate = Object.assign({
+        transactionId: context.rootId
+    }, payload);
     if (!payload.url && payload.routeName) {
-        payload.url = routeStore.makePath(payload.routeName, payload.params);
-        payload.routeName = null;
+        navigate.url = routeStore.makePath(payload.routeName, payload.params);
+        navigate.routeName = null;
     }
-    debug('dispatching NAVIGATE_START', payload);
-    context.dispatch('NAVIGATE_START', payload);
+
+    debug('dispatching NAVIGATE_START', navigate);
+    context.dispatch('NAVIGATE_START', navigate);
 
     if (!routeStore.getCurrentRoute) {
         done(new Error('RouteStore has not implemented `getCurrentRoute` method.'));
         return;
     }
-    debug('executing', payload);
 
     var route = routeStore.getCurrentRoute();
 
     if (!route) {
         var error404 = {
+            transactionId: navigate.transactionId,
             statusCode: 404,
             message: 'Url \'' + payload.url + '\' does not match any routes'
         };
@@ -49,6 +54,7 @@ module.exports = function navigateAction (context, payload, done) {
     context.executeAction(action, route, function (err) {
         if (err) {
             var error500 = {
+                transactionId: navigate.transactionId,
                 statusCode: err.statusCode || 500,
                 message: err.message
             };

tests/unit/lib/RouteStore-test.js

@@ -18,6 +18,7 @@ describe('RouteStore', function () {
         beforeEach(function () {
             routeStore = new StaticRouteStore();
             routeStore._handleNavigateStart({
+                transactionId: 'first',
                 url: '/foo',
                 method: 'get'
             });
@@ -28,6 +29,7 @@ describe('RouteStore', function () {
                 expect(state).to.be.an('object');
                 expect(state.currentUrl).to.equal('/foo');
                 expect(state.currentNavigate).to.deep.equal({
+                    transactionId: 'first',
                     url: '/foo',
                     method: 'get'
                 });
@@ -39,24 +41,73 @@ describe('RouteStore', function () {
                 var newStore = new StaticRouteStore();
                 newStore.rehydrate({
                     currentUrl: '/foo',
-                    currentNavigate: { url: '/foo', method: 'get' },
+                    currentNavigate: { transactionId: 'first', url: '/foo', method: 'get' },
                     routes: null
                 });
                 expect(newStore.getCurrentRoute()).to.be.an('object');
                 expect(newStore.getCurrentNavigate()).to.deep.equal({
+                    transactionId: 'first',
                     url: '/foo',
                     method: 'get'
                 });
                 expect(newStore._routes).to.equal(null);
             });
         });
+
         it('should reuse static router between instances', function () {
             var newStore = new StaticRouteStore();
             expect(newStore._router).to.equal(routeStore._router);
         });
+
+        it('should only use the latest navigate on success', function () {
+            // Start a new navigate before first has completed
+            routeStore._handleNavigateStart({
+                transactionId: 'second',
+                url: '/bar',
+                method: 'get'
+            });
+            expect(routeStore.isNavigateComplete()).to.equal(false);
+            routeStore._handleNavigateSuccess({
+                navigate: {
+                    transactionId: 'first'
+                },
+                url: '/bar',
+                method: 'get'
+            });
+            expect(routeStore.isNavigateComplete()).to.equal(false);
+            routeStore._handleNavigateSuccess({
+                navigate: {
+                    transactionId: 'second'
+                },
+                url: '/bar',
+                method: 'get'
+            });
+            expect(routeStore.isNavigateComplete()).to.equal(true);
+        });
+        it('should only use the latest navigate on failure', function () {
+            // Start a new navigate before first has completed
+            routeStore._handleNavigateStart({
+                transactionId: 'second',
+                url: '/bar',
+                method: 'get'
+            });
+            expect(routeStore.isNavigateComplete()).to.equal(false);
+            routeStore._handleNavigateFailure({
+                transactionId: 'first',
+                statusCode: 404,
+                message: 'Url /unknown does not match any routes'
+            });
+            expect(routeStore.isNavigateComplete()).to.equal(false);
+            routeStore._handleNavigateFailure({
+                transactionId: 'second',
+                statusCode: 404,
+                message: 'Url /unknown does not match any routes'
+            });
+            expect(routeStore.isNavigateComplete()).to.equal(true);
+        });
     });
 
-    describe('withoutStaticRoutes', function () {
+    describe('dynamic routes', function () {
         var routeStore;
         var routes = {
             foo: {

tests/unit/lib/navigateAction-test.js

@@ -96,6 +96,7 @@ describe('navigateAction', function () {
     it('should include navigate object on route match', function (done) {
         var url = '/';
         navigateAction(mockContext, {
+            transactionId: 'foo',
             url: url,
             someKey1: 'someData',
             someKey2: {
@@ -106,7 +107,12 @@ describe('navigateAction', function () {
             expect(mockContext.dispatchCalls.length).to.equal(2);
             expect(mockContext.dispatchCalls[0].name).to.equal('NAVIGATE_START');
             var route = mockContext.getStore('RouteStore').getCurrentRoute();
-            expect(route.navigate).to.eql({url: url, someKey1: 'someData', someKey2: {someKey3: ['a', 'b']}}, 'navigate added to route payload for NAVIGATE_START' + JSON.stringify(route));
+            expect(route.navigate).to.eql({
+                transactionId: 'foo',
+                url: url,
+                someKey1: 'someData',
+                someKey2: {someKey3: ['a', 'b']}
+            }, 'navigate added to route payload for NAVIGATE_START' + JSON.stringify(route));
             done();
         });
     });