Skip to content

Commit

Permalink
fix(transition): Fail a transition if a new one has started while res…
Browse files Browse the repository at this point in the history
…olves are loading

Closes ui-router/react#3
Probably closes #2972
  • Loading branch information
christopherthielen committed Sep 19, 2016
1 parent 2fcc920 commit bc87d9e
Showing 1 changed file with 40 additions and 28 deletions.
68 changes: 40 additions & 28 deletions src/transition/transitionHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
import {TransitionHookOptions, IEventHook, HookResult} from "./interface";
import {defaults, noop} from "../common/common";
import {fnToString, maxLength} from "../common/strings";
import {isDefined, isPromise } from "../common/predicates";
import {pattern, val, eq, is, parse } from "../common/hof";
import {isPromise} from "../common/predicates";
import {val, is, parse } from "../common/hof";
import {trace} from "../common/trace";
import {services} from "../common/coreservices";

import {Rejection} from "./rejectFactory";
import {TargetState} from "../state/targetState";
import {ResolveContext} from "../resolve/resolveContext";
import {Transition} from "./transition";
import {State} from "../state/stateObject";

Expand All @@ -31,7 +30,8 @@ export class TransitionHook {
this.options = defaults(options, defaultOptions);
}

private isSuperseded = () => this.options.current() !== this.options.transition;
private isSuperseded = () =>
this.options.current() !== this.options.transition;

invokeHook(): Promise<HookResult> {
let { options, eventHook } = this;
Expand All @@ -40,38 +40,50 @@ export class TransitionHook {
return Rejection.superseded(options.current()).toPromise();
}

let hookResult = !eventHook._deregistered
let synchronousHookResult = !eventHook._deregistered
? eventHook.callback.call(options.bind, this.transition, this.stateContext)
: undefined;
return this.handleHookResult(hookResult);

return this.handleHookResult(synchronousHookResult);
}

/**
* This method handles the return value of a Transition Hook.
*
* A hook can return false, a redirect (TargetState), or a promise (which may resolve to false or a redirect)
* A hook can return false (cancel), a TargetState (redirect),
* or a promise (which may later resolve to false or a redirect)
*
* This also handles "transition superseded" -- when a new transition
* was started while the hook was still running
*/
handleHookResult(hookResult: HookResult): Promise<any> {
if (!isDefined(hookResult)) return undefined;

This comment has been minimized.

Copy link
@christopherthielen

christopherthielen Sep 19, 2016

Author Contributor

This early exit was causing the check to isSuperseded to get skipped.
Resolves are implemented as a hook. when the resolve completed, the hook returns undefined (which means to continue processing the transition).
Since resolve is the last hook before onFinish, there were no more checks for isSuperseded.

This commit makes all hook results always check isSuperseded


/**
* Handles transition superseded, transition aborted and transition redirect.
*/
const mapHookResult = pattern([
// Transition is no longer current
[this.isSuperseded, () => Rejection.superseded(this.options.current()).toPromise()],
// If the hook returns false, abort the current Transition
[eq(false), () => Rejection.aborted("Hook aborted transition").toPromise()],
// If the hook returns a Transition, halt the current Transition and redirect to that Transition.
[is(TargetState), (target: TargetState) => Rejection.redirected(target).toPromise()],
// A promise was returned, wait for the promise and then chain another hookHandler
[isPromise, (promise: Promise<any>) => promise.then(this.handleHookResult.bind(this))]
]);

let transitionResult = mapHookResult(hookResult);
if (transitionResult) trace.traceHookResult(hookResult, transitionResult, this.options);

return transitionResult;
handleHookResult(result: HookResult): Promise<any> {
// This transition is no longer current.
// Another transition started while this hook was still running.
if (this.isSuperseded()) {
// Abort this transition
return Rejection.superseded(this.options.current()).toPromise();
}

// Hook returned a promise
if (isPromise(result)) {
// Wait for the promise, then reprocess the resolved value
return result.then(this.handleHookResult.bind(this));
}

trace.traceHookResult(result, this.options);

// Hook returned false
if (result === false) {
// Abort this Transition
return Rejection.aborted("Hook aborted transition").toPromise();
}

const isTargetState = is(TargetState);
// hook returned a TargetState
if (isTargetState(result)) {
// Halt the current Transition and start a redirected Transition (to the TargetState).
return Rejection.redirected(result).toPromise();
}
}

toString() {
Expand Down

0 comments on commit bc87d9e

Please sign in to comment.