Skip to content

Commit

Permalink
fix(forms): Make NgControlStatus host bindings OnPush compatible
Browse files Browse the repository at this point in the history
This commit makes the host bindings of `NgControlStatus[Group]`
compatible with `OnPush` components. Note that this intentionally _does not_
expose any new APIs in the forms module. The goal is only to remove
unpreventable `ExpressionChangedAfterItHasBeenCheckedError` in the forms
code that developers do not have control over.
  • Loading branch information
atscott committed May 7, 2024
1 parent 67bb310 commit 4456905
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 3 deletions.
11 changes: 11 additions & 0 deletions packages/forms/src/directives/ng_control_status.ts
Expand Up @@ -26,6 +26,8 @@ export class AbstractControlStatus {
}

protected get isTouched() {
// track the touched signal
this._cd?.control?._touched?.();
return !!this._cd?.control?.touched;
}

Expand All @@ -34,26 +36,35 @@ export class AbstractControlStatus {
}

protected get isPristine() {
// track the pristine signal
this._cd?.control?._pristine?.();
return !!this._cd?.control?.pristine;
}

protected get isDirty() {
// pristine signal already tracked above
return !!this._cd?.control?.dirty;
}

protected get isValid() {
// track the status signal
this._cd?.control?._status?.();
return !!this._cd?.control?.valid;
}

protected get isInvalid() {
// status signal already tracked above
return !!this._cd?.control?.invalid;
}

protected get isPending() {
// status signal already tracked above
return !!this._cd?.control?.pending;
}

