Simplify TransitionState resolution system.

lib/router/route-info.ts

@@ -40,8 +40,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 +220,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) => this.checkForAbort(transition, route))
       .then(() => this.runBeforeModelHook(transition))
-      .then(() => this.checkForAbort(shouldContinue, null))
+      .then(() => this.checkForAbort(transition, null))
       .then(() => this.getModel(transition))
-      .then((resolvedModel) => this.checkForAbort(shouldContinue, resolvedModel))
+      .then((resolvedModel) => this.checkForAbort(transition, resolvedModel))
       .then((resolvedModel) => this.runAfterModelHook(transition, resolvedModel))
       .then((resolvedModel) => this.becomeResolved(transition, resolvedModel));
   }
@@ -375,12 +370,12 @@ 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 checkForAbort<U>(transition: InternalTransition<T>, value: U) {
+    if (transition.isAborted) {
+      throw new Error('Transition aborted');
+    }
+
+    return value;
   }
 
   private stashResolvedModel(transition: InternalTransition<T>, resolvedModel: Dict<unknown>) {
@@ -429,10 +424,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,6 +1,6 @@
 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';
 
@@ -25,7 +25,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;
@@ -37,45 +37,35 @@ 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'));
-    }
+      .catch(handleError, this.promiseLabel('Handle error'))
+      .then(() => {
+        return currentState;
+      });
 
