fix(transitionHook): Prevent queued hookFn to be called if deregistered (#2939)

* fix(transitionHook): Prevent queued hookFn to be called if deregistered

Fix a bug where queued hooks are called even if they are deregistered

* fix(transitionHook): Update tests for API change

Closes #2928

Author: elboman

src/transition/hookBuilder.ts

@@ -98,7 +98,7 @@ export class HookBuilder {
        return matchingNodes.map(node => {
          let _options = extend({ bind: hook.bind, traceData: { hookType, context: node} }, this.baseHookOptions, options);
          let state = _options.stateHook ? node.state : null;
-         let transitionHook = new TransitionHook(this.transition, state, hook.callback, _options);
+         let transitionHook = new TransitionHook(this.transition, state, hook, _options);
          return <HookTuple> { hook, node, transitionHook };
        });
     };

src/transition/hookRegistry.ts

@@ -49,12 +49,14 @@ export class EventHook implements IEventHook {
   matchCriteria: HookMatchCriteria;
   priority: number;
   bind: any;
+  _deregistered: boolean;
 
   constructor(matchCriteria: HookMatchCriteria, callback: HookFn, options: HookRegOptions = <any>{}) {
     this.callback = callback;
     this.matchCriteria = extend({ to: true, from: true, exiting: true, retained: true, entering: true }, matchCriteria);
     this.priority = options.priority || 0;
     this.bind = options.bind || null;
+    this._deregistered = false;
   }
 
   private static _matchingNodes(nodes: PathNode[], criterion: HookMatchCriterion): PathNode[] {
@@ -99,6 +101,7 @@ function makeHookRegistrationFn(hooks: ITransitionEvents, name: string): IHookRe
     hooks[name].push(eventHook);
 
     return function deregisterEventHook() {
+      eventHook._deregistered = true;
       removeFrom(hooks[name])(eventHook);
     };
   };

src/transition/interface.ts

@@ -762,4 +762,5 @@ export interface IEventHook {
   priority?: number;
   bind?: any;
   matches:  (treeChanges: TreeChanges) => IMatchingNodes;
+  _deregistered: boolean;
 }

src/transition/transitionHook.ts

@@ -1,5 +1,5 @@
 /** @module transition */ /** for typedoc */
-import {TransitionHookOptions, HookFn, HookResult} from "./interface";
+import {TransitionHookOptions, IEventHook, HookResult} from "./interface";
 import {defaults, noop} from "../common/common";
 import {fnToString, maxLength} from "../common/strings";
 import {isDefined, isPromise } from "../common/predicates";
@@ -26,21 +26,23 @@ let defaultOptions: TransitionHookOptions = {
 export class TransitionHook {
   constructor(private transition: Transition,
               private stateContext: State,
-              private hookFn: HookFn,
+              private eventHook: IEventHook,
               private options: TransitionHookOptions) {
     this.options = defaults(options, defaultOptions);
   }
 
   private isSuperseded = () => this.options.current() !== this.options.transition;
 
   invokeHook(): Promise<HookResult> {
-    let { options, hookFn } = this;
+    let { options, eventHook } = this;
     trace.traceHookInvocation(this, options);
     if (options.rejectIfSuperseded && this.isSuperseded()) {
       return Rejection.superseded(options.current()).toPromise();
     }
 
-    let hookResult = hookFn.call(options.bind, this.transition, this.stateContext);
+    let hookResult = !eventHook._deregistered
+      ? eventHook.callback.call(options.bind, this.transition, this.stateContext)
+      : undefined;
     return this.handleHookResult(hookResult);
   }
 
@@ -73,10 +75,10 @@ export class TransitionHook {
   }
 
   toString() {
-    let { options, hookFn } = this;
+    let { options, eventHook } = this;
     let event = parse("traceData.hookType")(options) || "internal",
         context = parse("traceData.context.state.name")(options) || parse("traceData.context")(options) || "unknown",
-        name = fnToString(hookFn);
+        name = fnToString(eventHook.callback);
     return `${event} context: ${context}, ${maxLength(200, name)}`;
   }
 

test/core/hookBuilderSpec.ts

@@ -62,7 +62,7 @@ describe('HookBuilder:', function() {
     expect(typeof callback).toBe('function')
   });
 
-  const getFn = x => x['hookFn'];
+  const getFn = x => x['eventHook']['callback'];
 
   describe('HookMatchCriteria', function() {