From 39e1ba79ce297e90ff1617578dff0731e4ba2e24 Mon Sep 17 00:00:00 2001
From: Marco Botto <marco.botto@gmail.com>
Date: Wed, 24 Aug 2016 00:45:33 +0200
Subject: [PATCH] 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
---
 src/transition/hookBuilder.ts    |  2 +-
 src/transition/hookRegistry.ts   |  3 +++
 src/transition/interface.ts      |  1 +
 src/transition/transitionHook.ts | 14 ++++++++------
 test/core/hookBuilderSpec.ts     |  2 +-
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/transition/hookBuilder.ts b/src/transition/hookBuilder.ts
index 1c69ebc90..9d3dafa31 100644
--- a/src/transition/hookBuilder.ts
+++ b/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 };
        });
     };
diff --git a/src/transition/hookRegistry.ts b/src/transition/hookRegistry.ts
index 0e5c1171a..c35af09de 100644
--- a/src/transition/hookRegistry.ts
+++ b/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);
     };
   };
diff --git a/src/transition/interface.ts b/src/transition/interface.ts
index 2d2558fdc..89edd8d96 100644
--- a/src/transition/interface.ts
+++ b/src/transition/interface.ts
@@ -762,4 +762,5 @@ export interface IEventHook {
   priority?: number;
   bind?: any;
   matches:  (treeChanges: TreeChanges) => IMatchingNodes;
+  _deregistered: boolean;
 }
\ No newline at end of file
diff --git a/src/transition/transitionHook.ts b/src/transition/transitionHook.ts
index 44f86b9ee..7bdbf1dcd 100644
--- a/src/transition/transitionHook.ts
+++ b/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,7 +26,7 @@ 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);
   }
@@ -34,13 +34,15 @@ export class TransitionHook {
   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)}`;
   }
 
diff --git a/test/core/hookBuilderSpec.ts b/test/core/hookBuilderSpec.ts
index 1ecd6ee8b..62a0c2b51 100644
--- a/test/core/hookBuilderSpec.ts
+++ b/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() {