Skip to content

Commit

Permalink
fix(transitionHook): Do not process transition hooks after router has…
Browse files Browse the repository at this point in the history
… been disposed.
  • Loading branch information
christopherthielen committed Apr 20, 2017
1 parent 4bdce47 commit 666c6d7
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 18 deletions.
8 changes: 4 additions & 4 deletions src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@ let _routerInstance = 0;
* [[UrlService.listen]] then [[UrlService.sync]].
*/
export class UIRouter {
/** @hidden */
$id: number = _routerInstance++;
/** @hidden */ $id = _routerInstance++;
/** @hidden */ _disposed = false;
/** @hidden */ private _disposables: Disposable[] = [];

/** Provides trace information to the console */
trace: Trace = trace;
Expand Down Expand Up @@ -71,8 +72,6 @@ export class UIRouter {
/** Provides services related to the URL */
urlService: UrlService = new UrlService(this);

/** @hidden */
private _disposables: Disposable[] = [];

/** Registers an object to be notified when the router is disposed */
disposable(disposable: Disposable) {
Expand All @@ -95,6 +94,7 @@ export class UIRouter {
return undefined;
}

this._disposed = true;
this._disposables.slice().forEach(d => {
try {
typeof d.dispose === 'function' && d.dispose(this);
Expand Down
39 changes: 27 additions & 12 deletions src/transition/transitionHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,14 @@ export class TransitionHook {
let hook = this.registeredHook;
if (hook._deregistered) return;

let notCurrent = this.getNotCurrentRejection();
if (notCurrent) return notCurrent;

let options = this.options;
trace.traceHookInvocation(this, this.transition, options);

// A new transition started before this hook (from a previous transition) could be run.
if (this.isSuperseded()) {
return Rejection.superseded(options.current()).toPromise();
}

const invokeCallback = () =>
hook.callback.call(this.options.bind, this.transition, this.stateContext);
hook.callback.call(options.bind, this.transition, this.stateContext);

const normalizeErr = err =>
Rejection.normalize(err).toPromise();
Expand Down Expand Up @@ -127,12 +125,8 @@ export class TransitionHook {
* was started while the hook was still running
*/
handleHookResult(result: HookResult): Promise<HookResult> {
// 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();
}
let notCurrent = this.getNotCurrentRejection();
if (notCurrent) return notCurrent;

// Hook returned a promise
if (isPromise(result)) {
Expand All @@ -156,6 +150,27 @@ export class TransitionHook {
}
}


/**
* Return a Rejection promise if the transition is no longer current due
* to a stopped router (disposed), or a new transition has started and superseded this one.
*/
private getNotCurrentRejection() {
let router = this.transition.router;

// The router is stopped
if (router._disposed) {
return Rejection.aborted(`UIRouter instance #${router.$id} has been stopped (disposed)`).toPromise();
}

// 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();
}
}

toString() {
let { options, registeredHook } = this;
let event = parse("traceData.hookType")(options) || "internal",
Expand Down
4 changes: 4 additions & 0 deletions test/_testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { map } from "../src/common/common";

let stateProps = ["resolve", "resolvePolicy", "data", "template", "templateUrl", "url", "name", "params"];

export const delay = (ms) =>
new Promise<any>(resolve => setTimeout(resolve, ms));
export const _delay = (ms) => () => delay(ms);

export function tree2Array(tree, inheritName) {

function processState(parent, state, name) {
Expand Down
38 changes: 36 additions & 2 deletions test/hooksSpec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { UIRouter } from "../src/router";
import { tree2Array } from "./_testUtils";
import {delay, tree2Array} from "./_testUtils";
import { find } from "../src/common/common";
import { StateService } from "../src/state/stateService";
import { StateDeclaration } from "../src/state/interface";
Expand All @@ -19,7 +19,7 @@ let statetree = {
};

describe("hooks", () => {
let router;
let router: UIRouter;
let $state: StateService;
let $transitions: TransitionService;
let states: StateDeclaration[];
Expand Down Expand Up @@ -179,4 +179,38 @@ describe("hooks", () => {
});
});

it("should not run a hook after the router is stopped", (done) => {
init();
let called = false;
router.transitionService.onSuccess({}, () => called = true);

$state.go('A').catch(err => {
expect(called).toBe(false);
expect(err).toBeDefined();
expect(err.detail).toContain('disposed');
done();
});

router.dispose();
});

it("should not process a hook result after the router is stopped", (done) => {
init();
let called = false;
let disposed, isdisposed = new Promise<any>(resolve => disposed = resolve);

router.transitionService.onEnter({}, () => {
called = true;
return isdisposed.then(() => $state.target('B'));
});

$state.go('A').catch(err => {
expect(called).toBe(true);
expect(err).toBeDefined();
expect(err.detail).toContain('disposed');
done();
});

setTimeout(() => { router.dispose(); disposed(); }, 50);
});
});

0 comments on commit 666c6d7

Please sign in to comment.