Skip to content

Commit

Permalink
fix(onBefore): Skip remaining hooks after the (#2)
Browse files Browse the repository at this point in the history
first hook that modifies the transition.
  • Loading branch information
csvn authored and christopherthielen committed Oct 21, 2016
1 parent 1c77551 commit 8a45d04
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 15 deletions.
5 changes: 3 additions & 2 deletions src/transition/transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ export class Transition implements IHookRegistry {
*/
run(): Promise<any> {
let runSynchronousHooks = TransitionHook.runSynchronousHooks;
let runAllHooks = TransitionHook.runAllHooks;
let hookBuilder = this.hookBuilder();
let globals = <Globals> this.router.globals;
globals.transitionHistory.enqueue(this);
Expand Down Expand Up @@ -503,15 +504,15 @@ export class Transition implements IHookRegistry {
trace.traceSuccess(this.$to(), this);
this.success = true;
this._deferred.resolve(this.to());
runSynchronousHooks(hookBuilder.getOnSuccessHooks(), true);
runAllHooks(hookBuilder.getOnSuccessHooks());
};

const transitionError = (reason: any) => {
trace.traceError(reason, this);
this.success = false;
this._deferred.reject(reason);
this._error = reason;
runSynchronousHooks(hookBuilder.getOnErrorHooks(), true);
runAllHooks(hookBuilder.getOnErrorHooks());
};

trace.traceTransitionStart(this);
Expand Down
37 changes: 25 additions & 12 deletions src/transition/transitionHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,31 +94,44 @@ export class TransitionHook {
return `${event} context: ${context}, ${maxLength(200, name)}`;
}

/**
* Run all TransitionHooks, ignoring their return value.
*/
static runAllHooks(hooks: TransitionHook[]): void {
hooks.forEach(hook => {
try {
hook.invokeHook();
} catch (exception) {
let errorHandler = hook.transition.router.stateService.defaultErrorHandler();
errorHandler(exception);
}
});
}

/**
* Given an array of TransitionHooks, runs each one synchronously and sequentially.
* Should any hook return a Rejection synchronously, the remaining hooks will not run.
*
* Returns a promise chain composed of any promises returned from each hook.invokeStep() call
*/
static runSynchronousHooks(hooks: TransitionHook[], swallowExceptions: boolean = false): Promise<any> {
static runSynchronousHooks(hooks: TransitionHook[]): Promise<any> {
let results: Promise<HookResult>[] = [];
for (let i = 0; i < hooks.length; i++) {
let hook = hooks[i];

for (let hook of hooks) {
try {
results.push(hook.invokeHook());
} catch (exception) {
if (!swallowExceptions) {
return Rejection.errored(exception).toPromise();
let hookResult = hook.invokeHook();

if (Rejection.isTransitionRejectionPromise(hookResult)) {
// Break on first thrown error or false/TargetState
return hookResult;
}

let errorHandler = hook.transition.router.stateService.defaultErrorHandler();
errorHandler(exception);
results.push(hookResult);
} catch (exception) {
return Rejection.errored(exception).toPromise();
}
}

let rejections = results.filter(Rejection.isTransitionRejectionPromise);
if (rejections.length) return rejections[0];

return results
.filter(isPromise)
.reduce((chain: Promise<any>, promise: Promise<any>) => chain.then(val(promise)), services.$q.when());
Expand Down
86 changes: 85 additions & 1 deletion test/transitionSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,61 @@ describe('transition', function () {
}), 20);
}));

describe('.onBefore()', function() {
beforeEach(() => $state.defaultErrorHandler(() => {}));

it('should stop running remaining hooks if hook modifies transition synchronously', ((done) => {
let counter = 0;
let increment = (amount) => {
return () => {
counter += amount;
return false;
};
};

$transitions.onBefore({}, increment(1), { priority: 2 });
$transitions.onBefore({}, increment(100), { priority: 1 });

Promise.resolve()
.then(goFail('first', 'second'))
.then(() => expect(counter).toBe(1))
.then(goFail('second', 'third'))
.then(() => expect(counter).toBe(2))
.then(done);
}));

it('should stop running remaining hooks when synchronous result throws or returns false|TargetState', ((done) => {
let current = null;

$transitions.onBefore({}, (t) => { current = t.to().name; });
$transitions.onBefore({ to: 'first' }, () => {
throw Error('first-error');
}, { priority: 1 });
$transitions.onBefore({ to: 'second' }, () => false, { priority: 3 });
$transitions.onBefore({ to: 'third' }, () => $state.target('A'), { priority: 2 });

Promise.resolve()
.then(goFail('A', 'first'))
.then((res) => {
expect(res.type).toBe(RejectType.ERROR);
expect(current).toBe(null);
})
.then(goFail('A', 'second'))
.then((res) => {
expect(res.type).toBe(RejectType.ABORTED);
expect(current).toBe(null);
})
.then(goFail('A', 'third'))
.then((res) => {
expect(res.type).toBe(RejectType.SUPERSEDED);
expect(current).toBe(null);
})
.then(go('A', 'B'))
.then(() => expect(current).toBe('B'))
.then(done);
}));
});

describe('.onStart()', function() {
it('should fire matching events when transition starts', ((done) => {
var t = null;
Expand Down Expand Up @@ -317,7 +372,9 @@ describe('transition', function () {
.then(done);
}));

it('should be called even if other .onSuccess() callbacks fail (throw errors, etc)', ((done) => {
it('should call all .onSuccess() even when callbacks fail (throw errors, etc)', ((done) => {
$transitions.onSuccess({ from: "*", to: "*" }, () => false);
$transitions.onSuccess({ from: "*", to: "*" }, () => $state.target('A'));
$transitions.onSuccess({ from: "*", to: "*" }, function() { throw new Error("oops!"); });
$transitions.onSuccess({ from: "*", to: "*" }, function(trans) { states.push(trans.to().name); });

Expand Down Expand Up @@ -364,6 +421,33 @@ describe('transition', function () {
.then(() => expect(hooks).toEqual([ 'splatsplat', 'AD' ]))
.then(done);
}));

it('should call all error handlers when transition fails.', ((done) => {
let count = 0;

$state.defaultErrorHandler(() => {});
$transitions.onStart({}, () => false);
$transitions.onError({}, () => {
count += 1;
return false;
});
$transitions.onError({}, () => {
count += 10;
$state.target('A');
});
$transitions.onError({}, function() {
count += 100;
throw new Error("oops!");
});
$transitions.onError({}, () => {
count += 1000;
});

Promise.resolve()
.then(goFail("B", "C"))
.then(() => expect(count).toBe(1111))
.then(done);
}));
});

// Test for #2866
Expand Down

0 comments on commit 8a45d04

Please sign in to comment.