refactor(RegisteredHook): remove IEventHook; rename EventHook to RegisteredHook

refactor(TransitionHook): Remove TransitionHookOptions.rejectIfSuperseded

Author: christopherthielen

src/common/trace.ts

@@ -168,7 +168,7 @@ export class Trace {
         digest = this.approximateDigests,
         event = parse("traceData.hookType")(options) || "internal",
         context = parse("traceData.context.state.name")(options) || parse("traceData.context")(options) || "unknown",
-        name = functionToString((step as any).eventHook.callback);
+        name = functionToString((step as any).registeredHook.callback);
     console.log(`Transition #${tid} Digest #${digest}:   Hook -> ${event} context: ${context}, ${maxLength(200, name)}`);
   }
 

src/transition/hookBuilder.ts

@@ -4,7 +4,7 @@ import {extend, tail, assertPredicate, unnestR, identity} from "../common/common
 import {isArray} from "../common/predicates";
 
 import {
-    TransitionOptions, TransitionHookOptions, IHookRegistry, TreeChanges, IEventHook, IMatchingNodes,
+    TransitionOptions, TransitionHookOptions, IHookRegistry, TreeChanges, IMatchingNodes,
     TransitionHookPhase, TransitionHookScope
 } from "./interface";
 
@@ -14,15 +14,16 @@ import {State} from "../state/stateObject";
 import {PathNode} from "../path/node";
 import {TransitionService} from "./transitionService";
 import {TransitionHookType} from "./transitionHookType";
+import {RegisteredHook} from "./hookRegistry";
 
 /**
  * This class returns applicable TransitionHooks for a specific Transition instance.
  *
- * Hooks (IEventHook) may be registered globally, e.g., $transitions.onEnter(...), or locally, e.g.
- * myTransition.onEnter(...).  The HookBuilder finds matching IEventHooks (where the match criteria is
+ * Hooks ([[RegisteredHook]]) may be registered globally, e.g., $transitions.onEnter(...), or locally, e.g.
+ * myTransition.onEnter(...).  The HookBuilder finds matching RegisteredHooks (where the match criteria is
  * determined by the type of hook)
  *
- * The HookBuilder also converts IEventHooks objects to TransitionHook objects, which are used to run a Transition.
+ * The HookBuilder also converts RegisteredHooks objects to TransitionHook objects, which are used to run a Transition.
  *
  * The HookBuilder constructor is given the $transitions service and a Transition instance.  Thus, a HookBuilder
  * instance may only be used for one specific Transition object. (side note: the _treeChanges accessor is private
@@ -62,7 +63,7 @@ export class HookBuilder {
   /**
    * Returns an array of newly built TransitionHook objects.
    *
-   * - Finds all IEventHooks registered for the given `hookType` which matched the transition's [[TreeChanges]].
+   * - Finds all RegisteredHooks registered for the given `hookType` which matched the transition's [[TreeChanges]].
    * - Finds [[PathNode]] (or `PathNode[]`) to use as the TransitionHook context(s)
    * - For each of the [[PathNode]]s, creates a TransitionHook
    *
@@ -73,7 +74,7 @@ export class HookBuilder {
     let matchingHooks = this.getMatchingHooks(hookType, this.treeChanges);
     if (!matchingHooks) return [];
 
-     const makeTransitionHooks = (hook: IEventHook) => {
+     const makeTransitionHooks = (hook: RegisteredHook) => {
        // Fetch the Nodes that caused this hook to match.
        let matches: IMatchingNodes = hook.matches(this.treeChanges);
        // Select the PathNode[] that will be used as TransitionHook context objects
@@ -87,7 +88,7 @@ export class HookBuilder {
          }, this.baseHookOptions);
 
          let state = hookType.hookScope === TransitionHookScope.STATE ? node.state : null;
-         let transitionHook = new TransitionHook(this.transition, state, hook, hookType, _options);
+         let transitionHook = new TransitionHook(this.transition, state, hook, _options);
          return <HookTuple> { hook, node, transitionHook };
        });
     };
@@ -99,30 +100,30 @@ export class HookBuilder {
   }
 
   /**
-   * Finds all IEventHooks from:
+   * Finds all RegisteredHooks from:
    * - The Transition object instance hook registry
    * - The TransitionService ($transitions) global hook registry
    *
    * which matched:
    * - the eventType
    * - the matchCriteria (to, from, exiting, retained, entering)
    *
-   * @returns an array of matched [[IEventHook]]s
+   * @returns an array of matched [[RegisteredHook]]s
    */
