feat(forms): Make Form Statuses use stricter types. (#42952)

Specifically: narrow the type used for form statuses from string to a union of possible statuses. Change the API methods from any to use the new type.

This is a breaking change. However, as discussed in the PR, breakage seems minimal, and google3 has been prepped to land this.

Background: we uncovered these any typings in the course of design work for typed forms. They could be fixed in a non-breaking manner by piggybacking them on top of the new typed forms generics, but it would be much cleaner to fix them separately if possible.

BREAKING CHANGE:

A new type called `FormControlStatus` has been introduced, which is a union of all possible status strings for form controls. `AbstractControl.status` has been narrowed from `string` to `FormControlStatus`, and `statusChanges` has been narrowed from `Observable<any>` to `Observable<FormControlStatus>`. Most applications should consume the new types seamlessly. Any breakage caused by this change is likely due to one of the following two problems: (1) the app is comparing `AbstractControl.status` against a string which is not a valid status; or, (2) the app is using `statusChanges` events as if they were something other than strings.

PR Close #42952
This commit is contained in:
Dylan Hunn 2021-07-23 14:35:08 -07:00 committed by Andrew Kushnir
parent 3107988e6e
commit e49fc96ed3
2 changed files with 38 additions and 24 deletions

View File

@ -78,8 +78,8 @@ export abstract class AbstractControl {
setParent(parent: FormGroup | FormArray): void; setParent(parent: FormGroup | FormArray): void;
setValidators(validators: ValidatorFn | ValidatorFn[] | null): void; setValidators(validators: ValidatorFn | ValidatorFn[] | null): void;
abstract setValue(value: any, options?: Object): void; abstract setValue(value: any, options?: Object): void;
readonly status: string; readonly status: FormControlStatus;
readonly statusChanges: Observable<any>; readonly statusChanges: Observable<FormControlStatus>;
readonly touched: boolean; readonly touched: boolean;
get untouched(): boolean; get untouched(): boolean;
get updateOn(): FormHooks; get updateOn(): FormHooks;

View File

@ -45,6 +45,23 @@ export const PENDING = 'PENDING';
*/ */
export const DISABLED = 'DISABLED'; export const DISABLED = 'DISABLED';
/**
* A form can have several different statuses. Each
* possible status is returned as a string literal.
*
* * **VALID**: Reports that a FormControl is valid, meaning that no errors exist in the input
* value.
* * **INVALID**: Reports that a FormControl is invalid, meaning that an error exists in the input
* value.
* * **PENDING**: Reports that a FormControl is pending, meaning that that async validation is
* occurring and errors are not yet available for the input value.
* * **DISABLED**: Reports that a FormControl is
* disabled, meaning that the control is exempt from ancestor calculations of validity or value.
*
* @publicApi
*/
export type FormControlStatus = 'VALID'|'INVALID'|'PENDING'|'DISABLED';
function _find(control: AbstractControl, path: Array<string|number>|string, delimiter: string) { function _find(control: AbstractControl, path: Array<string|number>|string, delimiter: string) {
if (path == null) return null; if (path == null) return null;
@ -274,19 +291,15 @@ export abstract class AbstractControl {
} }
/** /**
* The validation status of the control. There are four possible * The validation status of the control.
* validation status values:
* *
* * **VALID**: This control has passed all validation checks. * @see `FormControlStatus`
* * **INVALID**: This control has failed at least one validation check.
* * **PENDING**: This control is in the midst of conducting a validation check.
* * **DISABLED**: This control is exempt from validation checks.
* *
* These status values are mutually exclusive, so a control cannot be * These status values are mutually exclusive, so a control cannot be
* both valid AND invalid or invalid AND disabled. * both valid AND invalid or invalid AND disabled.
*/ */
// TODO(issue/24571): remove '!'. // TODO(issue/24571): remove '!'.
public readonly status!: string; public readonly status!: FormControlStatus;
/** /**
* A control is `valid` when its `status` is `VALID`. * A control is `valid` when its `status` is `VALID`.
@ -409,11 +422,12 @@ export abstract class AbstractControl {
* A multicasting observable that emits an event every time the validation `status` of the control * A multicasting observable that emits an event every time the validation `status` of the control
* recalculates. * recalculates.
* *
* @see `FormControlStatus`
* @see {@link AbstractControl.status} * @see {@link AbstractControl.status}
* *
*/ */
// TODO(issue/24571): remove '!'. // TODO(issue/24571): remove '!'.
public readonly statusChanges!: Observable<any>; public readonly statusChanges!: Observable<FormControlStatus>;
/** /**
* Reports the update strategy of the `AbstractControl` (meaning * Reports the update strategy of the `AbstractControl` (meaning
@ -687,10 +701,10 @@ export abstract class AbstractControl {
* *
*/ */
markAsPending(opts: {onlySelf?: boolean, emitEvent?: boolean} = {}): void { markAsPending(opts: {onlySelf?: boolean, emitEvent?: boolean} = {}): void {
(this as {status: string}).status = PENDING; (this as {status: FormControlStatus}).status = PENDING;
if (opts.emitEvent !== false) { if (opts.emitEvent !== false) {
(this.statusChanges as EventEmitter<any>).emit(this.status); (this.statusChanges as EventEmitter<FormControlStatus>).emit(this.status);
} }
if (this._parent && !opts.onlySelf) { if (this._parent && !opts.onlySelf) {
@ -720,7 +734,7 @@ export abstract class AbstractControl {
// parent's dirtiness based on the children. // parent's dirtiness based on the children.
const skipPristineCheck = this._parentMarkedDirty(opts.onlySelf); const skipPristineCheck = this._parentMarkedDirty(opts.onlySelf);
(this as {status: string}).status = DISABLED; (this as {status: FormControlStatus}).status = DISABLED;
(this as {errors: ValidationErrors | null}).errors = null; (this as {errors: ValidationErrors | null}).errors = null;
this._forEachChild((control: AbstractControl) => { this._forEachChild((control: AbstractControl) => {
control.disable({...opts, onlySelf: true}); control.disable({...opts, onlySelf: true});
@ -729,7 +743,7 @@ export abstract class AbstractControl {
if (opts.emitEvent !== false) { if (opts.emitEvent !== false) {
(this.valueChanges as EventEmitter<any>).emit(this.value); (this.valueChanges as EventEmitter<any>).emit(this.value);
(this.statusChanges as EventEmitter<string>).emit(this.status); (this.statusChanges as EventEmitter<FormControlStatus>).emit(this.status);
} }
this._updateAncestors({...opts, skipPristineCheck}); this._updateAncestors({...opts, skipPristineCheck});
@ -759,7 +773,7 @@ export abstract class AbstractControl {
// parent's dirtiness based on the children. // parent's dirtiness based on the children.
const skipPristineCheck = this._parentMarkedDirty(opts.onlySelf); const skipPristineCheck = this._parentMarkedDirty(opts.onlySelf);
(this as {status: string}).status = VALID; (this as {status: FormControlStatus}).status = VALID;
this._forEachChild((control: AbstractControl) => { this._forEachChild((control: AbstractControl) => {
control.enable({...opts, onlySelf: true}); control.enable({...opts, onlySelf: true});
}); });
@ -823,7 +837,7 @@ export abstract class AbstractControl {
if (this.enabled) { if (this.enabled) {
this._cancelExistingSubscription(); this._cancelExistingSubscription();
(this as {errors: ValidationErrors | null}).errors = this._runValidator(); (this as {errors: ValidationErrors | null}).errors = this._runValidator();
(this as {status: string}).status = this._calculateStatus(); (this as {status: FormControlStatus}).status = this._calculateStatus();
if (this.status === VALID || this.status === PENDING) { if (this.status === VALID || this.status === PENDING) {
this._runAsyncValidator(opts.emitEvent); this._runAsyncValidator(opts.emitEvent);
@ -832,7 +846,7 @@ export abstract class AbstractControl {
if (opts.emitEvent !== false) { if (opts.emitEvent !== false) {
(this.valueChanges as EventEmitter<any>).emit(this.value); (this.valueChanges as EventEmitter<any>).emit(this.value);
(this.statusChanges as EventEmitter<string>).emit(this.status); (this.statusChanges as EventEmitter<FormControlStatus>).emit(this.status);
} }
if (this._parent && !opts.onlySelf) { if (this._parent && !opts.onlySelf) {
@ -847,7 +861,7 @@ export abstract class AbstractControl {
} }
private _setInitialStatus() { private _setInitialStatus() {
(this as {status: string}).status = this._allControlsDisabled() ? DISABLED : VALID; (this as {status: FormControlStatus}).status = this._allControlsDisabled() ? DISABLED : VALID;
} }
private _runValidator(): ValidationErrors|null { private _runValidator(): ValidationErrors|null {
@ -856,7 +870,7 @@ export abstract class AbstractControl {
private _runAsyncValidator(emitEvent?: boolean): void { private _runAsyncValidator(emitEvent?: boolean): void {
if (this.asyncValidator) { if (this.asyncValidator) {
(this as {status: string}).status = PENDING; (this as {status: FormControlStatus}).status = PENDING;
this._hasOwnPendingAsyncValidator = true; this._hasOwnPendingAsyncValidator = true;
const obs = toObservable(this.asyncValidator(this)); const obs = toObservable(this.asyncValidator(this));
this._asyncValidationSubscription = obs.subscribe((errors: ValidationErrors|null) => { this._asyncValidationSubscription = obs.subscribe((errors: ValidationErrors|null) => {
@ -1017,10 +1031,10 @@ export abstract class AbstractControl {
/** @internal */ /** @internal */
_updateControlsErrors(emitEvent: boolean): void { _updateControlsErrors(emitEvent: boolean): void {
(this as {status: string}).status = this._calculateStatus(); (this as {status: FormControlStatus}).status = this._calculateStatus();
if (emitEvent) { if (emitEvent) {
(this.statusChanges as EventEmitter<string>).emit(this.status); (this.statusChanges as EventEmitter<FormControlStatus>).emit(this.status);
} }
if (this._parent) { if (this._parent) {
@ -1031,11 +1045,11 @@ export abstract class AbstractControl {
/** @internal */ /** @internal */
_initObservables() { _initObservables() {
(this as {valueChanges: Observable<any>}).valueChanges = new EventEmitter(); (this as {valueChanges: Observable<any>}).valueChanges = new EventEmitter();
(this as {statusChanges: Observable<any>}).statusChanges = new EventEmitter(); (this as {statusChanges: Observable<FormControlStatus>}).statusChanges = new EventEmitter();
} }
private _calculateStatus(): string { private _calculateStatus(): FormControlStatus {
if (this._allControlsDisabled()) return DISABLED; if (this._allControlsDisabled()) return DISABLED;
if (this.errors) return INVALID; if (this.errors) return INVALID;
if (this._hasOwnPendingAsyncValidator || this._anyControlsHaveStatus(PENDING)) return PENDING; if (this._hasOwnPendingAsyncValidator || this._anyControlsHaveStatus(PENDING)) return PENDING;
@ -1059,7 +1073,7 @@ export abstract class AbstractControl {
abstract _syncPendingControls(): boolean; abstract _syncPendingControls(): boolean;
/** @internal */ /** @internal */
_anyControlsHaveStatus(status: string): boolean { _anyControlsHaveStatus(status: FormControlStatus): boolean {
return this._anyControls((control: AbstractControl) => control.status === status); return this._anyControls((control: AbstractControl) => control.status === status);
} }