fix(transitionHook): Do not process transition hooks after router has been disposed.

Author: christopherthielen

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);

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<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)) {
@@ -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",

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<any>(resolve => setTimeout(resolve, ms));
+export const _delay = (ms) => () => delay(ms);
+
 export function tree2Array(tree, inheritName) {
 
   function processState(parent, state, name) {

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<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);
+  });
 });