Skip to content

Commit 30b82aa

Browse files
christopherthielenchristopherthielen
committed
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
1 parent 07c0ae4 commit 30b82aa

File tree

6 files changed

+119
-99
lines changed

6 files changed

+119
-99
lines changed

‎src/hooks/ignoredTransition.ts

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
2+
import { trace } from '../common/trace';
3+
import { Rejection } from '../transition/rejectFactory';
4+
import { TransitionService } from '../transition/transitionService';
5+
import { Transition } from '../transition/transition';
6+
7+
/**
8+
* A [[TransitionHookFn]] that skips a transition if it should be ignored
9+
*
10+
* This hook is invoked at the end of the onBefore phase.
11+
*
12+
* If the transition should be ignored (because no parameter or states changed)
13+
* then the transition is ignored and not processed.
14+
*/
15+
function ignoredHook(trans: Transition) {
16+
if (trans.ignored()) {
17+
trace.traceTransitionIgnored(this);
18+
return Rejection.ignored().toPromise();
19+
}
20+
}
21+
22+
export const registerIgnoredTransitionHook = (transitionService: TransitionService) =>
23+
transitionService.onBefore({}, ignoredHook, { priority: -9999 });

‎src/hooks/invalidTransition.ts

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { TransitionService } from '../transition/transitionService';
2+
import { Transition } from '../transition/transition';
3+
4+
/**
5+
* A [[TransitionHookFn]] that rejects the Transition if it is invalid
6+
*
7+
* This hook is invoked at the end of the onBefore phase.
8+
* If the transition is invalid (for example, param values do not validate)
9+
* then the transition is rejected.
10+
*/
11+
function invalidTransitionHook(trans: Transition) {
12+
if (!trans.valid()) {
13+
throw new Error(trans.error());
14+
}
15+
}
16+
17+
export const registerInvalidTransitionHook = (transitionService: TransitionService) =>
18+
transitionService.onBefore({}, invalidTransitionHook, { priority: -10000 });

‎src/transition/interface.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -816,5 +816,5 @@ export interface PathType {
816816
*/
817817
export type HookMatchCriterion = (string|IStateMatch|boolean)
818818

819-
export enum TransitionHookPhase { CREATE, BEFORE, ASYNC, SUCCESS, ERROR }
819+
export enum TransitionHookPhase { CREATE, BEFORE, RUN, SUCCESS, ERROR }
820820
export enum TransitionHookScope { TRANSITION, STATE }

‎src/transition/transition.ts

+36-64
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,35 @@
11
/**
22
* @coreapi
33
* @module transition
4-
*/ /** for typedoc */
5-
import {stringify} from "../common/strings";
6-
import {trace} from "../common/trace";
7-
import {services} from "../common/coreservices";
4+
*/
5+
/** for typedoc */
6+
import { trace } from '../common/trace';
7+
import { services } from '../common/coreservices';
88
import {
9-
map, find, extend, mergeR, tail,
10-
omit, toJson, arrayTuples, unnestR, identity, anyTrueR
11-
} from "../common/common";
12-
import { isObject, isArray } from "../common/predicates";
13-
import { prop, propEq, val, not, is } from "../common/hof";
14-
15-
import {StateDeclaration, StateOrName} from "../state/interface";
9+
map, find, extend, mergeR, tail, omit, toJson, arrayTuples, unnestR, identity, anyTrueR
10+
} from '../common/common';
11+
import { isObject } from '../common/predicates';
12+
import { prop, propEq, val, not, is } from '../common/hof';
13+
import { StateDeclaration, StateOrName } from '../state/interface';
1614
import {
17-
TransitionOptions, TreeChanges, IHookRegistry, TransitionHookPhase,
18-
RegisteredHooks, HookRegOptions, HookMatchCriteria
19-
} from "./interface";
20-
21-
import { TransitionStateHookFn, TransitionHookFn } from "./interface"; // has or is using
22-
23-
import {TransitionHook} from "./transitionHook";
24-
import {matchState, makeEvent, RegisteredHook} from "./hookRegistry";
25-
import {HookBuilder} from "./hookBuilder";
26-
import {PathNode} from "../path/node";
27-
import {PathFactory} from "../path/pathFactory";
28-
import {State} from "../state/stateObject";
29-
import {TargetState} from "../state/targetState";
30-
import {Param} from "../params/param";
31-
import {Resolvable} from "../resolve/resolvable";
32-
import {ViewConfig} from "../view/interface";
33-
import {Rejection} from "./rejectFactory";
34-
import {ResolveContext} from "../resolve/resolveContext";
35-
import {UIRouter} from "../router";
36-
import {UIInjector} from "../interface";
37-
import {RawParams} from "../params/interface";
38-
import { ResolvableLiteral } from "../resolve/interface";
15+
TransitionOptions, TreeChanges, IHookRegistry, TransitionHookPhase, RegisteredHooks, HookRegOptions,
16+
HookMatchCriteria, TransitionStateHookFn, TransitionHookFn
17+
} from './interface'; // has or is using
18+
import { TransitionHook } from './transitionHook';
19+
import { matchState, makeEvent, RegisteredHook } from './hookRegistry';
20+
import { HookBuilder } from './hookBuilder';
21+
import { PathNode } from '../path/node';
22+
import { PathFactory } from '../path/pathFactory';
23+
import { State } from '../state/stateObject';
24+
import { TargetState } from '../state/targetState';
25+
import { Param } from '../params/param';
26+
import { Resolvable } from '../resolve/resolvable';
27+
import { ViewConfig } from '../view/interface';
28+
import { ResolveContext } from '../resolve/resolveContext';
29+
import { UIRouter } from '../router';
30+
import { UIInjector } from '../interface';
31+
import { RawParams } from '../params/interface';
32+
import { ResolvableLiteral } from '../resolve/interface';
3933

