fix(forms): handle numeric values properly in the validator (#36157)

Previously, the behavior of the `minLength` and `maxLength` validators
caused confusion, as they appeared to work with numeric values but
did not in fact produce consistent results. This commit fixes the issue
by skipping validation altogether when a numeric value is used.

BREAKING CHANGES:

* The `minLength` and `maxLength` validators now verify that a value has
numeric `length` property and invoke validation only if that's the case.
Previously, falsey values without the length property (such as `0` or
`false` values) were triggering validation errors. If your code relies on
the old behavior, you can include other validators such as [min][1] or
[requiredTrue][2] to the list of validators for a particular field.

[1]: https://angular.io/api/forms/Validators#min
[2]: https://angular.io/api/forms/Validators#requiredTrue

Closes #35591

PR Close #36157
This commit is contained in:
Sonu Kapoor 2020-03-20 07:49:25 -04:00 committed by Alex Rickabaugh
parent 7bd9e09c78
commit 88a235de3a
2 changed files with 55 additions and 9 deletions

View File

@ -11,13 +11,18 @@ import {forkJoin, from, Observable} from 'rxjs';
import {map} from 'rxjs/operators';
import {AsyncValidatorFn, ValidationErrors, Validator, ValidatorFn} from './directives/validators';
import {AbstractControl, FormControl} from './model';
import {AbstractControl} from './model';
function isEmptyInputValue(value: any): boolean {
// we don't check for string here so it also works with arrays
return value == null || value.length === 0;
}
function hasValidLength(value: any): boolean {
// non-strict comparison is intentional, to check for both `null` and `undefined` values
return value != null && typeof value.length === 'number';
}
/**
* @description
* An `InjectionToken` for registering additional synchronous validators used with
@ -295,12 +300,14 @@ export class Validators {
*/
static minLength(minLength: number): ValidatorFn {
return (control: AbstractControl): ValidationErrors|null => {
if (isEmptyInputValue(control.value)) {
return null; // don't validate empty values to allow optional controls
if (isEmptyInputValue(control.value) || !hasValidLength(control.value)) {
// don't validate empty values to allow optional controls
// don't validate values without `length` property
return null;
}
const length: number = control.value ? control.value.length : 0;
return length < minLength ?
{'minlength': {'requiredLength': minLength, 'actualLength': length}} :
return control.value.length < minLength ?
{'minlength': {'requiredLength': minLength, 'actualLength': control.value.length}} :
null;
};
}
@ -334,9 +341,8 @@ export class Validators {
*/
static maxLength(maxLength: number): ValidatorFn {
return (control: AbstractControl): ValidationErrors|null => {
const length: number = control.value ? control.value.length : 0;
return length > maxLength ?
{'maxlength': {'requiredLength': maxLength, 'actualLength': length}} :
return hasValidLength(control.value) && control.value.length > maxLength ?
{'maxlength': {'requiredLength': maxLength, 'actualLength': control.value.length}} :
null;
};
}

View File

@ -225,6 +225,26 @@ describe('Validators', () => {
'minlength': {'requiredLength': 2, 'actualLength': 1}
});
});
it('should always return null with numeric values', () => {
expect(Validators.minLength(1)(new FormControl(0))).toBeNull();
expect(Validators.minLength(1)(new FormControl(1))).toBeNull();
expect(Validators.minLength(1)(new FormControl(-1))).toBeNull();
expect(Validators.minLength(1)(new FormControl(+1))).toBeNull();
});
it('should trigger validation for an object that contains numeric length property', () => {
const value = {length: 5, someValue: [1, 2, 3, 4, 5]};
expect(Validators.minLength(1)(new FormControl(value))).toBeNull();
expect(Validators.minLength(10)(new FormControl(value))).toEqual({
'minlength': {'requiredLength': 10, 'actualLength': 5}
});
});
it('should return null when passing a boolean', () => {
expect(Validators.minLength(1)(new FormControl(true))).toBeNull();
expect(Validators.minLength(1)(new FormControl(false))).toBeNull();
});
});
describe('maxLength', () => {
@ -261,6 +281,26 @@ describe('Validators', () => {
'maxlength': {'requiredLength': 1, 'actualLength': 2}
});
});
it('should always return null with numeric values', () => {
expect(Validators.maxLength(1)(new FormControl(0))).toBeNull();
expect(Validators.maxLength(1)(new FormControl(1))).toBeNull();
expect(Validators.maxLength(1)(new FormControl(-1))).toBeNull();
expect(Validators.maxLength(1)(new FormControl(+1))).toBeNull();
});
it('should trigger validation for an object that contains numeric length property', () => {
const value = {length: 5, someValue: [1, 2, 3, 4, 5]};
expect(Validators.maxLength(10)(new FormControl(value))).toBeNull();
expect(Validators.maxLength(1)(new FormControl(value))).toEqual({
'maxlength': {'requiredLength': 1, 'actualLength': 5}
});
});
it('should return null when passing a boolean', () => {
expect(Validators.maxLength(1)(new FormControl(true))).toBeNull();
expect(Validators.maxLength(1)(new FormControl(false))).toBeNull();
});
});
describe('pattern', () => {