-    function handleError(error: Error) {
+    function handleError(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;
-      return Promise.reject(
-        new TransitionError(
-          error,
-          currentState.routeInfos[errorHandlerIndex].route!,
-          wasAborted,
-          currentState
-        )
+
+      let wasAborted = transition.isAborted;
+
+      throw new TransitionError(
+        error,
+        currentState.routeInfos[errorHandlerIndex].route!,
+        wasAborted,
+        currentState
       );
     }
 
-    function proceed(resolvedRouteInfo: InternalRouteInfo<T>): Promise<InternalRouteInfo<T>> {
+    function proceed(resolvedRouteInfo: ResolvedRouteInfo<T>): void | Promise<void> {
       let wasAlreadyResolved = currentState.routeInfos[transition.resolveIndex].isResolved;
 
       // Swap the previously unresolved routeInfo with
@@ -97,24 +87,24 @@ export default class TransitionState<T extends Route> {
 
       // Proceed after ensuring that the redirect hook
       // didn't abort this transition by transitioning elsewhere.
-      return innerShouldContinue().then(
-        resolveOneRouteInfo,
-        null,
-        currentState.promiseLabel('Resolve route')
-      );
+      if (transition.isAborted) {
+        throw new Error('Transition aborted');
+      }
+
+      return resolveOneRouteInfo();
     }
 
-    function resolveOneRouteInfo(): TransitionState<T> | Promise<any> {
+    function resolveOneRouteInfo(): void | Promise<void> {
       if (transition.resolveIndex === currentState.routeInfos.length) {
         // This is is the only possible
         // fulfill value of TransitionState#resolve
-        return currentState;
+        return;
       }
 
       let routeInfo = currentState.routeInfos[transition.resolveIndex];
 
       return routeInfo
-        .resolve(innerShouldContinue, transition)
+        .resolve(transition)
         .then(proceed, null, currentState.promiseLabel('Proceed'));
     }
   }

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] = {};

tests/route_info_test.ts

@@ -9,24 +9,21 @@ import RouteInfo, {
 } from 'router/route-info';
 import InternalTransition from 'router/transition';
 import URLTransitionIntent from 'router/transition-intent/url-transition-intent';
-import { reject, resolve } from 'rsvp';
+import { resolve } from 'rsvp';
 import { createHandler, createHandlerInfo, module, test, TestRouter } from './test_helpers';
 
-function noop() {
-  return resolve(true);
-}
-
 module('RouteInfo');
 
 test('ResolvedRouteInfo resolve to themselves', function (assert) {
   let router = new TestRouter();
   let routeInfo = new ResolvedRouteInfo(router, 'foo', [], {}, createHandler('empty'));
   let intent = new URLTransitionIntent(router, 'foo');
-  routeInfo
-    .resolve(() => false, new InternalTransition(router, intent, undefined))
-    .then(function (resolvedRouteInfo) {
-      assert.equal(routeInfo, resolvedRouteInfo);
-    });
+
+  let transition = new InternalTransition(router, intent, undefined);
+
+  routeInfo.resolve(transition).then(function (resolvedRouteInfo) {
+    assert.equal(routeInfo, resolvedRouteInfo);
+  });
 });
 
 test('UnresolvedRouteInfoByParam defaults params to {}', function (assert) {
@@ -39,16 +36,14 @@ test('UnresolvedRouteInfoByParam defaults params to {}', function (assert) {
 });
 
 test('RouteInfo can be aborted mid-resolve', function (assert) {
-  assert.expect(2);
+  assert.expect(1);
 
   let routeInfo = createHandlerInfo('stub');
 
-  function abortResolve() {
-    assert.ok(true, 'abort was called');
-    return reject('LOL');
-  }
+  let transition = {} as Transition;
+  transition.isAborted = true;
 
-  routeInfo.resolve(abortResolve, {} as Transition).catch(function (error: Error) {
+  routeInfo.resolve(transition).catch(function (error: Error) {
     assert.equal(error, 'LOL');
   });
 });
@@ -57,17 +52,15 @@ test('RouteInfo#resolve resolves with a ResolvedRouteInfo', function (assert) {
   assert.expect(1);
 
   let routeInfo = createHandlerInfo('stub');
-  routeInfo
-    .resolve(() => false, {} as Transition)
-    .then(function (resolvedRouteInfo: RouteInfo<Route>) {
-      assert.ok(resolvedRouteInfo instanceof ResolvedRouteInfo);
-    });
+  routeInfo.resolve({} as Transition).then(function (resolvedRouteInfo: RouteInfo<Route>) {
+    assert.ok(resolvedRouteInfo instanceof ResolvedRouteInfo);
+  });
 });
 
 test('RouteInfo#resolve runs beforeModel hook on handler', function (assert) {
   assert.expect(1);
 
-  let transition = {};
+  let transition = {} as Transition;
 
   let routeInfo = createHandlerInfo('stub', {
     route: createHandler('stub', {
@@ -81,27 +74,27 @@ test('RouteInfo#resolve runs beforeModel hook on handler', function (assert) {
     }),
   });
 
-  routeInfo.resolve(noop, transition as Transition);
+  routeInfo.resolve(transition);
 });
 
 test('RouteInfo#resolve runs getModel hook', function (assert) {
   assert.expect(1);
 
-  let transition = {};
+  let transition = {} as Transition;
 
   let routeInfo = createHandlerInfo('stub', {
     getModel(payload: Dict<unknown>) {
       assert.equal(payload, transition);
     },
   });
 
-  routeInfo.resolve(noop, transition as Transition);
+  routeInfo.resolve(transition);
 });
 
 test('RouteInfo#resolve runs afterModel hook on handler', function (assert) {
   assert.expect(3);
 
-  let transition = {};
+  let transition = {} as Transition;
   let model = {};
 
   let routeInfo = createHandlerInfo('foo', {
@@ -117,18 +110,16 @@ test('RouteInfo#resolve runs afterModel hook on handler', function (assert) {
     },
   });
 
-  routeInfo
-    .resolve(noop, transition as Transition)
-    .then(function (resolvedRouteInfo: RouteInfo<Route>) {
-      assert.equal(resolvedRouteInfo.context, model, 'RouteInfo resolved with correct model');
-    });
+  routeInfo.resolve(transition).then(function (resolvedRouteInfo: RouteInfo<Route>) {
+    assert.equal(resolvedRouteInfo.context, model, 'RouteInfo resolved with correct model');
+  });
 });
 
 test('UnresolvedRouteInfoByParam gets its model hook called', function (assert) {
   assert.expect(2);
   let router = new TestRouter();
 
-  let transition = {};
+  let transition = {} as Transition;
 
   let routeInfo = new UnresolvedRouteInfoByParam(
     router,
@@ -146,7 +137,7 @@ test('UnresolvedRouteInfoByParam gets its model hook called', function (assert)
     })
   );
 
-  routeInfo.resolve(noop, transition as Transition);
+  routeInfo.resolve(transition);
 });
 
 test('UnresolvedRouteInfoByObject does NOT get its model hook called', function (assert) {
@@ -173,7 +164,7 @@ test('UnresolvedRouteInfoByObject does NOT get its model hook called', function
     resolve({ name: 'dorkletons' })
   );
 
-  routeInfo.resolve(noop, {} as Transition).then(function (resolvedRouteInfo: RouteInfo<Route>) {
+  routeInfo.resolve({} as Transition).then(function (resolvedRouteInfo: RouteInfo<Route>) {
     // @ts-ignore
     assert.equal(resolvedRouteInfo.context!.name, 'dorkletons');
   });