From 3141a8fa4fe02049329d0f216e5358ec74b83099 Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Wed, 29 Mar 2017 15:58:06 -0500 Subject: [PATCH] feat(transition): Improve supersede logic: Do not supersede if the new trans is aborted before onStart Previously, any calls to state.go would supersede the running transition. Now, if the new transition is aborted before it reaches "run" phase (onStart hook), it will not cause the running transition to be aborted. Closes https://github.com/ui-router/core/issues/41 --- src/globals.ts | 6 +++++- src/hooks/updateGlobals.ts | 19 +++++++++--------- src/state/stateService.ts | 5 +++-- src/transition/transition.ts | 21 +++++++++++++------- src/transition/transitionHook.ts | 4 ++-- test/stateServiceSpec.ts | 33 +++++++++++++++++++++++++------- 6 files changed, 59 insertions(+), 29 deletions(-) diff --git a/src/globals.ts b/src/globals.ts index dc28c5d3..f4d46e42 100644 --- a/src/globals.ts +++ b/src/globals.ts @@ -41,10 +41,14 @@ export class UIRouterGlobals implements Disposable { $current: StateObject; /** - * The current transition (in progress) + * The current started/running transition. + * This transition has reached at least the onStart phase, but is not yet complete */ transition: Transition; + /** @internalapi */ + lastStartedTransitionId: number = -1; + /** @internalapi */ transitionHistory = new Queue([], 1); diff --git a/src/hooks/updateGlobals.ts b/src/hooks/updateGlobals.ts index d7acbdf4..09830a75 100644 --- a/src/hooks/updateGlobals.ts +++ b/src/hooks/updateGlobals.ts @@ -1,7 +1,8 @@ -/** @module hooks */ /** for typedoc */ -import { Transition } from "../transition/transition"; -import { copy } from "../common/common"; -import { TransitionService } from "../transition/transitionService"; +/** @module hooks */ +/** for typedoc */ +import { Transition } from '../transition/transition'; +import { copy } from '../common/common'; +import { TransitionService } from '../transition/transitionService'; /** * A [[TransitionHookFn]] which updates global UI-Router state @@ -17,25 +18,23 @@ import { TransitionService } from "../transition/transitionService"; */ const updateGlobalState = (trans: Transition) => { let globals = trans.router.globals; - globals.transition = trans; - globals.transitionHistory.enqueue(trans); - const updateGlobalState = () => { + const transitionSuccessful = () => { globals.successfulTransitions.enqueue(trans); globals.$current = trans.$to(); globals.current = globals.$current.self; + copy(trans.params(), globals.params); }; - trans.onSuccess({}, updateGlobalState, {priority: 10000}); - const clearCurrentTransition = () => { // Do not clear globals.transition if a different transition has started in the meantime if (globals.transition === trans) globals.transition = null; }; + trans.onSuccess({}, transitionSuccessful, { priority: 10000 }); trans.promise.then(clearCurrentTransition, clearCurrentTransition); }; export const registerUpdateGlobalState = (transitionService: TransitionService) => - transitionService.onBefore({}, updateGlobalState); + transitionService.onCreate({}, updateGlobalState); diff --git a/src/state/stateService.ts b/src/state/stateService.ts index 7985b5fc..d725e45a 100644 --- a/src/state/stateService.ts +++ b/src/state/stateService.ts @@ -309,9 +309,10 @@ export class StateService { transitionTo(to: StateOrName, toParams: RawParams = {}, options: TransitionOptions = {}): TransitionPromise { let router = this.router; let globals = router.globals; - let transHistory = globals.transitionHistory; options = defaults(options, defaultTransOpts); - options = extend(options, { current: transHistory.peekTail.bind(transHistory)}); + const getCurrent = () => + globals.transition; + options = extend(options, { current: getCurrent }); let ref: TargetState = this.target(to, toParams, options); let currentPath = this.getCurrentPath(); diff --git a/src/transition/transition.ts b/src/transition/transition.ts index 4782d745..40775c91 100644 --- a/src/transition/transition.ts +++ b/src/transition/transition.ts @@ -618,8 +618,6 @@ export class Transition implements IHookRegistry { */ run(): Promise { let runAllHooks = TransitionHook.runAllHooks; - let globals = this.router.globals; - globals.transitionHistory.enqueue(this); // Gets transition hooks array for the given phase const hooksFor = (phase: TransitionHookPhase) => @@ -630,6 +628,16 @@ export class Transition implements IHookRegistry { const chainFor = (phase: TransitionHookPhase) => TransitionHook.chain(hooksFor(phase)); + const startTransition = () => { + let globals = this.router.globals; + + globals.lastStartedTransitionId = this.$id; + globals.transition = this; + globals.transitionHistory.enqueue(this); + + trace.traceTransitionStart(this); + }; + // When the chain is complete, then resolve or reject the deferred const transitionSuccess = () => { trace.traceSuccess(this.$to(), this); @@ -648,7 +656,7 @@ export class Transition implements IHookRegistry { services.$q.when() .then(() => chainFor(TransitionHookPhase.BEFORE)) - .then(() => trace.traceTransitionStart(this)) + .then(startTransition) // This waits to build the RUN hook chain until after the "BEFORE" hooks complete // This allows a BEFORE hook to dynamically add RUN hooks via the Transition object. .then(() => chainFor(TransitionHookPhase.RUN)) @@ -657,10 +665,9 @@ export class Transition implements IHookRegistry { return this.promise; } - /** - * Checks if this transition is currently active/running. - */ - isActive = () => this === this._options.current(); + /** Checks if this transition is currently active/running. */ + isActive = () => + this.router.globals.transition === this; /** * Checks if the Transition is valid diff --git a/src/transition/transitionHook.ts b/src/transition/transitionHook.ts index 33cee733..5ecb3b97 100644 --- a/src/transition/transitionHook.ts +++ b/src/transition/transitionHook.ts @@ -3,7 +3,7 @@ * @module transition */ /** for typedoc */ -import { TransitionHookOptions, HookResult } from './interface'; +import { TransitionHookOptions, HookResult, TransitionHookPhase } from './interface'; import { defaults, noop, silentRejection } from '../common/common'; import { fnToString, maxLength } from '../common/strings'; import { isPromise } from '../common/predicates'; @@ -73,7 +73,7 @@ export class TransitionHook { }; private isSuperseded = () => - !this.type.synchronous && this.options.current() !== this.options.transition; + this.type.hookPhase === TransitionHookPhase.RUN && !this.options.transition.isActive(); logError(err): any { this.transition.router.stateService.defaultErrorHandler()(err); diff --git a/test/stateServiceSpec.ts b/test/stateServiceSpec.ts index eee8c194..529386ca 100644 --- a/test/stateServiceSpec.ts +++ b/test/stateServiceSpec.ts @@ -241,11 +241,8 @@ describe('stateService', function () { }); afterEach((done) => { - if (router.globals.transition) { - router.globals.transition.promise.then(done, done) - } else { - done(); - } + router.dispose(); + done(); }); describe('[ transition.dynamic() ]:', function() { @@ -545,12 +542,15 @@ describe('stateService', function () { done(); }); - it('aborts pending transitions when superseded from callbacks', async(done) => { + fit('aborts pending transitions when superseded from callbacks', async(done) => { + // router.trace.enable(); $state.defaultErrorHandler(() => null); $registry.register({ name: 'redir', url: "redir", - onEnter: (trans) => { trans.router.stateService.go('A') } + onEnter: (trans) => { + trans.router.stateService.go('A') + } }); let result = await $state.transitionTo('redir').catch(err => err); await router.globals.transition.promise; @@ -561,6 +561,25 @@ describe('stateService', function () { done(); }); + it('does not abort pending transition when a new transition is cancelled by onBefore hook', (done) => { + router.transitionService.onBefore({}, (t) => { + if (t.$id === 1) return false; + return (new Promise(resolve => setTimeout(resolve, 100))) as any; + }); + + let promise1 = $state.transitionTo('A'); // takes 100 ms + let promise2 = $state.transitionTo('A'); // is cancelled + let promise2Error; + + promise1.then(() => { + expect($state.current.name).toBe('A'); + expect(promise2Error).toBeDefined(); + done(); + }); + + promise2.catch((err) => promise2Error = err); + }); + it('triggers onEnter and onExit callbacks', async(done) => { let log = ""; await initStateTo(A);