Make continuation tied to Transition#isAborted explicitly.

Author: Robert Jackson

lib/router/route-info.ts

@@ -40,8 +40,6 @@ export interface Route {
   buildRouteInfoMetadata?(): unknown;
 }
 
-export type Continuation = () => 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,8 +370,10 @@ export default class InternalRouteInfo<T extends Route> {
     });
   }
 
-  private checkForAbort<T>(shouldContinue: Continuation, value: T) {
-    shouldContinue();
+  private checkForAbort<U>(transition: InternalTransition<T>, value: U) {
+    if (transition.isAborted) {
+      throw new Error('Transition aborted');
+    }
 
     return value;
   }
@@ -427,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, ResolvedRouteInfo } 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,7 +37,6 @@ export default class TransitionState<T extends Route> {
     transition.resolveIndex = 0;
 
     let currentState = this;
-    let wasAborted = false;
 
     // The prelude RSVP.resolve() asyncs us into the promise land.
     return Promise.resolve(null, this.promiseLabel('Start transition'))
@@ -47,18 +46,6 @@ export default class TransitionState<T extends Route> {
         return currentState;
       });
 
-    function innerShouldContinue() {
-      if (shouldContinue()) {
-        return true;
-      } else {
-        // 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;
-        throw new Error('Transition aborted');
-      }
-    }
-
     function handleError(error: Error): never {
       // This is the only possible
       // reject value of TransitionState#resolve
@@ -68,6 +55,8 @@ export default class TransitionState<T extends Route> {
           ? routeInfos.length - 1
           : transition.resolveIndex;
 
+      let wasAborted = transition.isAborted;
+
       throw new TransitionError(
         error,
         currentState.routeInfos[errorHandlerIndex].route!,
@@ -98,9 +87,11 @@ export default class TransitionState<T extends Route> {
 
       // Proceed after ensuring that the redirect hook
       // didn't abort this transition by transitioning elsewhere.
-      if (innerShouldContinue()) {
-        return resolveOneRouteInfo();
+      if (transition.isAborted) {
+        throw new Error('Transition aborted');
       }
+
+      return resolveOneRouteInfo();
     }
 
     function resolveOneRouteInfo(): void | Promise<void> {
@@ -113,7 +104,7 @@ export default class TransitionState<T extends Route> {
       let routeInfo = currentState.routeInfos[transition.resolveIndex];
 
       return routeInfo
-        .resolve(innerShouldContinue, transition)
+        .resolve(transition)
         .then(proceed, null, currentState.promiseLabel('Proceed'));
     }
   }

lib/router/transition.ts

@@ -169,11 +169,11 @@ export default class Transition<T extends Route> implements Partial<Promise<T>>
       }
 
       this.sequence = router.currentSequence++;
-      this.promise = state
-        .resolve(() => !this.isAborted, 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

@@ -18,11 +18,12 @@ 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) {
@@ -35,17 +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(): boolean {
-    assert.ok(true, 'abort was called');
-
-    throw new Error('foo');
-  }
+  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');
   });
 });
@@ -54,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', {
@@ -78,27 +74,27 @@ test('RouteInfo#resolve runs beforeModel hook on handler', function (assert) {
     }),
   });
 
-  routeInfo.resolve(() => true, 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(() => true, 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', {
@@ -114,18 +110,16 @@ test('RouteInfo#resolve runs afterModel hook on handler', function (assert) {
     },
   });
 
-  routeInfo
-    .resolve(() => true, 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,
@@ -143,7 +137,7 @@ test('UnresolvedRouteInfoByParam gets its model hook called', function (assert)
     })
   );
 
-  routeInfo.resolve(() => true, transition as Transition);
+  routeInfo.resolve(transition);
 });
 
 test('UnresolvedRouteInfoByObject does NOT get its model hook called', function (assert) {
@@ -163,12 +157,10 @@ test('UnresolvedRouteInfoByObject does NOT get its model hook called', function
     resolve({ name: 'dorkletons' })
   );
 
-  routeInfo
-    .resolve(() => true, {} as Transition)
-    .then(function (resolvedRouteInfo: RouteInfo<Route>) {
-      // @ts-ignore
-      assert.equal(resolvedRouteInfo.context!.name, 'dorkletons');
-    });
+  routeInfo.resolve({} as Transition).then(function (resolvedRouteInfo: RouteInfo<Route>) {
+    // @ts-ignore
+    assert.equal(resolvedRouteInfo.context!.name, 'dorkletons');
+  });
 });
 
 test('RouteInfo.find', function (assert) {

tests/transition_state_test.ts

@@ -1,11 +1,6 @@
 import { Transition } from 'router';
 import { Dict } from 'router/core';
-import {
-  Continuation,
-  Route,
-  UnresolvedRouteInfoByObject,
-  UnresolvedRouteInfoByParam,
-} from 'router/route-info';
+import { Route, UnresolvedRouteInfoByObject, UnresolvedRouteInfoByParam } from 'router/route-info';
 import TransitionState, { TransitionError } from 'router/transition-state';
 import { Promise, resolve } from 'rsvp';
 import {
@@ -25,7 +20,7 @@ test('it starts off with default state', function (assert) {
 });
 
 test("#resolve delegates to handleInfo objects' resolve()", function (assert) {
-  assert.expect(7);
+  assert.expect(3);
 
   let state = new TransitionState();
 
@@ -35,29 +30,22 @@ test("#resolve delegates to handleInfo objects' resolve()", function (assert) {
 
   state.routeInfos = [
     createHandlerInfo('one', {
-      resolve: function (shouldContinue: Continuation) {
+      resolve: function () {
         ++counter;
         assert.equal(counter, 1);
-        shouldContinue();
         return resolve(resolvedHandlerInfos[0]);
       },
     }),
     createHandlerInfo('two', {
-      resolve: function (shouldContinue: Continuation) {
+      resolve: function () {
         ++counter;
         assert.equal(counter, 2);
-        shouldContinue();
         return resolve(resolvedHandlerInfos[1]);
       },
     }),
   ];
 
-  function keepGoing() {
-    assert.ok(true, 'continuation function was called');
-    return true;
-  }
-
-  state.resolve(keepGoing, {} as Transition).then(function (result: TransitionState<Route>) {
+  state.resolve({} as Transition).then(function (result: TransitionState<Route>) {
     assert.deepEqual(result.routeInfos, resolvedHandlerInfos);
   });
 });
@@ -69,9 +57,7 @@ test('State resolution can be halted', function (assert) {
 
   state.routeInfos = [
     createHandlerInfo('one', {
-      resolve: function (shouldContinue: Continuation) {
-        return shouldContinue();
-      },
+      resolve: function () {},
     }),
     createHandlerInfo('two', {
       resolve: function () {
@@ -80,11 +66,10 @@ test('State resolution can be halted', function (assert) {
     }),
   ];
 
-  function keepGoing() {
-    return false;
-  }
+  let fakeTransition = {} as Transition;
+  fakeTransition.isAborted = true;
 
-  state.resolve(keepGoing, {} as Transition).catch(function (reason: TransitionError) {
+  state.resolve(fakeTransition).catch(function (reason: TransitionError) {
     assert.ok(reason.wasAborted, 'state resolution was correctly marked as aborted');
   });
 
@@ -118,7 +103,7 @@ test('Integration w/ HandlerInfos', function (assert) {
   ];
 
   state
-    .resolve(() => true, transition as Transition)
+    .resolve(transition as Transition)
     .then(function (result: TransitionState<Route>) {
       let models = [];
       for (let i = 0; i < result.routeInfos.length; i++) {