Skip to content

Commit 953e618

Browse files
feat(transition): Run hooks synchronously in current stack, when possible
1 parent 3141a8f commit 953e618

File tree

5 files changed

+78
-44
lines changed

5 files changed

+78
-44
lines changed

src/state/stateService.ts

+8-2
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,18 @@ export class StateService {
104104
if (!(result instanceof TargetState)) {
105105
return;
106106
}
107+
107108
let target = <TargetState> result;
108109
// Recreate the TargetState, in case the state is now defined.
109110
target = this.target(target.identifier(), target.params(), target.options());
110111

111-
if (!target.valid()) return Rejection.invalid(target.error()).toPromise();
112-
if (latestThing() !== latest) return Rejection.superseded().toPromise();
112+
if (!target.valid()) {
113+
return Rejection.invalid(target.error()).toPromise();
114+
}
115+
116+
if (latestThing() !== latest) {
117+
return Rejection.superseded().toPromise();
118+
}
113119

114120
return this.transitionTo(target.identifier(), target.params(), target.options());
115121
};

src/transition/transition.ts

+30-27
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
import { trace } from '../common/trace';
77
import { services } from '../common/coreservices';
88
import {
9-
map, find, extend, mergeR, tail, omit, toJson, arrayTuples, unnestR, identity, anyTrueR
9+
map, find, extend, mergeR, tail, omit, toJson, arrayTuples, unnestR, identity, anyTrueR,
1010
} from '../common/common';
11-
import { isObject, isUndefined } from '../common/predicates';
11+
import { isObject } from '../common/predicates';
1212
import { prop, propEq, val, not, is } from '../common/hof';
1313
import { StateDeclaration, StateOrName } from '../state/interface';
1414
import {
1515
TransitionOptions, TreeChanges, IHookRegistry, TransitionHookPhase, RegisteredHooks, HookRegOptions,
16-
HookMatchCriteria, TransitionStateHookFn, TransitionHookFn
16+
HookMatchCriteria, TransitionStateHookFn, TransitionHookFn,
1717
} from './interface'; // has or is using
1818
import { TransitionHook } from './transitionHook';
1919
import { matchState, makeEvent, RegisteredHook } from './hookRegistry';
@@ -94,21 +94,21 @@ export class Transition implements IHookRegistry {
9494

9595

9696
/** @hidden */
97-
onBefore(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return }
97+
onBefore(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return; }
9898
/** @inheritdoc */
99-
onStart(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return }
99+
onStart(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return; }
100100
/** @inheritdoc */
101-
onExit(criteria: HookMatchCriteria, callback: TransitionStateHookFn, options?: HookRegOptions): Function { return }
101+
onExit(criteria: HookMatchCriteria, callback: TransitionStateHookFn, options?: HookRegOptions): Function { return; }
102102
/** @inheritdoc */
103-
onRetain(criteria: HookMatchCriteria, callback: TransitionStateHookFn, options?: HookRegOptions): Function { return }
103+
onRetain(criteria: HookMatchCriteria, callback: TransitionStateHookFn, options?: HookRegOptions): Function { return; }
104104
/** @inheritdoc */
105-
onEnter(criteria: HookMatchCriteria, callback: TransitionStateHookFn, options?: HookRegOptions): Function { return }
105+
onEnter(criteria: HookMatchCriteria, callback: TransitionStateHookFn, options?: HookRegOptions): Function { return; }
106106
/** @inheritdoc */
107-
onFinish(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return }
107+
onFinish(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return; }
108108
/** @inheritdoc */
109-
onSuccess(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return }
109+
onSuccess(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return; }
110110
/** @inheritdoc */
111-
onError(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return }
111+
onError(criteria: HookMatchCriteria, callback: TransitionHookFn, options?: HookRegOptions): Function { return; }
112112

113113
/** @hidden
114114
* Creates the transition-level hook registration functions
@@ -511,7 +511,7 @@ export class Transition implements IHookRegistry {
511511
*/
512512
redirect(targetState: TargetState): Transition {
513513
let redirects = 1, trans: Transition = this;
514-
while((trans = trans.redirectedFrom()) != null) {
514+
while ((trans = trans.redirectedFrom()) != null) {
515515
if (++redirects > 20) throw new Error(`Too many consecutive Transition redirects (20+)`);
516516
}
517517

@@ -620,14 +620,9 @@ export class Transition implements IHookRegistry {
620620
let runAllHooks = TransitionHook.runAllHooks;
621621

622622
// Gets transition hooks array for the given phase
623-
const hooksFor = (phase: TransitionHookPhase) =>
623+
const getHooksFor = (phase: TransitionHookPhase) =>
624624
this._hookBuilder.buildHooksForPhase(phase);
625625

626-
// Builds a chain of transition hooks for the given phase
627-
// Each hook is invoked after the previous one completes
628-
const chainFor = (phase: TransitionHookPhase) =>
629-
TransitionHook.chain(hooksFor(phase));
630-
631626
const startTransition = () => {
632627
let globals = this.router.globals;
633628

@@ -636,31 +631,39 @@ export class Transition implements IHookRegistry {
636631
globals.transitionHistory.enqueue(this);
637632

638633
trace.traceTransitionStart(this);
634+
635+
return services.$q.when(undefined);
639636
};
640637

638+
641639
// When the chain is complete, then resolve or reject the deferred
642640
const transitionSuccess = () => {
643641
trace.traceSuccess(this.$to(), this);
644642
this.success = true;
645643
this._deferred.resolve(this.to());
646-
runAllHooks(hooksFor(TransitionHookPhase.SUCCESS));
644+
runAllHooks(getHooksFor(TransitionHookPhase.SUCCESS));
647645
};
648646

649647
const transitionError = (reason: any) => {
650648
trace.traceError(reason, this);
651649
this.success = false;
652650
this._deferred.reject(reason);
653651
this._error = reason;
654-
runAllHooks(hooksFor(TransitionHookPhase.ERROR));
652+
runAllHooks(getHooksFor(TransitionHookPhase.ERROR));
653+
};
654+
655+
// This waits to build the RUN hook chain until after the "BEFORE" hooks complete
656+
// This allows a BEFORE hook to dynamically add RUN hooks via the Transition object.
657+
const runTransition = () => {
658+
let allRunHooks = getHooksFor(TransitionHookPhase.RUN);
659+
let done = () => services.$q.when(undefined);
660+
TransitionHook.invokeHooks(allRunHooks, done)
661+
.then(transitionSuccess, transitionError);
655662
};
656663

657-
services.$q.when()
658-
.then(() => chainFor(TransitionHookPhase.BEFORE))
659-
.then(startTransition)
660-
// This waits to build the RUN hook chain until after the "BEFORE" hooks complete
661-
// This allows a BEFORE hook to dynamically add RUN hooks via the Transition object.
662-
.then(() => chainFor(TransitionHookPhase.RUN))
663-
.then(transitionSuccess, transitionError);
664+
let allBeforeHooks = getHooksFor(TransitionHookPhase.BEFORE);
665+
TransitionHook.invokeHooks(allBeforeHooks, startTransition)
666+
.then(runTransition);
664667

665668
return this.promise;
666669
}

src/transition/transitionHook.ts

+37-12
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { TargetState } from '../state/targetState';
1515
import { Transition } from './transition';
1616
import { TransitionEventType } from './transitionEventType';
1717
import { RegisteredHook } from './hookRegistry';
18-
import { StateDeclaration } from '../state/interface'; // has or is using
18+
import { StateDeclaration } from '../state/interface';
1919

2020
let defaultOptions: TransitionHookOptions = {
2121
current: noop,
@@ -79,7 +79,7 @@ export class TransitionHook {
7979
this.transition.router.stateService.defaultErrorHandler()(err);
8080
}
8181

82-
invokeHook(): Promise<HookResult> {
82+
invokeHook(): Promise<HookResult> | void {
8383
let hook = this.registeredHook;
8484
if (hook._deregistered) return;
8585

@@ -101,18 +101,19 @@ export class TransitionHook {
101101
const handleResult = result =>
102102
hook.eventType.getResultHandler(this)(result);
103103

104-
if (this.type.synchronous) {
105-
try {
106-
return handleResult(invokeCallback());
107-
} catch (err) {
108-
return handleError(Rejection.normalize(err));
104+
try {
105+
let result = invokeCallback();
106+
107+
if (isPromise(result)) {
108+
return result.catch(normalizeErr)
109+
.then(handleResult, handleError);
110+
} else {
111+
return handleResult(result);
109112
}
113+
} catch (err) {
114+
// If callback throws (synchronously)
115+
return handleError(Rejection.normalize(err));
110116
}
111-
112-
return services.$q.when()
113-
.then(invokeCallback)
114-
.catch(normalizeErr)
115-
.then(handleResult, handleError);
116117
}
117118

118119
/**
@@ -205,6 +206,30 @@ export class TransitionHook {
205206
}
206207

207208

209+
/**
210+
* Invokes all the provided TransitionHooks, in order.
211+
* Each hook's return value is checked.
212+
* If any hook returns a promise, then the rest of the hooks are chained off that promise, and the promise is returned.
213+
* If no hook returns a promise, then all hooks are processed synchronously.
214+
*
215+
* @param hooks the list of TransitionHooks to invoke
216+
* @param done a callback that is invoked after all the hooks have successfully completed
217+
*
218+
* @returns a promise for the async result, or the result of the callback
219+
*/
220+
static invokeHooks<T>(hooks: TransitionHook[], done: () => T): Promise<any> | T {
221+
for (let idx = 0; idx < hooks.length; idx++) {
222+
let hookResult = hooks[idx].invokeHook();
223+
224+
if (isPromise(hookResult)) {
225+
let remainingHooks = hooks.slice(idx + 1);
226+
return TransitionHook.chain(remainingHooks, hookResult).then(done);
227+
}
228+
}
229+
230+
return done();
231+
}
232+
208233
/**
209234
* Run all TransitionHooks, ignoring their return value.
210235
*/

test/stateServiceSpec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,8 @@ describe('stateService', function () {
542542
done();
543543
});
544544

545-
fit('aborts pending transitions when superseded from callbacks', async(done) => {
546-
// router.trace.enable();
545+
it('aborts pending transitions when superseded from callbacks', async(done) => {
546+
// router.trace.enable(1);
547547
$state.defaultErrorHandler(() => null);
548548
$registry.register({
549549
name: 'redir',

tslint.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"no-any": false,
1818
"no-arg": true,
1919
"no-bitwise": true,
20-
"no-conditional-assignment": true,
20+
"no-conditional-assignment": false,
2121
"no-console": [true, "debug", "info", "time", "timeEnd", "trace" ],
2222
"no-construct": true,
2323
"no-constructor-vars": false,

0 commit comments

Comments
 (0)