Skip to content

Commit

Permalink
fix(TargetState): make isDef check more thorough
Browse files Browse the repository at this point in the history
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
  • Loading branch information
xReeQz authored and christopherthielen committed Apr 19, 2020
1 parent f986698 commit e657cfe
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/state/targetState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { StateDeclaration, StateOrName, TargetStateDef } from './interface';
import { TransitionOptions } from '../transition/interface';
import { StateObject } from './stateObject';
import { isString } from '../common/predicates';
import { isObject, isString } from '../common/predicates';
import { stringify } from '../common/strings';
import { extend } from '../common';
import { StateRegistry } from './stateRegistry';
Expand Down Expand Up @@ -44,7 +44,9 @@ export class TargetState {
private _options: TransitionOptions;

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

/**
* The TargetState constructor
Expand Down
12 changes: 12 additions & 0 deletions test/targetStateSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ describe('TargetState object', function() {
expect(ref.error()).toBe("No such state 'notfound'");
});

describe('.isDef', function() {
it('should return true for TargetStateDef objects', () => {
expect(TargetState.isDef({ state: 'foo' })).toBeTrue();
expect(TargetState.isDef({ state: { name: 'foo' } })).toBeTrue();
});

it('should return false for TargetState instances', () => {
const ref = new TargetState(registry, 'foo');
expect(TargetState.isDef(ref)).toBeFalse();
});
});

describe('.withState', function() {
it('should replace the target state', () => {
const ref = new TargetState(registry, 'foo');
Expand Down

0 comments on commit e657cfe

Please sign in to comment.