Skip to content

Commit

Permalink
fix(forms): use strict runtimeType checks instead of instanceof
Browse files Browse the repository at this point in the history
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 angular#6981
  • Loading branch information
vsavkin authored and btford committed Feb 10, 2016
1 parent 8f47aa3 commit 50548fb
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
10 changes: 6 additions & 4 deletions modules/angular2/src/common/forms/directives/shared.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions modules/angular2/src/facade/lang.dart
Original file line number Diff line number Diff line change
Expand Up @@ -353,3 +353,7 @@ var global = null;
dynamic evalExpression(String sourceUrl, String expr, String declarations, Map<String, String> vars) {
throw "Dart does not support evaluating expression during runtime!";
}

bool hasConstructor(Object value, Type type) {
return value.runtimeType == type;
}
4 changes: 4 additions & 0 deletions modules/angular2/src/facade/lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
14 changes: 13 additions & 1 deletion modules/angular2/test/facade/lang_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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); });
});
});
}

0 comments on commit 50548fb

Please sign in to comment.