From e657cfef2a36be136456710974f8b3078d57e697 Mon Sep 17 00:00:00 2001 From: Vasiliy Yaloza Date: Sun, 19 Apr 2020 16:19:28 +0300 Subject: [PATCH] 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 --- src/state/targetState.ts | 6 ++++-- test/targetStateSpec.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/state/targetState.ts b/src/state/targetState.ts index d791105e..0a9ef524 100644 --- a/src/state/targetState.ts +++ b/src/state/targetState.ts @@ -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'; @@ -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 diff --git a/test/targetStateSpec.ts b/test/targetStateSpec.ts index 9c047d5a..d8460340 100644 --- a/test/targetStateSpec.ts +++ b/test/targetStateSpec.ts @@ -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');