Skip to content

Commit e657cfe

Browse files
xReeQzchristopherthielen
authored andcommitted
fix(TargetState): make isDef check more thorough
Existing condition resulted in true even when an instance of TargetState was passed. This caused custom rules, returning an instance of TargetState from a UrlRuleHandlerFn, to perform a transition leading nowhere
1 parent f986698 commit e657cfe

File tree

2 files changed

+16
-2
lines changed

2 files changed

+16
-2
lines changed

src/state/targetState.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import { StateDeclaration, StateOrName, TargetStateDef } from './interface';
44
import { TransitionOptions } from '../transition/interface';
55
import { StateObject } from './stateObject';
6-
import { isString } from '../common/predicates';
6+
import { isObject, isString } from '../common/predicates';
77
import { stringify } from '../common/strings';
88
import { extend } from '../common';
99
import { StateRegistry } from './stateRegistry';
@@ -44,7 +44,9 @@ export class TargetState {
4444
private _options: TransitionOptions;
4545

4646
/** Returns true if the object has a state property that might be a state or state name */
47-
static isDef = (obj): obj is TargetStateDef => obj && obj.state && (isString(obj.state) || isString(obj.state.name));
47+
static isDef = (obj): obj is TargetStateDef => {
48+
return obj && obj.state && (isString(obj.state) || (isObject(obj.state) && isString(obj.state.name)));
49+
};
4850

4951
/**
5052
* The TargetState constructor

test/targetStateSpec.ts

+12
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ describe('TargetState object', function() {
2828
expect(ref.error()).toBe("No such state 'notfound'");
2929
});
3030

31+
describe('.isDef', function() {
32+
it('should return true for TargetStateDef objects', () => {
33+
expect(TargetState.isDef({ state: 'foo' })).toBeTrue();
34+
expect(TargetState.isDef({ state: { name: 'foo' } })).toBeTrue();
35+
});
36+
37+
it('should return false for TargetState instances', () => {
38+
const ref = new TargetState(registry, 'foo');
39+
expect(TargetState.isDef(ref)).toBeFalse();
40+
});
41+
});
42+
3143
describe('.withState', function() {
3244
it('should replace the target state', () => {
3345
const ref = new TargetState(registry, 'foo');

0 commit comments

Comments
 (0)