4034
/** @hidden */
4135
const stateSelf: (_state: State) => StateDeclaration = prop("self");
@@ -630,28 +624,6 @@ export class Transition implements IHookRegistry {
630624
let globals = this.router.globals;
631625
globals.transitionHistory.enqueue(this);
632626

633-
let onBeforeHooks = hookBuilder.buildHooksForPhase(TransitionHookPhase.BEFORE);
634-
let syncResult = TransitionHook.runOnBeforeHooks(onBeforeHooks);
635-
636-
if (Rejection.isTransitionRejectionPromise(syncResult)) {
637-
syncResult.catch(() => 0); // issue #2676
638-
let rejectReason = (<any> syncResult)._transitionRejection;
639-
this._deferred.reject(rejectReason);
640-
return this.promise;
641-
}
642-
643-
if (!this.valid()) {
644-
let error = new Error(this.error());
645-
this._deferred.reject(error);
646-
return this.promise;
647-
}
648-
649-
if (this.ignored()) {
650-
trace.traceTransitionIgnored(this);
651-
this._deferred.reject(Rejection.ignored());
652-
return this.promise;
653-
}
654-
655627
// When the chain is complete, then resolve or reject the deferred
656628
const transitionSuccess = () => {
657629
trace.traceSuccess(this.$to(), this);
@@ -670,16 +642,16 @@ export class Transition implements IHookRegistry {
670642
runAllHooks(onErrorHooks);
671643
};
672644

673-
trace.traceTransitionStart(this);
674-
675-
// Chain the next hook off the previous
676-
const appendHookToChain = (prev: Promise<any>, nextHook: TransitionHook) =>
677-
prev.then(() => nextHook.invokeHook());
678-
679-
// Run the hooks, then resolve or reject the overall deferred in the .then() handler
680-
let asyncHooks = hookBuilder.buildHooksForPhase(TransitionHookPhase.ASYNC);
645+
// Builds a chain of transition hooks for the given phase
646+
// Each hook is invoked after the previous one completes
647+
const chainFor = (phase: TransitionHookPhase) =>
648+
TransitionHook.chain(hookBuilder.buildHooksForPhase(phase));
681649

682-
asyncHooks.reduce(appendHookToChain, syncResult)
650+
services.$q.when()
651+
.then(() => chainFor(TransitionHookPhase.BEFORE))
652+
// This waits to build the RUN hook chain until after the "BEFORE" hooks complete
653+
// This allows a BEFORE hook to dynamically add RUN hooks via the Transition object.
654+
.then(() => chainFor(TransitionHookPhase.RUN))
683655
.then(transitionSuccess, transitionError);
684656

685657
return this.promise;

‎src/transition/transitionHook.ts

+26-24
Original file line numberDiff line numberDiff line change
@@ -143,34 +143,36 @@ export class TransitionHook {
143143
}
144144

145145
/**
146-
* Run all TransitionHooks, ignoring their return value.
146+
* Chains together an array of TransitionHooks.
147+
*
148+
* Given a list of [[TransitionHook]] objects, chains them together.
149+
* Each hook is invoked after the previous one completes.
150+
*
151+
* #### Example:
152+
* ```js
153+
* var hooks: TransitionHook[] = getHooks();
154+
* let promise: Promise<any> = TransitionHook.chain(hooks);
155+
*
156+
* promise.then(handleSuccess, handleError);
157+
* ```
158+
*
159+
* @param hooks the list of hooks to chain together
160+
* @param waitFor if provided, the chain is `.then()`'ed off this promise
161+
* @returns a `Promise` for sequentially invoking the hooks (in order)
147162
*/
148-
static runAllHooks(hooks: TransitionHook[]): void {
149-
hooks.forEach(hook => hook.invokeHook());
163+
static chain(hooks: TransitionHook[], waitFor?: Promise<any>): Promise<any> {
164+
// Chain the next hook off the previous
165+
const createHookChainR = (prev: Promise<any>, nextHook: TransitionHook) =>
166+
prev.then(() => nextHook.invokeHook());
167+
return hooks.reduce(createHookChainR, waitFor || services.$q.when());
150168
}
151169

170+
152171
/**
153-
* Given an array of TransitionHooks, runs each one synchronously and sequentially.
154-
* Should any hook return a Rejection synchronously, the remaining hooks will not run.
155-
*
156-
* Returns a promise chain composed of any promises returned from each hook.invokeStep() call
172+
* Run all TransitionHooks, ignoring their return value.
157173
*/
158-
static runOnBeforeHooks(hooks: TransitionHook[]): Promise<any> {
159-
let results: Promise<HookResult>[] = [];
160-
161-
for (let hook of hooks) {
162-
let hookResult = hook.invokeHook();
163-
164-
if (Rejection.isTransitionRejectionPromise(hookResult)) {
165-
// Break on first thrown error or false/TargetState
166-
return hookResult;
167-
}
168-
169-
results.push(hookResult);
170-
}
171-
172-
return results
173-
.filter(isPromise)
174-
.reduce((chain: Promise<any>, promise: Promise<any>) => chain.then(val(promise)), services.$q.when());
174+
static runAllHooks(hooks: TransitionHook[]): void {
175+
hooks.forEach(hook => hook.invokeHook());
175176
}
177+
176178
}

‎src/transition/transitionService.ts

+15-10
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import { isDefined } from "../common/predicates";
2727
import { removeFrom, values, createProxyFunctions } from "../common/common";
2828
import { Disposable } from "../interface"; // has or is using
2929
import { val } from "../common/hof";
30+
import { registerIgnoredTransitionHook } from '../hooks/ignoredTransition';
31+
import { registerInvalidTransitionHook } from '../hooks/invalidTransition';
3032

3133
/**
3234
* The default [[Transition]] options.
@@ -170,6 +172,8 @@ export class TransitionService implements IHookRegistry, Disposable {
170172
*/
171173
_deregisterHookFns: {
172174
addCoreResolves: Function;
175+
ignored: Function;
176+
invalid: Function;
173177
redirectTo: Function;
174178
onExit: Function;
175179
onRetain: Function;
@@ -196,9 +200,8 @@ export class TransitionService implements IHookRegistry, Disposable {
196200
'getHooks',
197201
]);
198202

199-
this._defineDefaultPaths();
200-
this._defineDefaultEvents();
201-
203+
this._defineCorePaths();
204+
this._defineCoreEvents();
202205
this._registerCoreTransitionHooks();
203206
}
204207

@@ -228,7 +231,7 @@ export class TransitionService implements IHookRegistry, Disposable {
228231
}
229232

230233
/** @hidden */
231-
private _defineDefaultEvents() {
234+
private _defineCoreEvents() {
232235
const Phase = TransitionHookPhase;
233236
const TH = TransitionHook;
234237
const paths = this._criteriaPaths;
@@ -237,18 +240,18 @@ export class TransitionService implements IHookRegistry, Disposable {
237240

238241
this._defineEvent("onBefore", Phase.BEFORE, 0, paths.to, false, TH.HANDLE_RESULT);
239242

240-
this._defineEvent("onStart", Phase.ASYNC, 0, paths.to);
241-
this._defineEvent("onExit", Phase.ASYNC, 100, paths.exiting, true);
242-
this._defineEvent("onRetain", Phase.ASYNC, 200, paths.retained);
243-
this._defineEvent("onEnter", Phase.ASYNC, 300, paths.entering);
244-
this._defineEvent("onFinish", Phase.ASYNC, 400, paths.to);
243+
this._defineEvent("onStart", Phase.RUN, 0, paths.to);
244+
this._defineEvent("onExit", Phase.RUN, 100, paths.exiting, true);
245+
this._defineEvent("onRetain", Phase.RUN, 200, paths.retained);
246+
this._defineEvent("onEnter", Phase.RUN, 300, paths.entering);
247+
this._defineEvent("onFinish", Phase.RUN, 400, paths.to);
245248

246249
this._defineEvent("onSuccess", Phase.SUCCESS, 0, paths.to, false, TH.IGNORE_RESULT, TH.LOG_ERROR, false);
247250
this._defineEvent("onError", Phase.ERROR, 0, paths.to, false, TH.IGNORE_RESULT, TH.LOG_ERROR, false);
248251
}
249252

250253
/** @hidden */
251-
private _defineDefaultPaths() {
254+
private _defineCorePaths() {
252255
const { STATE, TRANSITION } = TransitionHookScope;
253256

254257
this._definePathType("to", TRANSITION);
@@ -318,6 +321,8 @@ export class TransitionService implements IHookRegistry, Disposable {
318321
let fns = this._deregisterHookFns;
319322

320323
fns.addCoreResolves = registerAddCoreResolvables(this);
324+
fns.ignored = registerIgnoredTransitionHook(this);
325+
fns.invalid = registerInvalidTransitionHook(this);
321326

322327
// Wire up redirectTo hook
323328
fns.redirectTo = registerRedirectToHook(this);

0 commit comments

Comments
 (0)