From 2e27ca9ddfc1f3f0387cd720071e85ff46f19db6 Mon Sep 17 00:00:00 2001 From: Matthieu Riegler Date: Sat, 4 May 2024 16:08:39 +0200 Subject: [PATCH] fix(forms): Allow canceled async validators to emit. (#55134) With this change, If an async validator that should have emitted was cancelled by a non-emitting validator, the status change will be reported on the `AbstractControl.events` observable. This issue can happen when a `FormControl` is added to a `FormGroup` and a FormGroupDirective/FormControlDirective trigger a non-emitting validation (which cancels the initial validator execution). Note: The behavior remains the same of the existing `statusChanges` observable as the change was too breaking to land in G3. fixes: angular#41519 PR Close #55134 --- packages/forms/src/model/abstract_model.ts | 56 +++++++++++++++++----- packages/forms/test/form_group_spec.ts | 24 ++++++++++ 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/packages/forms/src/model/abstract_model.ts b/packages/forms/src/model/abstract_model.ts index 14c4a10226b..e9d37c678eb 100644 --- a/packages/forms/src/model/abstract_model.ts +++ b/packages/forms/src/model/abstract_model.ts @@ -479,10 +479,11 @@ export abstract class AbstractControl).errors = this._runValidator(); (this as Writable).status = this._calculateStatus(); if (this.status === VALID || this.status === PENDING) { - this._runAsyncValidator(opts.emitEvent); + // If the canceled subscription should have emitted + // we make sure the async validator emits the status change on completion + this._runAsyncValidator(shouldHaveEmitted, opts.emitEvent); } } @@ -1368,26 +1372,32 @@ export abstract class AbstractControl).status = PENDING; - this._hasOwnPendingAsyncValidator = true; + this._hasOwnPendingAsyncValidator = {emitEvent: emitEvent !== false}; const obs = toObservable(this.asyncValidator(this)); this._asyncValidationSubscription = obs.subscribe((errors: ValidationErrors | null) => { - this._hasOwnPendingAsyncValidator = false; + this._hasOwnPendingAsyncValidator = null; // This will trigger the recalculation of the validation status, which depends on // the state of the asynchronous validation (whether it is in progress or not). So, it is // necessary that we have updated the `_hasOwnPendingAsyncValidator` boolean flag first. - this.setErrors(errors, {emitEvent}); + this.setErrors(errors, {emitEvent, shouldHaveEmitted}); }); } } - private _cancelExistingSubscription(): void { + private _cancelExistingSubscription(): boolean { if (this._asyncValidationSubscription) { this._asyncValidationSubscription.unsubscribe(); - this._hasOwnPendingAsyncValidator = false; + + // we're cancelling the validator subscribtion, we keep if it should have emitted + // because we want to emit eventually if it was required at least once. + const shouldHaveEmitted = this._hasOwnPendingAsyncValidator?.emitEvent ?? false; + this._hasOwnPendingAsyncValidator = null; + return shouldHaveEmitted; } + return false; } /** @@ -1418,9 +1428,19 @@ export abstract class AbstractControl).errors = errors; - this._updateControlsErrors(opts.emitEvent !== false, this); + this._updateControlsErrors(opts.emitEvent !== false, this, opts.shouldHaveEmitted); } /** @@ -1565,16 +1585,26 @@ export abstract class AbstractControl).status = this._calculateStatus(); if (emitEvent) { (this.statusChanges as EventEmitter).emit(this.status); + } + + // The Events Observable expose a slight different bevahior than the statusChanges obs + // An async validator will still emit a StatusChangeEvent is a previously cancelled + // async validator has emitEvent set to true + if (emitEvent || shouldHaveEmitted) { this._events.next(new StatusChangeEvent(this.status, changedControl)); } if (this._parent) { - this._parent._updateControlsErrors(emitEvent, changedControl); + this._parent._updateControlsErrors(emitEvent, changedControl, shouldHaveEmitted); } } diff --git a/packages/forms/test/form_group_spec.ts b/packages/forms/test/form_group_spec.ts index f2533ad4508..89ef3352e08 100644 --- a/packages/forms/test/form_group_spec.ts +++ b/packages/forms/test/form_group_spec.ts @@ -9,11 +9,13 @@ import {fakeAsync, tick, waitForAsync} from '@angular/core/testing'; import { AbstractControl, + ControlEvent, FormArray, FormControl, FormGroup, ValidationErrors, Validators, + ValueChangeEvent, } from '@angular/forms'; import {of} from 'rxjs'; @@ -23,6 +25,7 @@ import { currentStateOf, simpleAsyncValidator, } from './util'; +import {StatusChangeEvent} from '../src/model/abstract_model'; (function () { function simpleValidator(c: AbstractControl): ValidationErrors | null { @@ -2218,6 +2221,27 @@ import { expect(logger).toEqual([]); })); + it('should cancel initial run of the async validator and emit on the event Observable', fakeAsync(() => { + const c = new FormControl('', null, simpleAsyncValidator({timeout: 1, shouldFail: true})); + + const events: ControlEvent[] = []; + c.events.subscribe((e) => events.push(e)); + + expect(events.length).toBe(0); + + c.setValue('new!'); + + tick(1); + + // validator was invoked twice (init + setValue) + // but since we cancel pending validators we only get 1 status update cycle + expect(events[0]).toBeInstanceOf(ValueChangeEvent); + expect(events[1]).toBeInstanceOf(StatusChangeEvent); + expect((events[1] as StatusChangeEvent).status).toBe('PENDING'); + expect(events[2]).toBeInstanceOf(StatusChangeEvent); + expect((events[2] as StatusChangeEvent).status).toBe('INVALID'); + })); + it('should run the sync validator on stand alone controls and set status to `INVALID`', fakeAsync(() => { const logs: string[] = []; const c = new FormControl('new!', Validators.required);