fix(forms): error if control is removed as a result of another one being reset (#40462)

When a form is reset, it goes through `_forEachChild` to call `reset` on each of its children.
The problem is that if a control is removed while the loop is running (e.g. by a subscription),
the form will throw an error, because it built up the list of available control before the loop
started.

These changes fix the issue by adding a null check before invoing the callback.

Fixes #33401.

PR Close #40462
This commit is contained in:
Kristiyan Kostadinov 2021-01-17 12:07:26 +01:00 committed by Andrew Kushnir
parent afabb83696
commit 1d1304c70c
2 changed files with 52 additions and 4 deletions

View File

@ -1690,7 +1690,13 @@ export class FormGroup extends AbstractControl {
/** @internal */ /** @internal */
_forEachChild(cb: (v: any, k: string) => void): void { _forEachChild(cb: (v: any, k: string) => void): void {
Object.keys(this.controls).forEach(k => cb(this.controls[k], k)); Object.keys(this.controls).forEach(key => {
// The list of controls can change (for ex. controls might be removed) while the loop
// is running (as a result of invoking Forms API in `valueChanges` subscription), so we
// have to null check before invoking the callback.
const control = this.controls[key];
control && cb(control, key);
});
} }
/** @internal */ /** @internal */

View File

@ -7,13 +7,13 @@
*/ */
import {ɵgetDOM as getDOM} from '@angular/common'; import {ɵgetDOM as getDOM} from '@angular/common';
import {Component, Directive, forwardRef, Input, Type} from '@angular/core'; import {Component, Directive, forwardRef, Input, OnDestroy, Type} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {expect} from '@angular/core/testing/src/testing_internal'; import {expect} from '@angular/core/testing/src/testing_internal';
import {AbstractControl, AsyncValidator, AsyncValidatorFn, COMPOSITION_BUFFER_MODE, ControlValueAccessor, DefaultValueAccessor, FormArray, FormControl, FormControlDirective, FormControlName, FormGroup, FormGroupDirective, FormsModule, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, ReactiveFormsModule, Validator, Validators} from '@angular/forms'; import {AbstractControl, AsyncValidator, AsyncValidatorFn, COMPOSITION_BUFFER_MODE, ControlValueAccessor, DefaultValueAccessor, FormArray, FormControl, FormControlDirective, FormControlName, FormGroup, FormGroupDirective, FormsModule, NG_ASYNC_VALIDATORS, NG_VALIDATORS, NG_VALUE_ACCESSOR, ReactiveFormsModule, Validator, Validators} from '@angular/forms';
import {By} from '@angular/platform-browser/src/dom/debug/by'; import {By} from '@angular/platform-browser/src/dom/debug/by';
import {dispatchEvent, sortedClassList} from '@angular/platform-browser/testing/src/browser_util'; import {dispatchEvent, sortedClassList} from '@angular/platform-browser/testing/src/browser_util';
import {merge, NEVER, of, timer} from 'rxjs'; import {merge, NEVER, of, Subscription, timer} from 'rxjs';
import {map, tap} from 'rxjs/operators'; import {map, tap} from 'rxjs/operators';
import {MyInput, MyInputForm} from './value_accessor_integration_spec'; import {MyInput, MyInputForm} from './value_accessor_integration_spec';
@ -1129,6 +1129,48 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
expect(control.dirty).toBe(false, 'Expected pending dirty value to reset.'); expect(control.dirty).toBe(false, 'Expected pending dirty value to reset.');
}); });
it('should be able to remove a control as a result of another control being reset', () => {
@Component({
template: `
<form [formGroup]="form">
<input formControlName="name">
<input formControlName="surname">
</form>
`
})
class App implements OnDestroy {
private _subscription: Subscription;
form = new FormGroup({
name: new FormControl('Frodo'),
surname: new FormControl('Baggins'),
});
constructor() {
this._subscription = this.form.controls.name.valueChanges.subscribe(value => {
if (!value) {
this.form.removeControl('surname');
}
});
}
ngOnDestroy() {
this._subscription.unsubscribe();
}
}
const fixture = initTest(App);
fixture.detectChanges();
expect(fixture.componentInstance.form.value).toEqual({name: 'Frodo', surname: 'Baggins'});
expect(() => {
fixture.componentInstance.form.reset();
fixture.detectChanges();
}).not.toThrow();
expect(fixture.componentInstance.form.value).toEqual({name: null});
});
it('should not emit valueChanges or statusChanges until blur', () => { it('should not emit valueChanges or statusChanges until blur', () => {
const fixture = initTest(FormControlComp); const fixture = initTest(FormControlComp);
const control = new FormControl('', {validators: Validators.required, updateOn: 'blur'}); const control = new FormControl('', {validators: Validators.required, updateOn: 'blur'});
@ -4392,4 +4434,4 @@ class MultipleFormControls {
class NgForFormControlWithValidators { class NgForFormControlWithValidators {
form = new FormGroup({login: new FormControl('a')}); form = new FormGroup({login: new FormControl('a')});
logins = ['a', 'b', 'c']; logins = ['a', 'b', 'c'];
} }