fix(forms): properly handle the change to the FormGroup shape (#40829)

Currently the code in the `FormGroupDirective` assumes that the shape of the underlying `FormGroup` never
changes and `FormControl`s are not replaced with other types. In practice this is possible and Forms code
should be able to process such changes in FormGroup shape.

This commit adds extra check to the `FormGroupDirective` class to avoid applying FormControl-specific to
other types.

Fixes #13788.

PR Close #40829
This commit is contained in:
Andrew Kushnir 2021-02-12 16:26:08 -08:00 committed by Jessica Janiuk
parent e3ea8630bc
commit 8a9fe49a2a
2 changed files with 197 additions and 5 deletions

View File

@ -295,13 +295,22 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
/** @internal */
_updateDomValue() {
this.directives.forEach(dir => {
const newCtrl: any = this.form.get(dir.path);
if (dir.control !== newCtrl) {
const oldCtrl = dir.control;
const newCtrl = this.form.get(dir.path);
if (oldCtrl !== newCtrl) {
// Note: the value of the `dir.control` may not be defined, for example when it's a first
// `FormControl` that is added to a `FormGroup` instance (via `addControl` call).
cleanUpControl(dir.control || null, dir);
if (newCtrl) setUpControl(newCtrl, dir);
(dir as {control: FormControl}).control = newCtrl;
cleanUpControl(oldCtrl || null, dir);
// Check whether new control at the same location inside the corresponding `FormGroup` is an
// instance of `FormControl` and perform control setup only if that's the case.
// Note: we don't need to clear the list of directives (`this.directives`) here, it would be
// taken care of in the `removeControl` method invoked when corresponding `formControlName`
// directive instance is being removed (invoked from `FormControlName.ngOnDestroy`).
if (newCtrl instanceof FormControl) {
setUpControl(newCtrl, dir);
(dir as {control: FormControl}).control = newCtrl;
}
}
});

View File

@ -663,6 +663,189 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
expect(input.nativeElement.getAttribute('disabled')).toBe(null);
});
});
describe('dynamic change of FormGroup and FormArray shapes', () => {
it('should handle FormControl and FormGroup swap', () => {
@Component({
template: `
<form [formGroup]="form">
<input formControlName="name" id="standalone-id" *ngIf="!showAsGroup">
<ng-container formGroupName="name" *ngIf="showAsGroup">
<input formControlName="control" id="inside-group-id">
</ng-container>
</form>
`
})
class App {
showAsGroup = false;
form!: FormGroup;
useStandaloneControl() {
this.showAsGroup = false;
this.form = new FormGroup({
name: new FormControl('standalone'),
});
}
useControlInsideGroup() {
this.showAsGroup = true;
this.form = new FormGroup({
name: new FormGroup({
control: new FormControl('inside-group'),
})
});
}
}
const fixture = initTest(App);
fixture.componentInstance.useStandaloneControl();
fixture.detectChanges();
let input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('standalone-id');
expect(input.value).toBe('standalone');
// Replace `FormControl` with `FormGroup` at the same location
// in data model and trigger change detection.
fixture.componentInstance.useControlInsideGroup();
fixture.detectChanges();
input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('inside-group-id');
expect(input.value).toBe('inside-group');
// Swap `FormGroup` with `FormControl` back at the same location
// in data model and trigger change detection.
fixture.componentInstance.useStandaloneControl();
fixture.detectChanges();
input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('standalone-id');
expect(input.value).toBe('standalone');
});
it('should handle FormControl and FormArray swap', () => {
@Component({
template: `
<form [formGroup]="form">
<input formControlName="name" id="standalone-id" *ngIf="!showAsArray">
<ng-container formArrayName="name" *ngIf="showAsArray">
<input formControlName="0" id="inside-array-id">
</ng-container>
</form>
`
})
class App {
showAsArray = false;
form!: FormGroup;
useStandaloneControl() {
this.showAsArray = false;
this.form = new FormGroup({
name: new FormControl('standalone'),
});
}
useControlInsideArray() {
this.showAsArray = true;
this.form = new FormGroup({
name: new FormArray([
new FormControl('inside-array') //
])
});
}
}
const fixture = initTest(App);
fixture.componentInstance.useStandaloneControl();
fixture.detectChanges();
let input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('standalone-id');
expect(input.value).toBe('standalone');
// Replace `FormControl` with `FormArray` at the same location
// in data model and trigger change detection.
fixture.componentInstance.useControlInsideArray();
fixture.detectChanges();
input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('inside-array-id');
expect(input.value).toBe('inside-array');
// Swap `FormArray` with `FormControl` back at the same location
// in data model and trigger change detection.
fixture.componentInstance.useStandaloneControl();
fixture.detectChanges();
input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('standalone-id');
expect(input.value).toBe('standalone');
});
it('should handle FormGroup and FormArray swap', () => {
@Component({
template: `
<form [formGroup]="form">
<ng-container formGroupName="name" *ngIf="!showAsArray">
<input formControlName="control" id="inside-group-id">
</ng-container>
<ng-container formArrayName="name" *ngIf="showAsArray">
<input formControlName="0" id="inside-array-id">
</ng-container>
</form>
`
})
class App {
showAsArray = false;
form!: FormGroup;
useControlInsideGroup() {
this.showAsArray = false;
this.form = new FormGroup({
name: new FormGroup({
control: new FormControl('inside-group'),
})
});
}
useControlInsideArray() {
this.showAsArray = true;
this.form = new FormGroup({
name: new FormArray([
new FormControl('inside-array') //
])
});
}
}
const fixture = initTest(App);
fixture.componentInstance.useControlInsideGroup();
fixture.detectChanges();
let input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('inside-group-id');
expect(input.value).toBe('inside-group');
// Replace `FormGroup` with `FormArray` at the same location
// in data model and trigger change detection.
fixture.componentInstance.useControlInsideArray();
fixture.detectChanges();
input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('inside-array-id');
expect(input.value).toBe('inside-array');
// Swap `FormArray` with `FormGroup` back at the same location
// in data model and trigger change detection.
fixture.componentInstance.useControlInsideGroup();
fixture.detectChanges();
input = fixture.nativeElement.querySelector('input');
expect(input.id).toBe('inside-group-id');
expect(input.value).toBe('inside-group');
});
});
});
describe('user input', () => {