protected get isSubmitted() {
// track the submitted signal
(this._cd as Writable<NgForm | FormGroupDirective> | null)?._submitted?.();
// We check for the `submitted` field from `NgForm` and `FormGroupDirective` classes, but
// we avoid instanceof checks to prevent non-tree-shakable references to those types.
return !!(this._cd as Writable<NgForm | FormGroupDirective> | null)?.submitted;
Expand Down
5 changes: 5 additions & 0 deletions packages/forms/src/directives/ng_form.ts
Expand Up @@ -16,6 +16,7 @@ import {
Optional,
Provider,
Self,
signal,
ɵWritable as Writable,
} from '@angular/core';

Expand Down Expand Up @@ -127,6 +128,8 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
* Returns whether the form submission has been triggered.
*/
public readonly submitted: boolean = false;
/** @internal */
readonly _submitted = signal(false);

private _directives = new Set<NgModel>();

Expand Down Expand Up @@ -328,6 +331,7 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
*/
onSubmit($event: Event): boolean {
(this as Writable<this>).submitted = true;
this._submitted.set(true);
syncPendingControls(this.form, this._directives);
this.ngSubmit.emit($event);
// Forms with `method="dialog"` have some special behavior
Expand All @@ -352,6 +356,7 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
resetForm(value: any = undefined): void {
this.form.reset(value);
(this as Writable<this>).submitted = false;
this._submitted.set(false);
}

private _setUpdateStrategy() {
Expand Down
Expand Up @@ -18,6 +18,7 @@ import {
Output,
Provider,
Self,
signal,
SimpleChanges,
ɵWritable as Writable,
} from '@angular/core';
Expand Down Expand Up @@ -87,10 +88,12 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
* Reports whether the form submission has been triggered.
*/
public readonly submitted: boolean = false;
/** @internal */
readonly _submitted = signal(false);

/**
* Reference to an old form group input value, which is needed to cleanup old instance in case it
* was replaced with a new one.
* Reference to an old form group input value, which is needed to cleanup
* old instance in case it was replaced with a new one.
*/
private _oldForm: FormGroup | undefined;

Expand Down Expand Up @@ -300,6 +303,7 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
*/
onSubmit($event: Event): boolean {
(this as Writable<this>).submitted = true;
this._submitted.set(true);
syncPendingControls(this.form, this.directives);
this.ngSubmit.emit($event);
// Forms with `method="dialog"` have some special behavior that won't reload the page and that
Expand All @@ -325,6 +329,7 @@ export class FormGroupDirective extends ControlContainer implements Form, OnChan
resetForm(value: any = undefined): void {
this.form.reset(value);
(this as Writable<this>).submitted = false;
this._submitted.set(false);
}

/** @internal */
Expand Down
28 changes: 27 additions & 1 deletion packages/forms/src/model/abstract_model.ts
Expand Up @@ -6,7 +6,12 @@
* found in the LICENSE file at https://angular.io/license
*/

import {EventEmitter, ɵRuntimeError as RuntimeError, ɵWritable as Writable} from '@angular/core';
import {
EventEmitter,
signal,
ɵRuntimeError as RuntimeError,
ɵWritable as Writable,
} from '@angular/core';
import {Observable, Subject} from 'rxjs';

import {
Expand Down Expand Up @@ -569,6 +574,8 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
* both valid AND invalid or invalid AND disabled.
*/
public readonly status!: FormControlStatus;
/** @internal */
readonly _status = signal<FormControlStatus | null>(null);

/**
* A control is `valid` when its `status` is `VALID`.
Expand Down Expand Up @@ -649,6 +656,9 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
*/
public readonly pristine: boolean = true;

/** @internal */
readonly _pristine = signal(true);

/**
* A control is `dirty` if the user has changed the value
* in the UI.
Expand All @@ -668,6 +678,9 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
*/
public readonly touched: boolean = false;

/** @internal */
readonly _touched = signal(false);

/**
* True if the control has not been marked as touched
*
Expand Down Expand Up @@ -933,6 +946,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
): void {
const changed = this.touched === false;
(this as Writable<this>).touched = true;
this._touched.set(this.touched);

const sourceControl = opts.sourceControl ?? this;
if (this._parent && !opts.onlySelf) {
Expand Down Expand Up @@ -993,6 +1007,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
): void {
const changed = this.touched === true;
(this as Writable<this>).touched = false;
this._touched.set(this.touched);
this._pendingTouched = false;

const sourceControl = opts.sourceControl ?? this;
Expand Down Expand Up @@ -1039,6 +1054,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
): void {
const changed = this.pristine === true;
(this as Writable<this>).pristine = false;
this._pristine.set(false);

const sourceControl = opts.sourceControl ?? this;
if (this._parent && !opts.onlySelf) {
Expand Down Expand Up @@ -1083,6 +1099,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
): void {
const changed = this.pristine === false;
(this as Writable<this>).pristine = true;
this._pristine.set(true);
this._pendingDirty = false;

const sourceControl = opts.sourceControl ?? this;
Expand Down Expand Up @@ -1130,6 +1147,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
opts: {onlySelf?: boolean; emitEvent?: boolean; sourceControl?: AbstractControl} = {},
): void {
(this as Writable<this>).status = PENDING;
this._status.set(PENDING);

const sourceControl = opts.sourceControl ?? this;
if (opts.emitEvent !== false) {
Expand Down Expand Up @@ -1172,6 +1190,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
const skipPristineCheck = this._parentMarkedDirty(opts.onlySelf);

(this as Writable<this>).status = DISABLED;
this._status.set(DISABLED);
(this as Writable<this>).errors = null;
this._forEachChild((control: AbstractControl) => {
/** We don't propagate the source control downwards */
Expand Down Expand Up @@ -1215,6 +1234,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
const skipPristineCheck = this._parentMarkedDirty(opts.onlySelf);

(this as Writable<this>).status = VALID;
this._status.set(VALID);
this._forEachChild((control: AbstractControl) => {
control.enable({...opts, onlySelf: true});
});
Expand Down Expand Up @@ -1302,6 +1322,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
this._cancelExistingSubscription();
(this as Writable<this>).errors = this._runValidator();
(this as Writable<this>).status = this._calculateStatus();
this._status.set(this.status);

if (this.status === VALID || this.status === PENDING) {
this._runAsyncValidator(opts.emitEvent);
Expand Down Expand Up @@ -1329,6 +1350,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T

private _setInitialStatus() {
(this as Writable<this>).status = this._allControlsDisabled() ? DISABLED : VALID;
this._status.set(this.status);
}

private _runValidator(): ValidationErrors | null {
Expand All @@ -1338,6 +1360,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
private _runAsyncValidator(emitEvent?: boolean): void {
if (this.asyncValidator) {
(this as Writable<this>).status = PENDING;
this._status.set(PENDING);
this._hasOwnPendingAsyncValidator = true;
const obs = toObservable(this.asyncValidator(this));
this._asyncValidationSubscription = obs.subscribe((errors: ValidationErrors | null) => {
Expand Down Expand Up @@ -1534,6 +1557,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
/** @internal */
_updateControlsErrors(emitEvent: boolean, changedControl: AbstractControl): void {
(this as Writable<this>).status = this._calculateStatus();
this._status.set(this.status);

if (emitEvent) {
(this.statusChanges as EventEmitter<FormControlStatus>).emit(this.status);
Expand Down Expand Up @@ -1594,6 +1618,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
const newPristine = !this._anyControlsDirty();
const changed = this.pristine !== newPristine;
(this as Writable<this>).pristine = newPristine;
this._pristine.set(newPristine);

if (this._parent && !opts.onlySelf) {
this._parent._updatePristine(opts, changedControl);
Expand All @@ -1607,6 +1632,7 @@ export abstract class AbstractControl<TValue = any, TRawValue extends TValue = T
/** @internal */
_updateTouched(opts: {onlySelf?: boolean} = {}, changedControl: AbstractControl): void {
(this as Writable<this>).touched = this._anyControlsTouched();
this._touched.set(this.touched);
this._events.next(new TouchedChangeEvent(this.touched, changedControl));

if (this._parent && !opts.onlySelf) {
Expand Down

0 comments on commit 4456905

Please sign in to comment.