fix(forms): allow optional fields with pattern and minlength validators (#12147)

modules/@angular/forms/src/validators.ts

@@ -11,11 +11,13 @@ import {toPromise} from 'rxjs/operator/toPromise';
 
 import {AsyncValidatorFn, ValidatorFn} from './directives/validators';
 import {StringMapWrapper} from './facade/collection';
-import {isBlank, isPresent, isString} from './facade/lang';
+import {isPresent} from './facade/lang';
 import {AbstractControl} from './model';
 import {isPromise} from './private_import_core';
 
-
+function isEmptyInputValue(value: any) {
+  return value == null || typeof value === 'string' && value.length === 0;
+}
 
 /**
  * Providers for validators to be used for {@link FormControl}s in a form.
@@ -60,9 +62,7 @@ export class Validators {
    * Validator that requires controls to have a non-empty value.
    */
   static required(control: AbstractControl): {[key: string]: boolean} {
-    return isBlank(control.value) || (isString(control.value) && control.value == '') ?
+    return isEmptyInputValue(control.value) ? {'required': true} : null;
-        {'required': true} :
-        null;
   }
 
   /**
@@ -70,6 +70,9 @@ export class Validators {
    */
   static minLength(minLength: number): ValidatorFn {
     return (control: AbstractControl): {[key: string]: any} => {
+      if (isEmptyInputValue(control.value)) {
+        return null;  // don't validate empty values to allow optional controls
+      }
       const length = typeof control.value === 'string' ? control.value.length : 0;
       return length < minLength ?
           {'minlength': {'requiredLength': minLength, 'actualLength': length}} :
@@ -94,10 +97,14 @@ export class Validators {
    */
   static pattern(pattern: string): ValidatorFn {
     return (control: AbstractControl): {[key: string]: any} => {
-      let regex = new RegExp(`^${pattern}$`);
+      if (isEmptyInputValue(control.value)) {
-      let v: string = control.value;
+        return null;  // don't validate empty values to allow optional controls
-      return regex.test(v) ? null :
+      }
-                             {'pattern': {'requiredPattern': `^${pattern}$`, 'actualValue': v}};
+      const regex = new RegExp(`^${pattern}$`);
+      const value: string = control.value;
+      return regex.test(value) ?
+          null :
+          {'pattern': {'requiredPattern': `^${pattern}$`, 'actualValue': value}};
     };
   }
 

modules/@angular/forms/test/template_integration_spec.ts

@@ -22,7 +22,7 @@ export function main() {
           StandaloneNgModel, NgModelForm, NgModelGroupForm, NgModelValidBinding, NgModelNgIfForm,
           NgModelRadioForm, NgModelSelectForm, NgNoFormComp, InvalidNgModelNoName,
           NgModelOptionsStandalone, NgModelCustomComp, NgModelCustomWrapper,
-          NgModelValidationBindings
+          NgModelValidationBindings, NgModelMultipleValidators
         ],
         imports: [FormsModule]
       });
@@ -728,6 +728,50 @@ export function main() {
            expect(form.valid).toEqual(true);
          }));
 
