fix(forms): prevent event emission on enable/disable when emitEvent i…

…s false (angular#12366) (angular#21018) Previously, the emitEvent flag was only checked when emitting on the current control. Thus, if the control was part of a hierarchy, events were emitted on the parent and the childrens. This fixes the issue by properly passing the emitEvent flag to both parent and childrens. Fixes angular#12366 PR Close angular#21018
leo6104 · Mar 25, 2018 · ab98cb0 · ab98cb0
1 parent 443aa21
commit ab98cb0
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 7 deletions.
diff --git a/packages/forms/src/model.ts b/packages/forms/src/model.ts
@@ -366,15 +366,16 @@ export abstract class AbstractControl {
   disable(opts: {onlySelf?: boolean, emitEvent?: boolean} = {}): void {
     (this as{status: string}).status = DISABLED;
     (this as{errors: ValidationErrors | null}).errors = null;
-    this._forEachChild((control: AbstractControl) => { control.disable({onlySelf: true}); });
+    this._forEachChild(
+        (control: AbstractControl) => { control.disable({...opts, onlySelf: true}); });
     this._updateValue();
 
     if (opts.emitEvent !== false) {
       (this.valueChanges as EventEmitter<any>).emit(this.value);
       (this.statusChanges as EventEmitter<string>).emit(this.status);
     }
 
-    this._updateAncestors(!!opts.onlySelf);
+    this._updateAncestors(opts);
     this._onDisabledChange.forEach((changeFn) => changeFn(true));
   }
 
@@ -387,16 +388,17 @@ export abstract class AbstractControl {
    */
   enable(opts: {onlySelf?: boolean, emitEvent?: boolean} = {}): void {
     (this as{status: string}).status = VALID;
-    this._forEachChild((control: AbstractControl) => { control.enable({onlySelf: true}); });
+    this._forEachChild(
+        (control: AbstractControl) => { control.enable({...opts, onlySelf: true}); });
     this.updateValueAndValidity({onlySelf: true, emitEvent: opts.emitEvent});
 
-    this._updateAncestors(!!opts.onlySelf);
+    this._updateAncestors(opts);
     this._onDisabledChange.forEach((changeFn) => changeFn(false));
   }
 
-  private _updateAncestors(onlySelf: boolean) {
-    if (this._parent && !onlySelf) {
-      this._parent.updateValueAndValidity();
+  private _updateAncestors(opts: {onlySelf?: boolean, emitEvent?: boolean}) {
+    if (this._parent && !opts.onlySelf) {
+      this._parent.updateValueAndValidity(opts);
       this._parent._updatePristine();
       this._parent._updateTouched();
     }

diff --git a/packages/forms/test/form_array_spec.ts b/packages/forms/test/form_array_spec.ts
@@ -1053,6 +1053,28 @@ import {of } from 'rxjs/observable/of';
           expect(logger).toEqual(['control', 'array', 'form']);
         });
 
+        it('should not emit value change events when emitEvent = false', () => {
+          c.valueChanges.subscribe(() => logger.push('control'));
+          a.valueChanges.subscribe(() => logger.push('array'));
+          form.valueChanges.subscribe(() => logger.push('form'));
+
+          a.disable({emitEvent: false});
+          expect(logger).toEqual([]);
+          a.enable({emitEvent: false});
+          expect(logger).toEqual([]);
+        });
+
+        it('should not emit status change events when emitEvent = false', () => {
+          c.statusChanges.subscribe(() => logger.push('control'));
+          a.statusChanges.subscribe(() => logger.push('array'));
+          form.statusChanges.subscribe(() => logger.push('form'));
+
+          a.disable({emitEvent: false});
+          expect(logger).toEqual([]);
+          a.enable({emitEvent: false});
+          expect(logger).toEqual([]);
+        });
+
       });
 
       describe('setControl()', () => {

diff --git a/packages/forms/test/form_control_spec.ts b/packages/forms/test/form_control_spec.ts
@@ -1139,6 +1139,26 @@ import {FormArray} from '@angular/forms/src/model';
           expect(fn).toThrowError(`Expected validator to return Promise or Observable.`);
         });
 
+        it('should not emit value change events when emitEvent = false', () => {
+          c.valueChanges.subscribe(() => logger.push('control'));
+          g.valueChanges.subscribe(() => logger.push('group'));
+
+          c.disable({emitEvent: false});
+          expect(logger).toEqual([]);
+          c.enable({emitEvent: false});
+          expect(logger).toEqual([]);
+        });
+
+        it('should not emit status change events when emitEvent = false', () => {
+          c.statusChanges.subscribe(() => logger.push('control'));
+          g.statusChanges.subscribe(() => logger.push('form'));
+
+          c.disable({emitEvent: false});
+          expect(logger).toEqual([]);
+          c.enable({emitEvent: false});
+          expect(logger).toEqual([]);
+        });
+
       });
     });
   });

diff --git a/packages/forms/test/form_group_spec.ts b/packages/forms/test/form_group_spec.ts
@@ -1045,6 +1045,28 @@ import {of } from 'rxjs/observable/of';
           expect(logger).toEqual(['control', 'group', 'form']);
         });
 
+        it('should not emit value change events when emitEvent = false', () => {
+          c.valueChanges.subscribe(() => logger.push('control'));
+          g.valueChanges.subscribe(() => logger.push('group'));
+          form.valueChanges.subscribe(() => logger.push('form'));
+
+          g.disable({emitEvent: false});
+          expect(logger).toEqual([]);
+          g.enable({emitEvent: false});
+          expect(logger).toEqual([]);
+        });
+
+        it('should not emit status change events when emitEvent = false', () => {
+          c.statusChanges.subscribe(() => logger.push('control'));
+          g.statusChanges.subscribe(() => logger.push('group'));
+          form.statusChanges.subscribe(() => logger.push('form'));
+
+          g.disable({emitEvent: false});
+          expect(logger).toEqual([]);
+          g.enable({emitEvent: false});
+          expect(logger).toEqual([]);
+        });
+
       });
 
     });