feat(onBefore): Run onBefore hooks asynchronously.

BREAKING CHANGE: `onBefore` hooks are now run asynchronously like all the other hooks. - #### Old behavior Previously, the `onBefore` hooks were run in the same stackframe as `transitionTo`. If they threw an error, it could be caught using try/catch. ```js transitionService.onBefore({ to: 'foo' }), () => { throw new Error('doh'); }); try { stateService.go('foo'); } catch (error) { // handle error } ``` - #### New behavior Now, `onBefore` hooks are processed asynchronously. To handle errors, use any of the async error handling paradigms: - Chain off the promise ```js transitionService.onBefore({ to: 'foo' }), () => { throw new Error('doh'); }); stateService.go('foo').catch(error => { //handle error }); ``` - Define an error handler ```js transitionService.onBefore({ to: 'foo' }), () => { throw new Error('doh'); }); transitionService.onError({ to: 'foo' }), () => { // handle error }); stateService.go('foo'); ``` - Use the global defaultErrorHandler ```js transitionService.onBefore({ to: 'foo' }), () => { throw new Error('doh'); }); stateService.go('foo'); stateService.defaultErrorHandler(error => { // global error handler }); ``` - #### Motivation Why introduce a BC? - No subtle behavior differences by hook type - Simpler code and mental model - Fewer edge cases to account for - #### BC Liklihood How likely is this to affect my app? Very Low: Apps that registered onBefore hooks and depend on synchronous execution are affected. - #### BC Severity How severe is this BC? Low: Switch to asynchronous handling, such as chaining off the transition promise
ui-router · Feb 23, 2017 · 30b82aa · 30b82aa
1 parent 07c0ae4
commit 30b82aa
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 99 deletions.
diff --git a/src/hooks/ignoredTransition.ts b/src/hooks/ignoredTransition.ts
@@ -0,0 +1,23 @@
+
+import { trace } from '../common/trace';
+import { Rejection } from '../transition/rejectFactory';
+import { TransitionService } from '../transition/transitionService';
+import { Transition } from '../transition/transition';
+
+/**
+ * A [[TransitionHookFn]] that skips a transition if it should be ignored
+ *
+ * This hook is invoked at the end of the onBefore phase.
+ *
+ * If the transition should be ignored (because no parameter or states changed)
+ * then the transition is ignored and not processed.
+ */
+function ignoredHook(trans: Transition) {
+  if (trans.ignored()) {
+    trace.traceTransitionIgnored(this);
+    return Rejection.ignored().toPromise();
+  }
+}
+
+export const registerIgnoredTransitionHook = (transitionService: TransitionService) =>
+    transitionService.onBefore({}, ignoredHook, { priority: -9999 });
diff --git a/src/hooks/invalidTransition.ts b/src/hooks/invalidTransition.ts
@@ -0,0 +1,18 @@
+import { TransitionService } from '../transition/transitionService';
+import { Transition } from '../transition/transition';
+
+/**
+ * A [[TransitionHookFn]] that rejects the Transition if it is invalid
+ *
+ * This hook is invoked at the end of the onBefore phase.
+ * If the transition is invalid (for example, param values do not validate)
+ * then the transition is rejected.
+ */
+function invalidTransitionHook(trans: Transition) {
+  if (!trans.valid()) {
+    throw new Error(trans.error());
+  }
+}
+
+export const registerInvalidTransitionHook = (transitionService: TransitionService) =>
+    transitionService.onBefore({}, invalidTransitionHook, { priority: -10000 });
diff --git a/src/transition/interface.ts b/src/transition/interface.ts
@@ -816,5 +816,5 @@ export interface PathType {
  */
 export type HookMatchCriterion = (string|IStateMatch|boolean)
 
-export enum TransitionHookPhase { CREATE, BEFORE, ASYNC, SUCCESS, ERROR }
+export enum TransitionHookPhase { CREATE, BEFORE, RUN, SUCCESS, ERROR }
 export enum TransitionHookScope { TRANSITION, STATE }
diff --git a/src/transition/transition.ts b/src/transition/transition.ts
@@ -1,41 +1,35 @@
 /**
  * @coreapi
  * @module transition
- */ /** for typedoc */
-import {stringify} from "../common/strings";
-import {trace} from "../common/trace";
-import {services} from "../common/coreservices";
+ */
+/** for typedoc */
+import { trace } from '../common/trace';
+import { services } from '../common/coreservices';
 import {
-    map, find, extend, mergeR, tail,
-    omit, toJson, arrayTuples, unnestR, identity, anyTrueR
-} from "../common/common";
-import { isObject, isArray } from "../common/predicates";
-import { prop, propEq, val, not, is } from "../common/hof";
-
-import {StateDeclaration, StateOrName} from "../state/interface";
+  map, find, extend, mergeR, tail, omit, toJson, arrayTuples, unnestR, identity, anyTrueR
+} from '../common/common';
+import { isObject } from '../common/predicates';
+import { prop, propEq, val, not, is } from '../common/hof';
+import { StateDeclaration, StateOrName } from '../state/interface';
 import {
-    TransitionOptions, TreeChanges, IHookRegistry, TransitionHookPhase,
-    RegisteredHooks, HookRegOptions, HookMatchCriteria
-} from "./interface";
-
-import { TransitionStateHookFn, TransitionHookFn } from "./interface"; // has or is using
-
-import {TransitionHook} from "./transitionHook";
-import {matchState, makeEvent, RegisteredHook} from "./hookRegistry";
-import {HookBuilder} from "./hookBuilder";
-import {PathNode} from "../path/node";
-import {PathFactory} from "../path/pathFactory";
-import {State} from "../state/stateObject";
-import {TargetState} from "../state/targetState";
-import {Param} from "../params/param";
-import {Resolvable} from "../resolve/resolvable";
-import {ViewConfig} from "../view/interface";
-import {Rejection} from "./rejectFactory";
-import {ResolveContext} from "../resolve/resolveContext";
-import {UIRouter} from "../router";
-import {UIInjector} from "../interface";
-import {RawParams} from "../params/interface";
-import { ResolvableLiteral } from "../resolve/interface";
+  TransitionOptions, TreeChanges, IHookRegistry, TransitionHookPhase, RegisteredHooks, HookRegOptions,
+  HookMatchCriteria, TransitionStateHookFn, TransitionHookFn
+} from './interface'; // has or is using
+import { TransitionHook } from './transitionHook';
+import { matchState, makeEvent, RegisteredHook } from './hookRegistry';
+import { HookBuilder } from './hookBuilder';
+import { PathNode } from '../path/node';
+import { PathFactory } from '../path/pathFactory';
+import { State } from '../state/stateObject';
+import { TargetState } from '../state/targetState';
+import { Param } from '../params/param';
+import { Resolvable } from '../resolve/resolvable';
+import { ViewConfig } from '../view/interface';
+import { ResolveContext } from '../resolve/resolveContext';
+import { UIRouter } from '../router';
+import { UIInjector } from '../interface';
+import { RawParams } from '../params/interface';
+import { ResolvableLiteral } from '../resolve/interface';
 
 /** @hidden */
 const stateSelf: (_state: State) => StateDeclaration = prop("self");
