Merge pull request #306 from tildeio/refactor-transition-state-resolution

Author: Robert Jackson

lib/router/route-info.ts

@@ -14,6 +14,25 @@ interface IModel {
   id?: string | number;
 }
 
+interface AbortableTransition<T extends boolean, R extends Route> extends InternalTransition<R> {
+  isAborted: T;
+}
+
+function checkForAbort<U, T extends AbortableTransition<I, R>, R extends Route, I extends boolean>(
+  transition: T,
+  value: U
+): I extends true ? never : U;
+function checkForAbort<U, T extends AbortableTransition<I, R>, R extends Route, I extends boolean>(
+  transition: T,
+  value: U
+): never | U {
+  if (transition.isAborted) {
+    throw new Error('Transition aborted');
+  }
+
+  return value;
+}
+
 export interface Route {
   inaccessibleByURL?: boolean;
   routeName: string;
@@ -40,8 +59,6 @@ export interface Route {
   buildRouteInfoMetadata?(): unknown;
 }
 
-export type Continuation = () => PromiseLike<boolean> | boolean;
-
 export interface RouteInfo {
   readonly name: string;
   readonly parent: RouteInfo | RouteInfoWithAttributes | null;
@@ -222,16 +239,13 @@ export default class InternalRouteInfo<T extends Route> {
     return this.params || {};
   }
 
-  resolve(
-    shouldContinue: Continuation,
-    transition: InternalTransition<T>
-  ): Promise<ResolvedRouteInfo<T>> {
+  resolve(transition: InternalTransition<T>): Promise<ResolvedRouteInfo<T>> {
     return Promise.resolve(this.routePromise)
-      .then((route: Route) => this.checkForAbort(shouldContinue, route))
+      .then((route: Route) => checkForAbort(transition, route))
       .then(() => this.runBeforeModelHook(transition))
-      .then(() => this.checkForAbort(shouldContinue, null))
+      .then(() => checkForAbort(transition, null))
       .then(() => this.getModel(transition))
-      .then((resolvedModel) => this.checkForAbort(shouldContinue, resolvedModel))
+      .then((resolvedModel) => checkForAbort(transition, resolvedModel))
       .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel))
       .then((resolvedModel) => this.becomeResolved(transition, resolvedModel));
   }
@@ -375,14 +389,6 @@ export default class InternalRouteInfo<T extends Route> {
     });
   }
 
