From 50548fb5655bca742d1056ea91217a3b8460db08 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Tue, 9 Feb 2016 17:46:38 -0800 Subject: [PATCH] fix(forms): use strict runtimeType checks instead of instanceof Currently, validators extending built-in validators are treated as built-in. This can result in an error when both a real built-in validator and a custom one are applied to the same element. Closes #6981 --- .../angular2/src/common/forms/directives/shared.ts | 10 ++++++---- modules/angular2/src/facade/lang.dart | 4 ++++ modules/angular2/src/facade/lang.ts | 4 ++++ modules/angular2/test/facade/lang_spec.ts | 14 +++++++++++++- 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/modules/angular2/src/common/forms/directives/shared.ts b/modules/angular2/src/common/forms/directives/shared.ts index 9e9dac288b..02edbca183 100644 --- a/modules/angular2/src/common/forms/directives/shared.ts +++ b/modules/angular2/src/common/forms/directives/shared.ts @@ -1,5 +1,5 @@ import {ListWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; -import {isBlank, isPresent, looseIdentical} from 'angular2/src/facade/lang'; +import {isBlank, isPresent, looseIdentical, hasConstructor} from 'angular2/src/facade/lang'; import {BaseException, WrappedException} from 'angular2/src/facade/exceptions'; import {ControlContainer} from './control_container'; @@ -82,11 +82,13 @@ export function selectValueAccessor(dir: NgControl, var builtinAccessor; var customAccessor; valueAccessors.forEach(v => { - if (v instanceof DefaultValueAccessor) { + if (hasConstructor(v, DefaultValueAccessor)) { defaultAccessor = v; - } else if (v instanceof CheckboxControlValueAccessor || v instanceof NumberValueAccessor || - v instanceof SelectControlValueAccessor || v instanceof RadioControlValueAccessor) { + } else if (hasConstructor(v, CheckboxControlValueAccessor) || + hasConstructor(v, NumberValueAccessor) || + hasConstructor(v, SelectControlValueAccessor) || + hasConstructor(v, RadioControlValueAccessor)) { if (isPresent(builtinAccessor)) _throwError(dir, "More than one built-in value accessor matches"); builtinAccessor = v; diff --git a/modules/angular2/src/facade/lang.dart b/modules/angular2/src/facade/lang.dart index e09adbd2cd..13c8057997 100644 --- a/modules/angular2/src/facade/lang.dart +++ b/modules/angular2/src/facade/lang.dart @@ -353,3 +353,7 @@ var global = null; dynamic evalExpression(String sourceUrl, String expr, String declarations, Map vars) { throw "Dart does not support evaluating expression during runtime!"; } + +bool hasConstructor(Object value, Type type) { + return value.runtimeType == type; +} diff --git a/modules/angular2/src/facade/lang.ts b/modules/angular2/src/facade/lang.ts index 25a161e059..b1dd056830 100644 --- a/modules/angular2/src/facade/lang.ts +++ b/modules/angular2/src/facade/lang.ts @@ -464,3 +464,7 @@ export function evalExpression(sourceUrl: string, expr: string, declarations: st export function isPrimitive(obj: any): boolean { return !isJsObject(obj); } + +export function hasConstructor(value: Object, type: Type): boolean { + return value.constructor === type; +} \ No newline at end of file diff --git a/modules/angular2/test/facade/lang_spec.ts b/modules/angular2/test/facade/lang_spec.ts index 25a283adc9..352b1a75f7 100644 --- a/modules/angular2/test/facade/lang_spec.ts +++ b/modules/angular2/test/facade/lang_spec.ts @@ -4,9 +4,13 @@ import { RegExpWrapper, RegExpMatcherWrapper, StringWrapper, - CONST_EXPR + CONST_EXPR, + hasConstructor } from 'angular2/src/facade/lang'; +class MySuperclass {} +class MySubclass extends MySuperclass {} + export function main() { describe('RegExp', () => { it('should expose the index for each match', () => { @@ -119,5 +123,13 @@ export function main() { expect(StringWrapper.stripRight(null, "S")).toEqual(null); }); }); + + describe('hasConstructor', () => { + it("should be true when the type matches", + () => { expect(hasConstructor(new MySuperclass(), MySuperclass)).toEqual(true); }); + + it("should be false for subtypes", + () => { expect(hasConstructor(new MySubclass(), MySuperclass)).toEqual(false); }); + }); }); }