+      it('should support optional fields with pattern validator', fakeAsync(() => {
+           const fixture = TestBed.createComponent(NgModelMultipleValidators);
+           fixture.componentInstance.required = false;
+           fixture.componentInstance.pattern = '[a-z]+';
+           fixture.detectChanges();
+           tick();
+
+           const form = fixture.debugElement.children[0].injector.get(NgForm);
+           const input = fixture.debugElement.query(By.css('input'));
+
+           input.nativeElement.value = '';
+           dispatchEvent(input.nativeElement, 'input');
+           fixture.detectChanges();
+           expect(form.valid).toBeTruthy();
+
+           input.nativeElement.value = '1';
+           dispatchEvent(input.nativeElement, 'input');
+           fixture.detectChanges();
+           expect(form.valid).toBeFalsy();
+           expect(form.control.hasError('pattern', ['tovalidate'])).toBeTruthy();
+         }));
+
+      it('should support optional fields with minlength validator', fakeAsync(() => {
+           const fixture = TestBed.createComponent(NgModelMultipleValidators);
+           fixture.componentInstance.required = false;
+           fixture.componentInstance.minLen = 2;
+           fixture.detectChanges();
+           tick();
+
+           const form = fixture.debugElement.children[0].injector.get(NgForm);
+           const input = fixture.debugElement.query(By.css('input'));
+
+           input.nativeElement.value = '';
+           dispatchEvent(input.nativeElement, 'input');
+           fixture.detectChanges();
+           expect(form.valid).toBeTruthy();
+
+           input.nativeElement.value = '1';
+           dispatchEvent(input.nativeElement, 'input');
+           fixture.detectChanges();
+           expect(form.valid).toBeFalsy();
+           expect(form.control.hasError('minlength', ['tovalidate'])).toBeTruthy();
+         }));
+
       it('changes on bound properties should change the validation state of the form',
          fakeAsync(() => {
            const fixture = TestBed.createComponent(NgModelValidationBindings);
@@ -1037,6 +1081,20 @@ class NgModelValidationBindings {
   pattern: string;
 }
 
+@Component({
+  selector: 'ng-model-multiple-validators',
+  template: `
+    <form>
+      <input name="tovalidate" ngModel  [required]="required" [minlength]="minLen" [pattern]="pattern">
+    </form>
+  `
+})
+class NgModelMultipleValidators {
+  required: boolean;
+  minLen: number;
+  pattern: string;
+}
+
 function sortedClassList(el: HTMLElement) {
   const l = getDOM().classList(el);
   l.sort();

modules/@angular/forms/test/validators_spec.ts

@@ -44,33 +44,24 @@ export function main() {
          () => { expect(Validators.required(new FormControl(null))).toEqual({'required': true}); });
 
       it('should not error on a non-empty string',
-         () => { expect(Validators.required(new FormControl('not empty'))).toEqual(null); });
+         () => { expect(Validators.required(new FormControl('not empty'))).toBeNull(); });
 
       it('should accept zero as valid',
-         () => { expect(Validators.required(new FormControl(0))).toEqual(null); });
+         () => { expect(Validators.required(new FormControl(0))).toBeNull(); });
     });
 
     describe('minLength', () => {
-      it('should error on an empty string', () => {
+      it('should not error on an empty string',
-        expect(Validators.minLength(2)(new FormControl(''))).toEqual({
+         () => { expect(Validators.minLength(2)(new FormControl(''))).toBeNull(); });
-          'minlength': {'requiredLength': 2, 'actualLength': 0}
-        });
-      });
 
-      it('should error on null', () => {
+      it('should not error on null',
-        expect(Validators.minLength(2)(new FormControl(null))).toEqual({
+         () => { expect(Validators.minLength(2)(new FormControl(null))).toBeNull(); });
-          'minlength': {'requiredLength': 2, 'actualLength': 0}
-        });
-      });
 
-      it('should error on undefined', () => {
+      it('should not error on undefined',
-        expect(Validators.minLength(2)(new FormControl(null))).toEqual({
+         () => { expect(Validators.minLength(2)(new FormControl(null))).toBeNull(); });
-          'minlength': {'requiredLength': 2, 'actualLength': 0}
-        });
-      });
 
       it('should not error on valid strings',
-         () => { expect(Validators.minLength(2)(new FormControl('aa'))).toEqual(null); });
+         () => { expect(Validators.minLength(2)(new FormControl('aa'))).toBeNull(); });
 
       it('should error on short strings', () => {
         expect(Validators.minLength(2)(new FormControl('a'))).toEqual({
@@ -81,13 +72,13 @@ export function main() {
 
     describe('maxLength', () => {
       it('should not error on an empty string',
-         () => { expect(Validators.maxLength(2)(new FormControl(''))).toEqual(null); });
+         () => { expect(Validators.maxLength(2)(new FormControl(''))).toBeNull(); });
 
       it('should not error on null',
-         () => { expect(Validators.maxLength(2)(new FormControl(null))).toEqual(null); });
+         () => { expect(Validators.maxLength(2)(new FormControl(null))).toBeNull(); });
 
       it('should not error on valid strings',
-         () => { expect(Validators.maxLength(2)(new FormControl('aa'))).toEqual(null); });
+         () => { expect(Validators.maxLength(2)(new FormControl('aa'))).toBeNull(); });
 
       it('should error on long strings', () => {
         expect(Validators.maxLength(2)(new FormControl('aaa'))).toEqual({
@@ -98,29 +89,25 @@ export function main() {
 
     describe('pattern', () => {
       it('should not error on an empty string',
-         () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl(''))).toEqual(null); });
+         () => { expect(Validators.pattern('[a-zA-Z ]+')(new FormControl(''))).toBeNull(); });
 
       it('should not error on null',
-         () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl(null))).toEqual(null); });
+         () => { expect(Validators.pattern('[a-zA-Z ]+')(new FormControl(null))).toBeNull(); });
+
+      it('should not error on undefined',
+         () => { expect(Validators.pattern('[a-zA-Z ]+')(new FormControl(null))).toBeNull(); });
 
       it('should not error on null value and "null" pattern',
-         () => { expect(Validators.pattern('null')(new FormControl(null))).toEqual(null); });
+         () => { expect(Validators.pattern('null')(new FormControl(null))).toBeNull(); });
 
-      it('should not error on valid strings', () => {
+      it('should not error on valid strings',
-        expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaAA'))).toEqual(null);
+         () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaAA'))).toBeNull(); });
-      });
 
       it('should error on failure to match string', () => {
         expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaa0'))).toEqual({
           'pattern': {'requiredPattern': '^[a-zA-Z ]*$', 'actualValue': 'aaa0'}
         });
       });
-
-      it('should error on failure to match empty string', () => {
-        expect(Validators.pattern('[a-zA-Z]+')(new FormControl(''))).toEqual({
-          'pattern': {'requiredPattern': '^[a-zA-Z]+$', 'actualValue': ''}
-        });
-      });
     });
 
     describe('compose', () => {
@@ -139,7 +126,7 @@ export function main() {
 
       it('should return null when no errors', () => {
         var c = Validators.compose([Validators.nullValidator, Validators.nullValidator]);
-        expect(c(new FormControl(''))).toEqual(null);
+        expect(c(new FormControl(''))).toBeNull();
       });
 
       it('should ignore nulls', () => {
@@ -166,7 +153,7 @@ export function main() {
       }
 
       it('should return null when given null',
-         () => { expect(Validators.composeAsync(null)).toEqual(null); });
+         () => { expect(Validators.composeAsync(null)).toBeNull(); });
 
       it('should collect errors from all the validators', fakeAsync(() => {
            var c = Validators.composeAsync([
@@ -199,7 +186,7 @@ export function main() {
            (<Promise<any>>c(new FormControl('expected'))).then(v => value = v);
            tick(1);
 
-           expect(value).toEqual(null);
+           expect(value).toBeNull();
          }));
 
       it('should ignore nulls', fakeAsync(() => {