-  private checkForAbort<T>(shouldContinue: Continuation, value: T) {
-    return Promise.resolve(shouldContinue()).then(function () {
-      // We don't care about shouldContinue's resolve value;
-      // pass along the original value passed to this fn.
-      return value;
-    }, null);
-  }
-
   private stashResolvedModel(transition: InternalTransition<T>, resolvedModel: Dict<unknown>) {
     transition.resolvedModels = transition.resolvedModels || {};
     transition.resolvedModels[this.name] = resolvedModel;
@@ -429,10 +435,7 @@ export class ResolvedRouteInfo<T extends Route> extends InternalRouteInfo<T> {
     this.context = context;
   }
 
-  resolve(
-    _shouldContinue: Continuation,
-    transition: InternalTransition<T>
-  ): Promise<InternalRouteInfo<T>> {
+  resolve(transition: InternalTransition<T>): Promise<InternalRouteInfo<T>> {
     // A ResolvedRouteInfo just resolved with itself.
     if (transition && transition.resolvedModels) {
       transition.resolvedModels[this.name] = this.context as Dict<unknown>;

lib/router/transition-state.ts

@@ -1,13 +1,84 @@
 import { Promise } from 'rsvp';
 import { Dict } from './core';
-import InternalRouteInfo, { Continuation, Route } from './route-info';
+import InternalRouteInfo, { Route, ResolvedRouteInfo } from './route-info';
 import Transition from './transition';
 import { forEach, promiseLabel } from './utils';
 
 interface IParams {
   [key: string]: unknown;
 }
 
+function handleError<T extends Route>(
+  currentState: TransitionState<T>,
+  transition: Transition<T>,
+  error: Error
+): never {
+  // This is the only possible
+  // reject value of TransitionState#resolve
+  let routeInfos = currentState.routeInfos;
+  let errorHandlerIndex =
+    transition.resolveIndex >= routeInfos.length ? routeInfos.length - 1 : transition.resolveIndex;
+
+  let wasAborted = transition.isAborted;
+
+  throw new TransitionError(
+    error,
+    currentState.routeInfos[errorHandlerIndex].route!,
+    wasAborted,
+    currentState
+  );
+}
+
+function resolveOneRouteInfo<T extends Route>(
+  currentState: TransitionState<T>,
+  transition: Transition<T>
+): void | Promise<void> {
+  if (transition.resolveIndex === currentState.routeInfos.length) {
+    // This is is the only possible
+    // fulfill value of TransitionState#resolve
+    return;
+  }
+
+  let routeInfo = currentState.routeInfos[transition.resolveIndex];
+
+  return routeInfo
+    .resolve(transition)
+    .then(proceed.bind(null, currentState, transition), null, currentState.promiseLabel('Proceed'));
+}
+
+function proceed<T extends Route>(
+  currentState: TransitionState<T>,
+  transition: Transition<T>,
+  resolvedRouteInfo: ResolvedRouteInfo<T>
+): void | Promise<void> {
+  let wasAlreadyResolved = currentState.routeInfos[transition.resolveIndex].isResolved;
+
+  // Swap the previously unresolved routeInfo with
+  // the resolved routeInfo
+  currentState.routeInfos[transition.resolveIndex++] = resolvedRouteInfo;
+
+  if (!wasAlreadyResolved) {
+    // Call the redirect hook. The reason we call it here
+    // vs. afterModel is so that redirects into child
+    // routes don't re-run the model hooks for this
+    // already-resolved route.
+    let { route } = resolvedRouteInfo;
+    if (route !== undefined) {
+      if (route.redirect) {
+        route.redirect(resolvedRouteInfo.context as Dict<unknown>, transition);
+      }
+    }
+  }
+
+  // Proceed after ensuring that the redirect hook
+  // didn't abort this transition by transitioning elsewhere.
+  if (transition.isAborted) {
+    throw new Error('Transition aborted');
+  }
+
+  return resolveOneRouteInfo(currentState, transition);
+}
+
 export default class TransitionState<T extends Route> {
   routeInfos: InternalRouteInfo<T>[] = [];
   queryParams: Dict<unknown> = {};
@@ -25,7 +96,7 @@ export default class TransitionState<T extends Route> {
     return promiseLabel("'" + targetName + "': " + label);
   }
 
-  resolve(shouldContinue: Continuation, transition: Transition<T>): Promise<TransitionState<T>> {
+  resolve(transition: Transition<T>): Promise<TransitionState<T>> {
     // First, calculate params for this state. This is useful
     // information to provide to the various route hooks.
     let params = this.params;
@@ -36,87 +107,15 @@ export default class TransitionState<T extends Route> {
 
     transition.resolveIndex = 0;
 
-    let currentState = this;
-    let wasAborted = false;
-
     // The prelude RSVP.resolve() async moves us into the promise land.
     return Promise.resolve(null, this.promiseLabel('Start transition'))
-      .then(resolveOneRouteInfo, null, this.promiseLabel('Resolve route'))
-      .catch(handleError, this.promiseLabel('Handle error'));
-
-    function innerShouldContinue() {
-      return Promise.resolve(
-        shouldContinue(),
-        currentState.promiseLabel('Check if should continue')
-      ).catch(function (reason) {
-        // We distinguish between errors that occurred
-        // during resolution (e.g. before"Model/model/afterModel),
-        // and aborts due to a rejecting promise from shouldContinue().
-        wasAborted = true;
-        return Promise.reject(reason);
-      }, currentState.promiseLabel('Handle abort'));
-    }
-
-    function handleError(error: Error) {
-      // This is the only possible
-      // reject value of TransitionState#resolve
-      let routeInfos = currentState.routeInfos;
-      let errorHandlerIndex =
-        transition.resolveIndex >= routeInfos.length
-          ? routeInfos.length - 1
-          : transition.resolveIndex;
-      return Promise.reject(
-        new TransitionError(
-          error,
-          currentState.routeInfos[errorHandlerIndex].route!,
-          wasAborted,
-          currentState
-        )
-      );
-    }
-
-    function proceed(resolvedRouteInfo: InternalRouteInfo<T>): Promise<InternalRouteInfo<T>> {
-      let wasAlreadyResolved = currentState.routeInfos[transition.resolveIndex].isResolved;
-
-      // Swap the previously unresolved routeInfo with
-      // the resolved routeInfo
-      currentState.routeInfos[transition.resolveIndex++] = resolvedRouteInfo;
-
-      if (!wasAlreadyResolved) {
-        // Call the redirect hook. The reason we call it here
-        // vs. afterModel is so that redirects into child
-        // routes don't re-run the model hooks for this
-        // already-resolved route.
-        let { route } = resolvedRouteInfo;
-        if (route !== undefined) {
-          if (route.redirect) {
-            route.redirect(resolvedRouteInfo.context as Dict<unknown>, transition);
-          }
-        }
-      }
-
-      // Proceed after ensuring that the redirect hook
-      // didn't abort this transition by transitioning elsewhere.
-      return innerShouldContinue().then(
-        resolveOneRouteInfo,
+      .then(
+        resolveOneRouteInfo.bind(null, this, transition),
         null,
-        currentState.promiseLabel('Resolve route')
-      );
-    }
-
-    function resolveOneRouteInfo(): TransitionState<T> | Promise<any> {
-      if (transition.resolveIndex === currentState.routeInfos.length) {
-        // This is is the only possible
-        // fulfill value of TransitionState#resolve
-        return currentState;
-      }
-
-      let routeInfo = currentState.routeInfos[transition.resolveIndex];
-
-      return routeInfo
-        .resolve(innerShouldContinue, transition)
-        .then(proceed, null, currentState.promiseLabel('Proceed'));
-    }
+        this.promiseLabel('Resolve route')
+      )
+      .catch(handleError.bind(null, this, transition), this.promiseLabel('Handle error'))
+      .then(() => this);
   }
 }
 

lib/router/transition.ts

@@ -169,17 +169,11 @@ export default class Transition<T extends Route> implements Partial<Promise<T>>
       }
 
       this.sequence = router.currentSequence++;
-      this.promise = state
-        .resolve(() => {
-          if (this.isAborted) {
-            return Promise.reject(false, promiseLabel('Transition aborted - reject'));
-          }
-
-          return Promise.resolve(true);
-        }, this)
-        .catch((result: TransitionError) => {
-          return Promise.reject(this.router.transitionDidError(result, this));
-        }, promiseLabel('Handle Abort'));
+      this.promise = state.resolve(this).catch((result: TransitionError) => {
+        let error = this.router.transitionDidError(result, this);
+
+        throw error;
+      }, promiseLabel('Handle Abort'));
     } else {
       this.promise = Promise.resolve(this[STATE_SYMBOL]!);
       this[PARAMS_SYMBOL] = {};