From ca7c3661fc1451c16e685dc1a856ed598d801d5a Mon Sep 17 00:00:00 2001 From: Mudit Ameta Date: Thu, 3 Nov 2016 16:00:34 +0100 Subject: [PATCH] fix($state.transitionTo): trigger $stateChangeCancel appropriately (#3039) Current behaviour: We are in state A. We start a transition to state B. `$stateChangeStart` is triggered. Before `$stateChangeSuccess` is triggered for state B, we start a transition back to state A. No state events are triggered and we are left hanging without any notification. Updated behaviour: When the state change to B is superseded by the state change back to A, `$stateChangeCancel` is broadcasted on `$rootScope` for the transition from B -> A. This behaviour makes sure that for every `$stateChangeStart` there is a corresponding `$stateChange` thus completing the lifecycle. Fixes issue #3027 --- src/state.js | 30 ++++++++++++++++++++++++------ test/stateSpec.js | 8 +++++--- test/testUtils.js | 2 +- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/state.js b/src/state.js index e3b455336..91b1c8e41 100644 --- a/src/state.js +++ b/src/state.js @@ -716,7 +716,9 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { $get.$inject = ['$rootScope', '$q', '$view', '$injector', '$resolve', '$stateParams', '$urlRouter', '$location', '$urlMatcherFactory']; function $get( $rootScope, $q, $view, $injector, $resolve, $stateParams, $urlRouter, $location, $urlMatcherFactory) { - var TransitionSuperseded = $q.reject(new Error('transition superseded')); + var TransitionSupersededError = new Error('transition superseded'); + + var TransitionSuperseded = $q.reject(TransitionSupersededError); var TransitionPrevented = $q.reject(new Error('transition prevented')); var TransitionAborted = $q.reject(new Error('transition aborted')); var TransitionFailed = $q.reject(new Error('transition failed')); @@ -775,7 +777,10 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { var retryTransition = $state.transition = $q.when(evt.retry); retryTransition.then(function() { - if (retryTransition !== $state.transition) return TransitionSuperseded; + if (retryTransition !== $state.transition) { + $rootScope.$broadcast('$stateChangeCancel', redirect.to, redirect.toParams, state, params); + return TransitionSuperseded; + } redirect.options.$retry = true; return $state.transitionTo(redirect.to, redirect.toParams, redirect.options); }, function() { @@ -1114,7 +1119,10 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { var transition = $state.transition = resolved.then(function () { var l, entering, exiting; - if ($state.transition !== transition) return TransitionSuperseded; + if ($state.transition !== transition) { + $rootScope.$broadcast('$stateChangeCancel', to.self, toParams, from.self, fromParams); + return TransitionSuperseded; + } // Exit 'from' states not kept for (l = fromPath.length - 1; l >= keep; l--) { @@ -1135,7 +1143,10 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { } // Run it again, to catch any transitions in callbacks - if ($state.transition !== transition) return TransitionSuperseded; + if ($state.transition !== transition) { + $rootScope.$broadcast('$stateChangeCancel', to.self, toParams, from.self, fromParams); + return TransitionSuperseded; + } // Update globals in $state $state.$current = to; @@ -1171,7 +1182,14 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { return $state.current; }).then(null, function (error) { - if ($state.transition !== transition) return TransitionSuperseded; + // propagate TransitionSuperseded error without emitting $stateChangeCancel + // as it was already emitted in the success handler above + if (error === TransitionSupersededError) return TransitionSuperseded; + + if ($state.transition !== transition) { + $rootScope.$broadcast('$stateChangeCancel', to.self, toParams, from.self, fromParams); + return TransitionSuperseded; + } $state.transition = null; /** @@ -1195,7 +1213,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { evt = $rootScope.$broadcast('$stateChangeError', to.self, toParams, from.self, fromParams, error); if (!evt.defaultPrevented) { - $urlRouter.update(); + $urlRouter.update(); } return $q.reject(error); diff --git a/test/stateSpec.js b/test/stateSpec.js index 17034444b..5610cd337 100644 --- a/test/stateSpec.js +++ b/test/stateSpec.js @@ -514,6 +514,7 @@ describe('state', function () { expect(log).toBe( '$stateChangeStart(B,A);' + '$stateChangeStart(C,A);' + + '$stateChangeCancel(B,A);' + '$stateChangeSuccess(C,A);'); })); @@ -521,12 +522,13 @@ describe('state', function () { initStateTo(A); logEvents = true; - var superseded = $state.transitionTo(B, {}); - $state.transitionTo(A, {}); + // added direction param to help future contributors debug state events + var superseded = $state.transitionTo(B, {direction: 'A to B'}); + $state.transitionTo(A, { direction: 'A to A'}); $q.flush(); expect($state.current).toBe(A); expect(resolvedError(superseded)).toBeTruthy(); - expect(log).toBe('$stateChangeStart(B,A);'); + expect(log).toBe('$stateChangeStart(B,A);$stateChangeCancel(B,A);'); })); it('aborts pending transitions when aborted from callbacks', inject(function ($state, $q) { diff --git a/test/testUtils.js b/test/testUtils.js index ceff4574a..d6bd4c497 100644 --- a/test/testUtils.js +++ b/test/testUtils.js @@ -85,7 +85,7 @@ function resolvedValue(promise) { function resolvedError(promise) { var result = resolvedPromise(promise); - if (result.success) throw new Error('Promise was expected to fail but returned ' + jasmin.pp(res.value) + '.'); + if (result.success) throw new Error('Promise was expected to fail but returned ' + jasmine.pp(result.value) + '.'); return result.error; }