fix(forms): clear errors on disable (#11463)

Closes #11287

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

@@ -190,6 +190,7 @@ export abstract class AbstractControl {
     emitEvent = isPresent(emitEvent) ? emitEvent : true;
 
     this._status = DISABLED;
+    this._errors = null;
     this._forEachChild((control: AbstractControl) => { control.disable({onlySelf: true}); });
     this._updateValue();
 
@@ -232,17 +233,16 @@ export abstract class AbstractControl {
     onlySelf = normalizeBool(onlySelf);
     emitEvent = isPresent(emitEvent) ? emitEvent : true;
 
+    this._setInitialStatus();
     this._updateValue();
-    this._errors = this._runValidator();
-    const originalStatus = this._status;
-    this._status = this._calculateStatus();
 
-    if (this._status == VALID || this._status == PENDING) {
+    if (this.enabled) {
-      this._runAsyncValidator(emitEvent);
+      this._errors = this._runValidator();
-    }
+      this._status = this._calculateStatus();
 
-    if (this._disabledChanged(originalStatus)) {
+      if (this._status === VALID || this._status === PENDING) {
-      this._updateValue();
+        this._runAsyncValidator(emitEvent);
+      }
     }
 
     if (emitEvent) {
@@ -261,6 +261,8 @@ export abstract class AbstractControl {
     this.updateValueAndValidity({onlySelf: true, emitEvent});
   }
 
+  private _setInitialStatus() { this._status = this._allControlsDisabled() ? DISABLED : VALID; }
+
   private _runValidator(): {[key: string]: any} {
     return isPresent(this.validator) ? this.validator(this) : null;
   }
@@ -281,11 +283,6 @@ export abstract class AbstractControl {
     }
   }
 
-  private _disabledChanged(originalStatus: string): boolean {
-    return this._status !== originalStatus &&
-        (this._status === DISABLED || originalStatus === DISABLED);
-  }
-
   /**
    * Sets errors on a form control.
    *

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

@@ -14,13 +14,11 @@ import {isPresent} from '../src/facade/lang';
 import {Validators} from '../src/validators';
 
 export function main() {
-  function asyncValidator(expected: any /** TODO #9100 */, timeouts = {}) {
+  function asyncValidator(expected: string, timeouts = {}) {
-    return (c: any /** TODO #9100 */) => {
+    return (c: AbstractControl) => {
       var resolve: (result: any) => void;
       var promise = new Promise(res => { resolve = res; });
-      var t = isPresent((timeouts as any /** TODO #9100 */)[c.value]) ?
+      var t = isPresent((timeouts as any)[c.value]) ? (timeouts as any)[c.value] : 0;
-          (timeouts as any /** TODO #9100 */)[c.value] :
-          0;
       var res = c.value != expected ? {'async': true} : null;
 
       if (t == 0) {
@@ -799,6 +797,69 @@ export function main() {
         expect(arr.status).toEqual('VALID');
       });
 
+      it('should not run validators on disabled controls', () => {
+        const validator = jasmine.createSpy('validator');
+        const arr = new FormArray([new FormControl()], validator);
+        expect(validator.calls.count()).toEqual(1);
+
+        arr.disable();
+        expect(validator.calls.count()).toEqual(1);
+
+        arr.setValue(['value']);
+        expect(validator.calls.count()).toEqual(1);
+
+        arr.enable();
+        expect(validator.calls.count()).toEqual(2);
+      });
+
+      describe('disabled errors', () => {
+        it('should clear out array errors when disabled', () => {
+          const arr = new FormArray([new FormControl()], () => { return {'expected': true}; });
+          expect(arr.errors).toEqual({'expected': true});
+
+          arr.disable();
+          expect(arr.errors).toEqual(null);
+
+          arr.enable();
+          expect(arr.errors).toEqual({'expected': true});
+        });
+
+        it('should re-populate array errors when enabled from a child', () => {
+          const arr = new FormArray([new FormControl()], () => { return {'expected': true}; });
+          arr.disable();
+          expect(arr.errors).toEqual(null);
+
+          arr.push(new FormControl());
+          expect(arr.errors).toEqual({'expected': true});
+        });
+
+        it('should clear out async array errors when disabled', fakeAsync(() => {
+             const arr = new FormArray([new FormControl()], null, asyncValidator('expected'));
+             tick();
+             expect(arr.errors).toEqual({'async': true});
+
+             arr.disable();
+             expect(arr.errors).toEqual(null);
+
+             arr.enable();
+             tick();
+             expect(arr.errors).toEqual({'async': true});
+           }));
+
+        it('should re-populate async array errors when enabled from a child', fakeAsync(() => {
+             const arr = new FormArray([new FormControl()], null, asyncValidator('expected'));
+             tick();
+             expect(arr.errors).toEqual({'async': true});
+
+             arr.disable();
+             expect(arr.errors).toEqual(null);
+
+             arr.push(new FormControl());
+             tick();
+             expect(arr.errors).toEqual({'async': true});
+           }));
+      });
+
       describe('disabled events', () => {
         let logger: string[];
         let c: FormControl;

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

@@ -15,13 +15,11 @@ import {isPresent} from '../src/facade/lang';
 import {FormArray} from '../src/model';
 
 export function main() {
-  function asyncValidator(expected: any /** TODO #9100 */, timeouts = {}) {
+  function asyncValidator(expected: string, timeouts = {}) {
-    return (c: any /** TODO #9100 */) => {
+    return (c: FormControl) => {
       var resolve: (result: any) => void;
       var promise = new Promise(res => { resolve = res; });
-      var t = isPresent((timeouts as any /** TODO #9100 */)[c.value]) ?
+      var t = isPresent((timeouts as any)[c.value]) ? (timeouts as any)[c.value] : 0;
-          (timeouts as any /** TODO #9100 */)[c.value] :
-          0;
       var res = c.value != expected ? {'async': true} : null;
 
       if (t == 0) {
@@ -573,7 +571,7 @@ export function main() {
     });
 
     describe('valueChanges & statusChanges', () => {
-      var c: any /** TODO #9100 */;
+      let c: FormControl;
 
       beforeEach(() => { c = new FormControl('old', Validators.required); });
 
@@ -892,6 +890,47 @@ export function main() {
         expect(g.touched).toBe(true);
       });
 
+      it('should not run validators on disabled controls', () => {
+        const validator = jasmine.createSpy('validator');
+        const c = new FormControl('', validator);
+        expect(validator.calls.count()).toEqual(1);
+
+        c.disable();
+        expect(validator.calls.count()).toEqual(1);
+
+        c.setValue('value');
+        expect(validator.calls.count()).toEqual(1);
+
+        c.enable();
+        expect(validator.calls.count()).toEqual(2);
+      });
+
+      describe('disabled errors', () => {
+        it('should clear out the errors when disabled', () => {
+          const c = new FormControl('', Validators.required);
+          expect(c.errors).toEqual({required: true});
+
+          c.disable();
+          expect(c.errors).toEqual(null);
+
+          c.enable();
+          expect(c.errors).toEqual({required: true});
+        });
+
+        it('should clear out async errors when disabled', fakeAsync(() => {
+             const c = new FormControl('', null, asyncValidator('expected'));
+             tick();
+             expect(c.errors).toEqual({'async': true});
+
+             c.disable();
+             expect(c.errors).toEqual(null);
+
+             c.enable();
+             tick();
+             expect(c.errors).toEqual({'async': true});
+           }));
+      });
+
       describe('disabled events', () => {
         let logger: string[];
         let c: FormControl;

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

@@ -8,19 +8,17 @@
 
 import {async, fakeAsync, tick} from '@angular/core/testing';
 import {AsyncTestCompleter, beforeEach, ddescribe, describe, iit, inject, it, xit} from '@angular/core/testing/testing_internal';
-import {FormControl, FormGroup, Validators} from '@angular/forms';
+import {AbstractControl, FormControl, FormGroup, Validators} from '@angular/forms';
 
 import {EventEmitter} from '../src/facade/async';
 import {isPresent} from '../src/facade/lang';
 
 export function main() {
-  function asyncValidator(expected: any /** TODO #9100 */, timeouts = {}) {
+  function asyncValidator(expected: string, timeouts = {}) {
-    return (c: any /** TODO #9100 */) => {
+    return (c: AbstractControl) => {
       var resolve: (result: any) => void;
       var promise = new Promise(res => { resolve = res; });
-      var t = isPresent((timeouts as any /** TODO #9100 */)[c.value]) ?
+      var t = isPresent((timeouts as any)[c.value]) ? (timeouts as any)[c.value] : 0;
-          (timeouts as any /** TODO #9100 */)[c.value] :
-          0;
       var res = c.value != expected ? {'async': true} : null;
 
       if (t == 0) {
@@ -801,6 +799,69 @@ export function main() {
         expect(group.status).toEqual('VALID');
       });
 
+      it('should not run validators on disabled controls', () => {
+        const validator = jasmine.createSpy('validator');
+        const g = new FormGroup({'one': new FormControl()}, validator);
+        expect(validator.calls.count()).toEqual(1);
+
+        g.disable();
+        expect(validator.calls.count()).toEqual(1);
+
+        g.setValue({one: 'value'});
+        expect(validator.calls.count()).toEqual(1);
+
+        g.enable();
+        expect(validator.calls.count()).toEqual(2);
+      });
+
+      describe('disabled errors', () => {
+        it('should clear out group errors when disabled', () => {
+          const g = new FormGroup({'one': new FormControl()}, () => { return {'expected': true}; });
+          expect(g.errors).toEqual({'expected': true});
+
+          g.disable();
+          expect(g.errors).toEqual(null);
+
+          g.enable();
+          expect(g.errors).toEqual({'expected': true});
+        });
+
+        it('should re-populate group errors when enabled from a child', () => {
+          const g = new FormGroup({'one': new FormControl()}, () => { return {'expected': true}; });
+          g.disable();
+          expect(g.errors).toEqual(null);
+
+          g.addControl('two', new FormControl());
+          expect(g.errors).toEqual({'expected': true});
+        });
+
+        it('should clear out async group errors when disabled', fakeAsync(() => {
+             const g = new FormGroup({'one': new FormControl()}, null, asyncValidator('expected'));
+             tick();
+             expect(g.errors).toEqual({'async': true});
+
+             g.disable();
+             expect(g.errors).toEqual(null);
+
+             g.enable();
+             tick();
+             expect(g.errors).toEqual({'async': true});
+           }));
+
+        it('should re-populate async group errors when enabled from a child', fakeAsync(() => {
+             const g = new FormGroup({'one': new FormControl()}, null, asyncValidator('expected'));
+             tick();
+             expect(g.errors).toEqual({'async': true});
+
+             g.disable();
+             expect(g.errors).toEqual(null);
+
+             g.addControl('two', new FormControl());
+             tick();
+             expect(g.errors).toEqual({'async': true});
+           }));
+      });
+
       describe('disabled events', () => {
         let logger: string[];
         let c: FormControl;