@@ -630,28 +624,6 @@ export class Transition implements IHookRegistry {
     let globals = this.router.globals;
     globals.transitionHistory.enqueue(this);
 
-    let onBeforeHooks = hookBuilder.buildHooksForPhase(TransitionHookPhase.BEFORE);
-    let syncResult = TransitionHook.runOnBeforeHooks(onBeforeHooks);
-
-    if (Rejection.isTransitionRejectionPromise(syncResult)) {
-      syncResult.catch(() => 0); // issue #2676
-      let rejectReason = (<any> syncResult)._transitionRejection;
-      this._deferred.reject(rejectReason);
-      return this.promise;
-    }
-
-    if (!this.valid()) {
-      let error = new Error(this.error());
-      this._deferred.reject(error);
-      return this.promise;
-    }
-
-    if (this.ignored()) {
-      trace.traceTransitionIgnored(this);
-      this._deferred.reject(Rejection.ignored());
-      return this.promise;
-    }
-
     // When the chain is complete, then resolve or reject the deferred
     const transitionSuccess = () => {
       trace.traceSuccess(this.$to(), this);
@@ -670,16 +642,16 @@ export class Transition implements IHookRegistry {
       runAllHooks(onErrorHooks);
     };
 
-    trace.traceTransitionStart(this);
-
-    // Chain the next hook off the previous
-    const appendHookToChain = (prev: Promise<any>, nextHook: TransitionHook) =>
-        prev.then(() => nextHook.invokeHook());
-
-    // Run the hooks, then resolve or reject the overall deferred in the .then() handler
-    let asyncHooks = hookBuilder.buildHooksForPhase(TransitionHookPhase.ASYNC);
+    // Builds a chain of transition hooks for the given phase
+    // Each hook is invoked after the previous one completes
+    const chainFor = (phase: TransitionHookPhase) =>
+        TransitionHook.chain(hookBuilder.buildHooksForPhase(phase));
 
-    asyncHooks.reduce(appendHookToChain, syncResult)
+    services.$q.when()
+        .then(() => chainFor(TransitionHookPhase.BEFORE))
+        // This waits to build the RUN hook chain until after the "BEFORE" hooks complete
+        // This allows a BEFORE hook to dynamically add RUN hooks via the Transition object.
+        .then(() => chainFor(TransitionHookPhase.RUN))
         .then(transitionSuccess, transitionError);
 
     return this.promise;

diff --git a/src/transition/transitionHook.ts b/src/transition/transitionHook.ts
@@ -143,34 +143,36 @@ export class TransitionHook {
   }
 
   /**
-   * Run all TransitionHooks, ignoring their return value.
+   * Chains together an array of TransitionHooks.
+   *
+   * Given a list of [[TransitionHook]] objects, chains them together.
+   * Each hook is invoked after the previous one completes.
+   *
+   * #### Example:
+   * ```js
+   * var hooks: TransitionHook[] = getHooks();
+   * let promise: Promise<any> = TransitionHook.chain(hooks);
+   *
+   * promise.then(handleSuccess, handleError);
+   * ```
+   *
+   * @param hooks the list of hooks to chain together
+   * @param waitFor if provided, the chain is `.then()`'ed off this promise
+   * @returns a `Promise` for sequentially invoking the hooks (in order)
    */
-  static runAllHooks(hooks: TransitionHook[]): void {
-    hooks.forEach(hook => hook.invokeHook());
+  static chain(hooks: TransitionHook[], waitFor?: Promise<any>): Promise<any> {
+    // Chain the next hook off the previous
+    const createHookChainR = (prev: Promise<any>, nextHook: TransitionHook) =>
+        prev.then(() => nextHook.invokeHook());
+    return hooks.reduce(createHookChainR, waitFor || services.$q.when());
   }
 
+
   /**
-   * Given an array of TransitionHooks, runs each one synchronously and sequentially.
-   * Should any hook return a Rejection synchronously, the remaining hooks will not run.
-   *
-   * Returns a promise chain composed of any promises returned from each hook.invokeStep() call
+   * Run all TransitionHooks, ignoring their return value.
    */
-  static runOnBeforeHooks(hooks: TransitionHook[]): Promise<any> {
-    let results: Promise<HookResult>[] = [];
-
-    for (let hook of hooks) {
-      let hookResult = hook.invokeHook();
-
-      if (Rejection.isTransitionRejectionPromise(hookResult)) {
-        // Break on first thrown error or false/TargetState
-        return hookResult;
-      }
-
-      results.push(hookResult);
-    }
-
-    return results
-        .filter(isPromise)
-        .reduce((chain: Promise<any>, promise: Promise<any>) => chain.then(val(promise)), services.$q.when());
+  static runAllHooks(hooks: TransitionHook[]): void {
+    hooks.forEach(hook => hook.invokeHook());
   }
+
 }
diff --git a/src/transition/transitionService.ts b/src/transition/transitionService.ts
@@ -27,6 +27,8 @@ import { isDefined } from "../common/predicates";
 import { removeFrom, values, createProxyFunctions } from "../common/common";
 import { Disposable } from "../interface"; // has or is using
 import { val } from "../common/hof";
+import { registerIgnoredTransitionHook } from '../hooks/ignoredTransition';
+import { registerInvalidTransitionHook } from '../hooks/invalidTransition';
 
 /**
  * The default [[Transition]] options.
@@ -170,6 +172,8 @@ export class TransitionService implements IHookRegistry, Disposable {
    */
   _deregisterHookFns: {
     addCoreResolves: Function;
+    ignored: Function;
+    invalid: Function;
     redirectTo: Function;
     onExit: Function;
     onRetain: Function;
@@ -196,9 +200,8 @@ export class TransitionService implements IHookRegistry, Disposable {
       'getHooks',
     ]);
 
-    this._defineDefaultPaths();
-    this._defineDefaultEvents();
-
+    this._defineCorePaths();
+    this._defineCoreEvents();
     this._registerCoreTransitionHooks();
   }
 
@@ -228,7 +231,7 @@ export class TransitionService implements IHookRegistry, Disposable {
   }
 
   /** @hidden */
-  private _defineDefaultEvents() {
+  private _defineCoreEvents() {
     const Phase = TransitionHookPhase;
     const TH = TransitionHook;
     const paths = this._criteriaPaths;
@@ -237,18 +240,18 @@ export class TransitionService implements IHookRegistry, Disposable {
 
     this._defineEvent("onBefore",  Phase.BEFORE,  0,   paths.to, false, TH.HANDLE_RESULT);
 
-    this._defineEvent("onStart",   Phase.ASYNC,   0,   paths.to);
-    this._defineEvent("onExit",    Phase.ASYNC,   100, paths.exiting, true);
-    this._defineEvent("onRetain",  Phase.ASYNC,   200, paths.retained);
-    this._defineEvent("onEnter",   Phase.ASYNC,   300, paths.entering);
-    this._defineEvent("onFinish",  Phase.ASYNC,   400, paths.to);
+    this._defineEvent("onStart",   Phase.RUN,     0,   paths.to);
+    this._defineEvent("onExit",    Phase.RUN,     100, paths.exiting, true);
+    this._defineEvent("onRetain",  Phase.RUN,     200, paths.retained);
+    this._defineEvent("onEnter",   Phase.RUN,     300, paths.entering);
+    this._defineEvent("onFinish",  Phase.RUN,     400, paths.to);
 
     this._defineEvent("onSuccess", Phase.SUCCESS, 0,   paths.to, false, TH.IGNORE_RESULT, TH.LOG_ERROR, false);
     this._defineEvent("onError",   Phase.ERROR,   0,   paths.to, false, TH.IGNORE_RESULT, TH.LOG_ERROR, false);
   }
 
   /** @hidden */
-  private _defineDefaultPaths() {
+  private _defineCorePaths() {
     const { STATE, TRANSITION } = TransitionHookScope;
 
     this._definePathType("to", TRANSITION);
@@ -318,6 +321,8 @@ export class TransitionService implements IHookRegistry, Disposable {
     let fns = this._deregisterHookFns;
 
     fns.addCoreResolves = registerAddCoreResolvables(this);
+    fns.ignored         = registerIgnoredTransitionHook(this);
+    fns.invalid         = registerInvalidTransitionHook(this);
 
     // Wire up redirectTo hook
     fns.redirectTo      = registerRedirectToHook(this);