Skip to content

Commit

Permalink
feat(transition): Improve supersede logic: Do not supersede if the ne…
Browse files Browse the repository at this point in the history
…w 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 #41
  • Loading branch information
christopherthielen committed Apr 20, 2017
1 parent da0bdfe commit 3141a8f
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 29 deletions.
6 changes: 5 additions & 1 deletion src/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Transition>([], 1);

Expand Down
19 changes: 9 additions & 10 deletions src/hooks/updateGlobals.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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);
5 changes: 3 additions & 2 deletions src/state/stateService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
21 changes: 14 additions & 7 deletions src/transition/transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,8 +618,6 @@ export class Transition implements IHookRegistry {
*/
run(): Promise<any> {
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) =>
Expand All @@ -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);
Expand All @@ -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))
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/transition/transitionHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
33 changes: 26 additions & 7 deletions test/stateServiceSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down

0 comments on commit 3141a8f

Please sign in to comment.