From 1df3aefb81e9bc505c9d2b8bfd42de43a6043a5f Mon Sep 17 00:00:00 2001 From: Artem Lanovyy Date: Sun, 27 Jan 2019 23:59:47 +0200 Subject: [PATCH] fix(forms): mark form as pristine before emitting value and status change events (#28395) BREAKING CHANGE Previous to this change, when a control was reset, value and status change events would be emitted before the control was reset to pristine. As a result, if one were to check a control's pristine state in a valueChange listener, it would appear that the control was still dirty after reset. This change delays emission of value and status change events until after controls have been marked pristine. This means the pristine state will be reset as expected if one checks in a listener. Theoretically, there could be applications depending on checking whether a control *used to be dirty*, so this is marked as breaking. In these cases, apps should cache the state on the app side before calling reset. Fixes #28130 PR Close #28395 --- packages/forms/src/model.ts | 4 ++-- packages/forms/test/form_array_spec.ts | 17 +++++++++++++++++ packages/forms/test/form_group_spec.ts | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/forms/src/model.ts b/packages/forms/src/model.ts index c8ee0d5d3b..23b066866c 100644 --- a/packages/forms/src/model.ts +++ b/packages/forms/src/model.ts @@ -1477,9 +1477,9 @@ export class FormGroup extends AbstractControl { this._forEachChild((control: AbstractControl, name: string) => { control.reset(value[name], {onlySelf: true, emitEvent: options.emitEvent}); }); - this.updateValueAndValidity(options); this._updatePristine(options); this._updateTouched(options); + this.updateValueAndValidity(options); } /** @@ -1878,9 +1878,9 @@ export class FormArray extends AbstractControl { this._forEachChild((control: AbstractControl, index: number) => { control.reset(value[index], {onlySelf: true, emitEvent: options.emitEvent}); }); - this.updateValueAndValidity(options); this._updatePristine(options); this._updateTouched(options); + this.updateValueAndValidity(options); } /** diff --git a/packages/forms/test/form_array_spec.ts b/packages/forms/test/form_array_spec.ts index ec40306ce5..2a94c68a44 100644 --- a/packages/forms/test/form_array_spec.ts +++ b/packages/forms/test/form_array_spec.ts @@ -586,6 +586,23 @@ import {of } from 'rxjs'; a.reset(); expect(logger).toEqual(['control1', 'control2', 'array', 'form']); }); + + it('should mark as pristine and not dirty before emitting valueChange and statusChange events when resetting', + () => { + const pristineAndNotDirty = () => { + expect(a.pristine).toBe(true); + expect(a.dirty).toBe(false); + }; + + c2.markAsDirty(); + expect(a.pristine).toBe(false); + expect(a.dirty).toBe(true); + + a.valueChanges.subscribe(pristineAndNotDirty); + a.statusChanges.subscribe(pristineAndNotDirty); + + a.reset(); + }); }); }); diff --git a/packages/forms/test/form_group_spec.ts b/packages/forms/test/form_group_spec.ts index 2996cfc010..9e6180fa6a 100644 --- a/packages/forms/test/form_group_spec.ts +++ b/packages/forms/test/form_group_spec.ts @@ -654,6 +654,23 @@ import {of } from 'rxjs'; g.reset({'one': {value: '', disabled: true}}); expect(logger).toEqual(['control1', 'control2', 'group', 'form']); }); + + it('should mark as pristine and not dirty before emitting valueChange and statusChange events when resetting', + () => { + const pristineAndNotDirty = () => { + expect(form.pristine).toBe(true); + expect(form.dirty).toBe(false); + }; + + c3.markAsDirty(); + expect(form.pristine).toBe(false); + expect(form.dirty).toBe(true); + + form.valueChanges.subscribe(pristineAndNotDirty); + form.statusChanges.subscribe(pristineAndNotDirty); + + form.reset(); + }); }); });