-  public getMatchingHooks(hookType: TransitionHookType, treeChanges: TreeChanges): IEventHook[] {
+  public getMatchingHooks(hookType: TransitionHookType, treeChanges: TreeChanges): RegisteredHook[] {
     let isCreate = hookType.hookPhase === TransitionHookPhase.CREATE;
 
     // Instance and Global hook registries
     let registries = isCreate ? [ this.$transitions ] : [ this.transition, this.$transitions ];
 
     return registries.map((reg: IHookRegistry) => reg.getHooks(hookType.name))    // Get named hooks from registries
         .filter(assertPredicate(isArray, `broken event named: ${hookType.name}`)) // Sanity check
-        .reduce(unnestR, [])                                                      // Un-nest IEventHook[][] to IEventHook[] array
+        .reduce(unnestR, [])                                                      // Un-nest RegisteredHook[][] to RegisteredHook[] array
         .filter(hook => hook.matches(treeChanges));                               // Only those satisfying matchCriteria
   }
 }
 
-interface HookTuple { hook: IEventHook, node: PathNode, transitionHook: TransitionHook }
+interface HookTuple { hook: RegisteredHook, node: PathNode, transitionHook: TransitionHook }
 
 /**
  * A factory for a sort function for HookTuples.

src/transition/hookRegistry.ts

@@ -5,11 +5,12 @@ import {PathNode} from "../path/node";
 import {TransitionStateHookFn, TransitionHookFn} from "./interface"; // has or is using
 
 import {
-    HookRegOptions, HookMatchCriteria, IEventHook, IHookRegistry, IHookRegistration, TreeChanges,
+    HookRegOptions, HookMatchCriteria, IHookRegistration, TreeChanges,
     HookMatchCriterion, IMatchingNodes, HookFn
 } from "./interface";
 import {Glob} from "../common/glob";
 import {State} from "../state/stateObject";
+import {TransitionHookType} from "./transitionHookType";
 
 /**
  * Determines if the given state matches the matchCriteria
@@ -43,15 +44,23 @@ export function matchState(state: State, criterion: HookMatchCriterion) {
   return !!matchFn(state);
 }
 
-/** @hidden */
-export class EventHook implements IEventHook {
+/**
+ * @hidden
+ * The registration data for a registered transition hook
+ */
+export class RegisteredHook implements RegisteredHook {
+  hookType: TransitionHookType;
   callback: HookFn;
   matchCriteria: HookMatchCriteria;
   priority: number;
   bind: any;
   _deregistered: boolean;
 
-  constructor(matchCriteria: HookMatchCriteria, callback: HookFn, options: HookRegOptions = <any>{}) {
+  constructor(hookType: TransitionHookType,
+              matchCriteria: HookMatchCriteria,
+              callback: HookFn,
+              options: HookRegOptions = <any>{}) {
+    this.hookType = hookType;
     this.callback = callback;
     this.matchCriteria = extend({ to: true, from: true, exiting: true, retained: true, entering: true }, matchCriteria);
     this.priority = options.priority || 0;
@@ -72,7 +81,7 @@ export class EventHook implements IEventHook {
    * are the matching [[PathNode]]s for each [[HookMatchCriterion]] (to, from, exiting, retained, entering)
    */
   matches(treeChanges: TreeChanges): IMatchingNodes {
-    let mc = this.matchCriteria, _matchingNodes = EventHook._matchingNodes;
+    let mc = this.matchCriteria, _matchingNodes = RegisteredHook._matchingNodes;
 
     let matches: IMatchingNodes = {
       to:       _matchingNodes([tail(treeChanges.to)], mc.to),
@@ -92,20 +101,22 @@ export class EventHook implements IEventHook {
 }
 
 /** @hidden */
-export interface IEventHooks {
-  [key: string]: IEventHook[];
+export interface RegisteredHooks {
+  [key: string]: RegisteredHook[];
 }
 
 /** @hidden Return a registration function of the requested type. */
-export function makeHookRegistrationFn(hooks: IEventHooks, name:string): IHookRegistration {
-  hooks[name] = [];
+export function makeHookRegistrationFn(registeredHooks: RegisteredHooks, type: TransitionHookType): IHookRegistration {
+  let name = type.name;
+  registeredHooks[name] = [];
+
   return function (matchObject, callback, options = {}) {
-    let eventHook = new EventHook(matchObject, callback, options);
-    hooks[name].push(eventHook);
+    let registeredHook = new RegisteredHook(type, matchObject, callback, options);
+    registeredHooks[name].push(registeredHook);
 
     return function deregisterEventHook() {
-      eventHook._deregistered = true;
-      removeFrom(hooks[name])(eventHook);
+      registeredHook._deregistered = true;
+      removeFrom(registeredHooks[name])(registeredHook);
     };
   };
 }

src/transition/interface.ts

@@ -6,6 +6,7 @@ import {Transition} from "./transition";
 import {State} from "../state/stateObject";
 import {PathNode} from "../path/node";
 import {TargetState} from "../state/targetState";
+import {RegisteredHook} from "./hookRegistry";
 
 /**
  * The TransitionOptions object can be used to change the behavior of a transition.
@@ -86,8 +87,6 @@ export interface TransitionOptions {
 
 /** @internalapi */
 export interface TransitionHookOptions {
-  async               ?: boolean;
-  rejectIfSuperseded  ?: boolean;
   current             ?: () => Transition;  //path?
   transition          ?: Transition;
   hookType            ?: string;
@@ -694,7 +693,7 @@ export interface IHookRegistry {
    * $transitions.getHooks("onEnter")
    * ```
    */
-  getHooks(hookName: string): IEventHook[];
+  getHooks(hookName: string): RegisteredHook[];
 }
 
 /** A predicate type which takes a [[State]] and returns a boolean */
@@ -791,14 +790,5 @@ export interface IMatchingNodes {
  */
 export type HookMatchCriterion = (string|IStateMatch|boolean)
 
-/** @hidden */
-export interface IEventHook {
-  callback: HookFn;
-  priority?: number;
-  bind?: any;
-  matches:  (treeChanges: TreeChanges) => IMatchingNodes;
-  _deregistered: boolean;
-}
-
 export enum TransitionHookPhase { CREATE, BEFORE, ASYNC, SUCCESS, ERROR }
 export enum TransitionHookScope { TRANSITION, STATE }

src/transition/transition.ts

@@ -11,14 +11,14 @@ import { prop, propEq, val, not } from "../common/hof";
 
 import {StateDeclaration, StateOrName} from "../state/interface";
 import {
-    TransitionOptions, TreeChanges, IHookRegistry, IEventHook, TransitionHookPhase,
+    TransitionOptions, TreeChanges, IHookRegistry, TransitionHookPhase,
     TransitionCreateHookFn
 } from "./interface";
 
 import { TransitionStateHookFn, TransitionHookFn } from "./interface"; // has or is using
 
 import {TransitionHook} from "./transitionHook";
-import {matchState, IEventHooks, makeHookRegistrationFn} from "./hookRegistry";
+import {matchState, RegisteredHooks, makeHookRegistrationFn, RegisteredHook} from "./hookRegistry";
 import {HookBuilder} from "./hookBuilder";
 import {PathNode} from "../path/node";
 import {PathFactory} from "../path/pathFactory";
@@ -86,7 +86,7 @@ export class Transition implements IHookRegistry {
   private _error: any;
 
   /** @hidden Holds the hook registration functions such as those passed to Transition.onStart() */
-  private _transitionHooks: IEventHooks = { };
+  private _transitionHooks: RegisteredHooks = { };
 
   /** @hidden */
   private _options: TransitionOptions;
@@ -120,11 +120,11 @@ export class Transition implements IHookRegistry {
   private createTransitionHookRegFns() {
     this.router.transitionService.getTransitionHookTypes()
         .filter(type => type.hookPhase !== TransitionHookPhase.CREATE)
-        .forEach(type => this[type.name] =  makeHookRegistrationFn(this._transitionHooks, type.name));
+        .forEach(type => this[type.name] =  makeHookRegistrationFn(this._transitionHooks, type));
   }
 
   /** @hidden @internalapi */
-  getHooks(hookName: string): IEventHook[] {
+  getHooks(hookName: string): RegisteredHook[] {
     return this._transitionHooks[hookName];
   }
 

src/transition/transitionHook.ts

@@ -1,5 +1,5 @@
 /** @coreapi @module transition */ /** for typedoc */
-import {TransitionHookOptions, IEventHook, HookResult} from "./interface";
+import {TransitionHookOptions, HookResult} from "./interface";
 import {defaults, noop, identity} from "../common/common";
 import {fnToString, maxLength} from "../common/strings";
 import {isPromise} from "../common/predicates";
@@ -11,12 +11,11 @@ import {Rejection} from "./rejectFactory";
 import {TargetState} from "../state/targetState";
 import {Transition} from "./transition";
 import {State} from "../state/stateObject";
+import {TransitionHookType} from "./transitionHookType";
 import {StateService} from "../state/stateService"; // has or is using
-import {TransitionHookType} from "./transitionHookType"; // has or is using
+import {RegisteredHook} from "./hookRegistry"; // has or is using
 
 let defaultOptions: TransitionHookOptions = {
-  async: true,
-  rejectIfSuperseded: true,
   current: noop,
   transition: null,
   traceData: {},
@@ -33,8 +32,7 @@ export type ErrorHandler  = (error)              => Promise<any>;
 export class TransitionHook {
   constructor(private transition: Transition,
               private stateContext: State,
-              private eventHook: IEventHook,
-              private hookType: TransitionHookType,
+              private registeredHook: RegisteredHook,
               private options: TransitionHookOptions) {
     this.options = defaults(options, defaultOptions);
   }
@@ -59,43 +57,40 @@ export class TransitionHook {
   static THROW_ERROR: GetErrorHandler = (hook: TransitionHook) =>
       undefined;
 
-  private rejectForSuperseded = () =>
-      this.hookType.rejectIfSuperseded && this.options.current() !== this.options.transition;
+  private rejectIfSuperseded = () =>
+      this.registeredHook.hookType.rejectIfSuperseded && this.options.current() !== this.options.transition;
 
   invokeHook(): Promise<HookResult> {
-    if (this.eventHook._deregistered) return;
+    let hook = this.registeredHook;
+    if (hook._deregistered) return;
 
     let options = this.options;
     trace.traceHookInvocation(this, options);
 
-    if (this.rejectForSuperseded()) {
+    if (this.rejectIfSuperseded()) {
       return Rejection.superseded(options.current()).toPromise();
     }
 
-    let errorHandler  = this.hookType.errorHandler(this);
-    let resultHandler = this.hookType.resultHandler(this);
-
-    return this._invokeCallback(resultHandler, errorHandler);
-  }
-  
-  private _invokeCallback(resultHandler: ResultHandler, errorHandler: ErrorHandler): Promise<HookResult> {
-    let cb = this.eventHook.callback;
+    let cb = hook.callback;
     let bind = this.options.bind;
     let trans = this.transition;
     let state = this.stateContext;
+
+    let errorHandler  = hook.hookType.getErrorHandler(this);
+    let resultHandler = hook.hookType.getResultHandler(this);
     resultHandler = resultHandler || identity;
 
     if (!errorHandler) {
       return resultHandler(cb.call(bind, trans, state));
-    } 
-    
+    }
+
     try {
       return resultHandler(cb.call(bind, trans, state));
     } catch (error) {
       return errorHandler(error);
     }
   }
-
+  
   /**
    * This method handles the return value of a Transition Hook.
    *
@@ -108,7 +103,7 @@ export class TransitionHook {
   handleHookResult(result: HookResult): Promise<HookResult> {
     // This transition is no longer current.
     // Another transition started while this hook was still running.
-    if (this.rejectForSuperseded()) {
+    if (this.rejectIfSuperseded()) {
       // Abort this transition
       return Rejection.superseded(this.options.current()).toPromise();
     }
@@ -136,10 +131,10 @@ export class TransitionHook {
   }
 
   toString() {
-    let { options, eventHook } = this;
+    let { options, registeredHook } = this;
     let event = parse("traceData.hookType")(options) || "internal",
         context = parse("traceData.context.state.name")(options) || parse("traceData.context")(options) || "unknown",
-        name = fnToString(eventHook.callback);
+        name = fnToString(registeredHook.callback);
     return `${event} context: ${context}, ${maxLength(200, name)}`;
   }