diff --git a/src/router.ts b/src/router.ts index e6ec6bab..f93b62fe 100644 --- a/src/router.ts +++ b/src/router.ts @@ -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; @@ -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) { @@ -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); diff --git a/src/transition/transitionHook.ts b/src/transition/transitionHook.ts index d7bb7287..33cee733 100644 --- a/src/transition/transitionHook.ts +++ b/src/transition/transitionHook.ts @@ -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(); @@ -127,12 +125,8 @@ export class TransitionHook { * was started while the hook was still running */ handleHookResult(result: HookResult): Promise { - // 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)) { @@ -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", diff --git a/test/_testUtils.ts b/test/_testUtils.ts index 33541a21..6d5ce84b 100644 --- a/test/_testUtils.ts +++ b/test/_testUtils.ts @@ -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(resolve => setTimeout(resolve, ms)); +export const _delay = (ms) => () => delay(ms); + export function tree2Array(tree, inheritName) { function processState(parent, state, name) { diff --git a/test/hooksSpec.ts b/test/hooksSpec.ts index 9c753f5c..4df70cd4 100644 --- a/test/hooksSpec.ts +++ b/test/hooksSpec.ts @@ -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"; @@ -19,7 +19,7 @@ let statetree = { }; describe("hooks", () => { - let router; + let router: UIRouter; let $state: StateService; let $transitions: TransitionService; let states: StateDeclaration[]; @@ -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(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); + }); }); \ No newline at end of file