fix(forms): allow `patchValue()` method of `FormGroup` and `FormArray` classes to skip `null` values (#40534)

Prior to this commit, the `patchValue()` of the `FormGroup` and `FormArray` classes used to throw an exception
when the `value` argument contained a data structure that has `null` or `undefined` as a value for a field
that represents an instance of `FormGroup` or `FormArray` (for `FormControl` it's not a problem, since it
doesn't have nested controls), since the `patchValue()` method tried to iterate over provided values to
match current data structure.

This commit updates the `patchValue()` logic in `FormGroup` and `FormArray` classes to just ignore `null` and
`undefined` values (without any changes to corresponding `FormGroup` and `FormArray` instances). This
behavior looks inline with the `patchValue()` method goal of "doing its best to match the values to the
correct controls" (quote from docs).

Fixes #36672.
Fixes #21021.

PR Close #40534
This commit is contained in:
Andrew Kushnir 2021-01-22 15:07:03 -08:00 committed by Jessica Janiuk
parent 4d66185cbc
commit c9fe455fa2
3 changed files with 89 additions and 12 deletions

View File

@ -1569,14 +1569,18 @@ export class FormGroup extends AbstractControl {
* * `onlySelf`: When true, each change only affects this control and not its parent. Default is
* true.
* * `emitEvent`: When true or not supplied (the default), both the `statusChanges` and
* `valueChanges`
* observables emit events with the latest status and value when the control value is updated.
* When false, no events are emitted.
* The configuration options are passed to the {@link AbstractControl#updateValueAndValidity
* updateValueAndValidity} method.
* `valueChanges` observables emit events with the latest status and value when the control value
* is updated. When false, no events are emitted. The configuration options are passed to
* the {@link AbstractControl#updateValueAndValidity updateValueAndValidity} method.
*/
patchValue(value: {[key: string]: any}, options: {onlySelf?: boolean, emitEvent?: boolean} = {}):
void {
// Even though the `value` argument type doesn't allow `null` and `undefined` values, the
// `patchValue` can be called recursively and inner data structures might have these values, so
// we just ignore such cases when a field containing FormGroup instance receives `null` or
// `undefined` as a value.
if (value == null /* both `null` and `undefined` */) return;
Object.keys(value).forEach(name => {
if (this.controls[name]) {
this.controls[name].patchValue(value[name], {onlySelf: true, emitEvent: options.emitEvent});
@ -2002,13 +2006,17 @@ export class FormArray extends AbstractControl {
* * `onlySelf`: When true, each change only affects this control, and not its parent. Default
* is false.
* * `emitEvent`: When true or not supplied (the default), both the `statusChanges` and
* `valueChanges`
* observables emit events with the latest status and value when the control value is updated.
* When false, no events are emitted.
* The configuration options are passed to the {@link AbstractControl#updateValueAndValidity
* updateValueAndValidity} method.
* `valueChanges` observables emit events with the latest status and value when the control value
* is updated. When false, no events are emitted. The configuration options are passed to
* the {@link AbstractControl#updateValueAndValidity updateValueAndValidity} method.
*/
patchValue(value: any[], options: {onlySelf?: boolean, emitEvent?: boolean} = {}): void {
// Even though the `value` argument type doesn't allow `null` and `undefined` values, the
// `patchValue` can be called recursively and inner data structures might have these values, so
// we just ignore such cases when a field containing FormArray instance receives `null` or
// `undefined` as a value.
if (value == null /* both `null` and `undefined` */) return;
value.forEach((newValue: any, index: number) => {
if (this.at(index)) {
this.at(index).patchValue(newValue, {onlySelf: true, emitEvent: options.emitEvent});

View File

@ -266,12 +266,13 @@ describe('FormArray', () => {
});
describe('patchValue', () => {
let c: FormControl, c2: FormControl, a: FormArray;
let c: FormControl, c2: FormControl, a: FormArray, a2: FormArray;
beforeEach(() => {
c = new FormControl('');
c2 = new FormControl('');
a = new FormArray([c, c2]);
a2 = new FormArray([a]);
});
it('should set its own value', () => {
@ -329,6 +330,16 @@ describe('FormArray', () => {
expect(a.value).toEqual(['', '']);
});
it('should ignore a array if `null` or `undefined` are used as values', () => {
const INITIAL_STATE = [['', '']];
a2.patchValue([null]);
expect(a2.value).toEqual(INITIAL_STATE);
a2.patchValue([undefined]);
expect(a2.value).toEqual(INITIAL_STATE);
});
describe('patchValue() events', () => {
let form: FormGroup;
let logger: any[];
@ -358,6 +369,23 @@ describe('FormArray', () => {
expect(logger).toEqual(['control1', 'array', 'form']);
});
it('should not emit valueChange events for skipped controls (represented as `null` or `undefined`)',
() => {
const logEvent = () => logger.push('valueChanges event');
const [formArrayControl1, formArrayControl2] = (a2.controls as FormArray[])[0].controls;
formArrayControl1.valueChanges.subscribe(logEvent);
formArrayControl2.valueChanges.subscribe(logEvent);
a2.patchValue([null]);
a2.patchValue([undefined]);
// No events are expected in `valueChanges` since
// all controls were skipped in `patchValue`.
expect(logger).toEqual([]);
});
it('should not fire an event when explicitly specified', fakeAsync(() => {
form.valueChanges.subscribe((value) => {
throw 'Should not happen';

View File

@ -311,12 +311,16 @@ describe('FormGroup', () => {
});
describe('patchValue', () => {
let c: FormControl, c2: FormControl, g: FormGroup;
let c: FormControl, c2: FormControl, g: FormGroup, g2: FormGroup;
beforeEach(() => {
c = new FormControl('');
c2 = new FormControl('');
g = new FormGroup({'one': c, 'two': c2});
g2 = new FormGroup({
'array': new FormArray([new FormControl(1), new FormControl(2)]),
'group': new FormGroup({'one': new FormControl(3)}),
});
});
it('should set its own value', () => {
@ -374,6 +378,22 @@ describe('FormGroup', () => {
expect(g.value).toEqual({'one': '', 'two': ''});
});
it('should ignore a control if `null` or `undefined` are used as values', () => {
const INITIAL_STATE = {'array': [1, 2], 'group': {'one': 3}};
g2.patchValue({'array': null});
expect(g2.value).toEqual(INITIAL_STATE);
g2.patchValue({'array': undefined});
expect(g2.value).toEqual(INITIAL_STATE);
g2.patchValue({'group': null});
expect(g2.value).toEqual(INITIAL_STATE);
g2.patchValue({'group': undefined});
expect(g2.value).toEqual(INITIAL_STATE);
});
describe('patchValue() events', () => {
let form: FormGroup;
let logger: any[];
@ -403,6 +423,27 @@ describe('FormGroup', () => {
expect(logger).toEqual(['control1', 'group', 'form']);
});
it('should not emit valueChange events for skipped controls (represented as `null` or `undefined`)',
() => {
const logEvent = () => logger.push('valueChanges event');
const [formArrayControl1, formArrayControl2] = (g2.controls.array as FormArray).controls;
const formGroupControl = (g2.controls.group as FormGroup).controls.one;
formArrayControl1.valueChanges.subscribe(logEvent);
formArrayControl2.valueChanges.subscribe(logEvent);
formGroupControl.valueChanges.subscribe(logEvent);
g2.patchValue({'array': null});
g2.patchValue({'array': undefined});
g2.patchValue({'group': null});
g2.patchValue({'group': undefined});
// No events are expected in `valueChanges` since
// all controls were skipped in `patchValue`.
expect(logger).toEqual([]);
});
it('should not fire an event when explicitly specified', fakeAsync(() => {
form.valueChanges.subscribe((value) => {
throw 'Should not happen';