Skip to content

Commit

Permalink
fix($state.transitionTo): trigger $stateChangeCancel appropriately (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
zeusdeux authored and christopherthielen committed Nov 3, 2016
1 parent a8aa40a commit ca7c366
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 10 deletions.
30 changes: 24 additions & 6 deletions src/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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--) {
Expand All @@ -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;
Expand Down Expand Up @@ -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;
/**
Expand All @@ -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);
Expand Down
8 changes: 5 additions & 3 deletions test/stateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -514,19 +514,21 @@ describe('state', function () {
expect(log).toBe(
'$stateChangeStart(B,A);' +
'$stateChangeStart(C,A);' +
'$stateChangeCancel(B,A);' +
'$stateChangeSuccess(C,A);');
}));

it('aborts pending transitions even when going back to the current state', inject(function ($state, $q) {
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) {
Expand Down
2 changes: 1 addition & 1 deletion test/testUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit ca7c366

Please sign in to comment.