diff --git a/lib/router/route-info.ts b/lib/router/route-info.ts index 75181b01..84efa2b6 100644 --- a/lib/router/route-info.ts +++ b/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>; diff --git a/lib/router/transition-state.ts b/lib/router/transition-state.ts index 46bd2085..122ab780 100644 --- a/lib/router/transition-state.ts +++ b/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')); } } diff --git a/lib/router/transition.ts b/lib/router/transition.ts index 351d8717..f0f30438 100644 --- a/lib/router/transition.ts +++ b/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] = {}; diff --git a/tests/transition_state_test.ts b/tests/transition_state_test.ts index 9aee1275..a1a93fe3 100644 --- a/tests/transition_state_test.ts +++ b/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++) {