Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix($state.transitionTo): trigger $stateChangeCancel appropriately #3039

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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