Skip to content

Commit ca7c366

Browse files
zeusdeuxchristopherthielen
authored andcommitted
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<Success|Error|Cancel>` thus completing the lifecycle. Fixes issue #3027
1 parent a8aa40a commit ca7c366

File tree

3 files changed

+30
-10
lines changed

3 files changed

+30
-10
lines changed

src/state.js

+24-6
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,9 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
716716
$get.$inject = ['$rootScope', '$q', '$view', '$injector', '$resolve', '$stateParams', '$urlRouter', '$location', '$urlMatcherFactory'];
717717
function $get( $rootScope, $q, $view, $injector, $resolve, $stateParams, $urlRouter, $location, $urlMatcherFactory) {
718718

719-
var TransitionSuperseded = $q.reject(new Error('transition superseded'));
719+
var TransitionSupersededError = new Error('transition superseded');
720+
721+
var TransitionSuperseded = $q.reject(TransitionSupersededError);
720722
var TransitionPrevented = $q.reject(new Error('transition prevented'));
721723
var TransitionAborted = $q.reject(new Error('transition aborted'));
722724
var TransitionFailed = $q.reject(new Error('transition failed'));
@@ -775,7 +777,10 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
775777
var retryTransition = $state.transition = $q.when(evt.retry);
776778

777779
retryTransition.then(function() {
778-
if (retryTransition !== $state.transition) return TransitionSuperseded;
780+
if (retryTransition !== $state.transition) {
781+
$rootScope.$broadcast('$stateChangeCancel', redirect.to, redirect.toParams, state, params);
782+
return TransitionSuperseded;
783+
}
779784
redirect.options.$retry = true;
780785
return $state.transitionTo(redirect.to, redirect.toParams, redirect.options);
781786
}, function() {
@@ -1114,7 +1119,10 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
11141119
var transition = $state.transition = resolved.then(function () {
11151120
var l, entering, exiting;
11161121

1117-
if ($state.transition !== transition) return TransitionSuperseded;
1122+
if ($state.transition !== transition) {
1123+
$rootScope.$broadcast('$stateChangeCancel', to.self, toParams, from.self, fromParams);
1124+
return TransitionSuperseded;
1125+
}
11181126

11191127
// Exit 'from' states not kept
11201128
for (l = fromPath.length - 1; l >= keep; l--) {
@@ -1135,7 +1143,10 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
11351143
}
11361144

11371145
// Run it again, to catch any transitions in callbacks
1138-
if ($state.transition !== transition) return TransitionSuperseded;
1146+
if ($state.transition !== transition) {
1147+
$rootScope.$broadcast('$stateChangeCancel', to.self, toParams, from.self, fromParams);
1148+
return TransitionSuperseded;
1149+
}
11391150

11401151
// Update globals in $state
11411152
$state.$current = to;
@@ -1171,7 +1182,14 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
11711182

11721183
return $state.current;
11731184
}).then(null, function (error) {
1174-
if ($state.transition !== transition) return TransitionSuperseded;
1185+
// propagate TransitionSuperseded error without emitting $stateChangeCancel
1186+
// as it was already emitted in the success handler above
1187+
if (error === TransitionSupersededError) return TransitionSuperseded;
1188+
1189+
if ($state.transition !== transition) {
1190+
$rootScope.$broadcast('$stateChangeCancel', to.self, toParams, from.self, fromParams);
1191+
return TransitionSuperseded;
1192+
}
11751193

11761194
$state.transition = null;
11771195
/**
@@ -1195,7 +1213,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) {
11951213
evt = $rootScope.$broadcast('$stateChangeError', to.self, toParams, from.self, fromParams, error);
11961214

11971215
if (!evt.defaultPrevented) {
1198-
$urlRouter.update();
1216+
$urlRouter.update();
11991217
}
12001218

12011219
return $q.reject(error);

test/stateSpec.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -514,19 +514,21 @@ describe('state', function () {
514514
expect(log).toBe(
515515
'$stateChangeStart(B,A);' +
516516
'$stateChangeStart(C,A);' +
517+
'$stateChangeCancel(B,A);' +
517518
'$stateChangeSuccess(C,A);');
518519
}));
519520

520521
it('aborts pending transitions even when going back to the current state', inject(function ($state, $q) {
521522
initStateTo(A);
522523
logEvents = true;
523524

524-
var superseded = $state.transitionTo(B, {});
525-
$state.transitionTo(A, {});
525+
// added direction param to help future contributors debug state events
526+
var superseded = $state.transitionTo(B, {direction: 'A to B'});
527+
$state.transitionTo(A, { direction: 'A to A'});
526528
$q.flush();
527529
expect($state.current).toBe(A);
528530
expect(resolvedError(superseded)).toBeTruthy();
529-
expect(log).toBe('$stateChangeStart(B,A);');
531+
expect(log).toBe('$stateChangeStart(B,A);$stateChangeCancel(B,A);');
530532
}));
531533

532534
it('aborts pending transitions when aborted from callbacks', inject(function ($state, $q) {

test/testUtils.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ function resolvedValue(promise) {
8585

8686
function resolvedError(promise) {
8787
var result = resolvedPromise(promise);
88-
if (result.success) throw new Error('Promise was expected to fail but returned ' + jasmin.pp(res.value) + '.');
88+
if (result.success) throw new Error('Promise was expected to fail but returned ' + jasmine.pp(result.value) + '.');
8989
return result.error;
9090
}
9191

0 commit comments

Comments
 (0)