Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions packages/forms/src/directives/ng_form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ import {
forwardRef,
Inject,
Input,
OnDestroy,
Optional,
Provider,
Self,
signal,
untracked,
ɵWritable as Writable,
} from '@angular/core';
import {of, Subject, Subscription} from 'rxjs';
import {filter, map, switchMap, take} from 'rxjs/operators';

import {AbstractControl, FormHooks, FormSubmittedEvent} from '../model/abstract_model';
import {FormControl} from '../model/form_control';
Expand Down Expand Up @@ -124,7 +127,7 @@ const resolvedPromise = (() => Promise.resolve())();
exportAs: 'ngForm',
standalone: false,
})
export class NgForm extends ControlContainer implements Form, AfterViewInit {
export class NgForm extends ControlContainer implements Form, AfterViewInit, OnDestroy {
/**
* @description
* Returns whether the form submission has been triggered.
Expand Down Expand Up @@ -161,6 +164,23 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
*/
@Input('ngFormOptions') options!: {updateOn?: FormHooks};

/**
* @description
* When set to `true`, the `ngSubmit` event will not emit until all pending async validators
* on the form have completed. Defaults to `false` to preserve existing behavior.
*
* @usageNotes
* ```html
* <form (ngSubmit)="onSubmit()" awaitAsyncValidators>
* ...
* </form>
* ```
*/
@Input() awaitAsyncValidators: boolean = false;

private readonly _pendingSubmit$ = new Subject<Event>();
private _pendingSubmitSub!: Subscription;

constructor(
@Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator | ValidatorFn)[],
@Optional()
Expand All @@ -177,6 +197,26 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
composeValidators(validators),
composeAsyncValidators(asyncValidators),
);
this._pendingSubmitSub = this._pendingSubmit$
.pipe(
switchMap((event) =>
this.awaitAsyncValidators && this.form.status === 'PENDING'
? (() => {
const statusChanges$ = this.form.statusChanges.pipe(
filter((status) => status !== 'PENDING'),
take(1),
map(() => event),
);
this.form._updateTreeValidity({emitEvent: true});
return statusChanges$;
})()
: of(event),
),
)
.subscribe((event) => {
this.ngSubmit.emit(event);
this.form._events.next(new FormSubmittedEvent(this.control));
});
}

/** @docs-private */
Expand Down Expand Up @@ -334,13 +374,20 @@ export class NgForm extends ControlContainer implements Form, AfterViewInit {
onSubmit($event: Event): boolean {
this.submittedReactive.set(true);
syncPendingControls(this.form, this._directives);
this.ngSubmit.emit($event);
this.form._events.next(new FormSubmittedEvent(this.control));

this._pendingSubmit$.next($event);

// Forms with `method="dialog"` have some special behavior
// that won't reload the page and that shouldn't be prevented.
return ($event?.target as HTMLFormElement | null)?.method === 'dialog';
Comment on lines 375 to 382
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't much better.
We're plugging an async API in a sync API. It will return before the async validators will have run and this is probably not what users would expect.

I don't think there is a trivial evolution here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before pushing another update — the approach I'm considering is removing the constructor-based Subject pipeline entirely and instead handling the deferred emit directly inside onSubmit() with an inline statusChanges subscription (only when awaitAsyncValidators=true and the form is PENDING; default path stays fully synchronous). Does that direction address your concern, or did you have a different approach in mind?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After further looking at it, I see the issue goes deeper than just where the async work lives — changing ngSubmit semantics makes a synchronous submit API behave asynchronously, which is probably not what users would expect.

I'll hold off on pushing more changes. Would the Angular team prefer this handled as documentation/workaround guidance, or would a separate explicit async submission API be worth discussing? @JeanMeche

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of changing ngSubmit semantics, would a separate explicit event be more appropriate?

For example, ngSubmitValid could leave ngSubmit fully synchronous and emit only after pending async validators settle and the form is VALID. It would not emit if the form resolves to INVALID.

If that API direction is acceptable, I can update the PR with implementation, docs, and integration tests for both reactive and template-driven forms. @JeanMeche @SkyZeroZx

}

/** @nodoc */
ngOnDestroy(): void {
this._pendingSubmit$.complete();
this._pendingSubmitSub.unsubscribe();
}

/**
* @description
* Method called when the "reset" event is triggered on the form.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Directive,
EventEmitter,
Inject,
Input,
OnChanges,
OnDestroy,
Optional,
Expand All @@ -19,6 +20,8 @@ import {
signal,
untracked,
} from '@angular/core';
import {of, Subject, Subscription} from 'rxjs';
import {filter, map, switchMap, take} from 'rxjs/operators';

import {FormGroup} from '../../model/form_group';
import {FormArray} from '../../model/form_array';
Expand Down Expand Up @@ -102,6 +105,23 @@ export abstract class AbstractFormDirective
*/
abstract ngSubmit: EventEmitter<any>;

/**
* @description
* When set to `true`, the `ngSubmit` event will not emit until all pending async validators
* on the form have completed. Defaults to `false` to preserve existing behavior.
*
* @usageNotes
* ```html
* <form [formGroup]="myForm" (ngSubmit)="onSubmit()" awaitAsyncValidators>
* ...
* </form>
* ```
*/
@Input() awaitAsyncValidators: boolean = false;

private readonly _pendingSubmit$ = new Subject<Event>();
private _pendingSubmitSub!: Subscription;

constructor(
@Optional() @Self() @Inject(NG_VALIDATORS) validators: (Validator | ValidatorFn)[],
@Optional()
Expand All @@ -115,6 +135,26 @@ export abstract class AbstractFormDirective
super();
this._setValidators(validators);
this._setAsyncValidators(asyncValidators);
this._pendingSubmitSub = this._pendingSubmit$
.pipe(
switchMap((event) =>
this.awaitAsyncValidators && this.form.status === 'PENDING'
? (() => {
const statusChanges$ = this.form.statusChanges.pipe(
filter((status) => status !== 'PENDING'),
take(1),
map(() => event),
);
this.form._updateTreeValidity({emitEvent: true});
return statusChanges$;
})()
: of(event),
),
)
.subscribe((event) => {
this.ngSubmit.emit(event);
this.form._events.next(new FormSubmittedEvent(this.control));
});
}

/** @nodoc */
Expand All @@ -140,6 +180,8 @@ export abstract class AbstractFormDirective

/** @nodoc */
protected onDestroy() {
this._pendingSubmit$.complete();
this._pendingSubmitSub.unsubscribe();
if (this.form) {
cleanUpValidators(this.form, this);

Expand Down Expand Up @@ -312,8 +354,8 @@ export abstract class AbstractFormDirective
onSubmit($event: Event): boolean {
(this as {submitted: boolean}).submitted = true;
syncPendingControls(this.form, this.directives);
this.ngSubmit.emit($event);
this.form._events.next(new FormSubmittedEvent(this.control));

this._pendingSubmit$.next($event);

// Forms with `method="dialog"` have some special behavior that won't reload the page and that
// shouldn't be prevented. Note that we need to null check the `event` and the `target`, because
Expand Down
92 changes: 91 additions & 1 deletion packages/forms/test/reactive_integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
} from '@angular/private/testing';
import {expect} from '@angular/private/testing/matchers';
import {merge, NEVER, Observable, of, Subject, Subscription, timer} from 'rxjs';
import {map, tap} from 'rxjs/operators';
import {map, take, tap} from 'rxjs/operators';
import {
AbstractControl,
AsyncValidator,
Expand Down Expand Up @@ -1108,6 +1108,96 @@ describe('reactive forms integration tests', () => {
form.reset();
expect(loginEl.value).toBe('');
});

describe('awaitAsyncValidators', () => {
let pendingSubject: Subject<null>;

beforeEach(() => {
pendingSubject = new Subject<null>();
});

it('should emit ngSubmit immediately when awaitAsyncValidators is false (default)', () => {
const asyncValidator = () => pendingSubject.asObservable().pipe(take(1));
const fixture = initTest(FormGroupComp);
fixture.componentInstance.form = new FormGroup({
'login': new FormControl('', null, asyncValidator),
});
fixture.detectChanges();

const formGroupDir = fixture.debugElement.children[0].injector.get(FormGroupDirective);
const submitted: Event[] = [];
formGroupDir.ngSubmit.subscribe((e: Event) => submitted.push(e));

expect(fixture.componentInstance.form.status).toBe('PENDING');

dispatchEvent(fixture.debugElement.query(By.css('form')).nativeElement, 'submit');
fixture.detectChanges();

expect(submitted.length)
.withContext('Expected ngSubmit to fire immediately when awaitAsyncValidators is false')
.toBe(1);
});

it('should defer ngSubmit until async validators complete when awaitAsyncValidators is true', async () => {
const asyncValidator = () => pendingSubject.asObservable().pipe(take(1));
const fixture = initTest(FormGroupComp);
fixture.componentInstance.form = new FormGroup({
'login': new FormControl('', null, asyncValidator),
});
fixture.detectChanges();

const formGroupDir = fixture.debugElement.children[0].injector.get(FormGroupDirective);
formGroupDir.awaitAsyncValidators = true;

const submitted: Event[] = [];
formGroupDir.ngSubmit.subscribe((e: Event) => submitted.push(e));

expect(fixture.componentInstance.form.status).toBe('PENDING');

dispatchEvent(fixture.debugElement.query(By.css('form')).nativeElement, 'submit');
fixture.detectChanges();

expect(submitted.length)
.withContext('Expected ngSubmit not to fire while form is PENDING')
.toBe(0);

pendingSubject.next(null);
await timeout();
fixture.detectChanges();

expect(submitted.length)
.withContext('Expected ngSubmit to fire after async validators complete')
.toBe(1);
});

it('should emit ngSubmit immediately when awaitAsyncValidators is true but form is not PENDING', async () => {
const asyncValidator = () => pendingSubject.asObservable().pipe(take(1));
const fixture = initTest(FormGroupComp);
fixture.componentInstance.form = new FormGroup({
'login': new FormControl('', null, asyncValidator),
});
fixture.detectChanges();

const formGroupDir = fixture.debugElement.children[0].injector.get(FormGroupDirective);
formGroupDir.awaitAsyncValidators = true;

pendingSubject.next(null);
await timeout();
fixture.detectChanges();

expect(fixture.componentInstance.form.status).not.toBe('PENDING');

const submitted: Event[] = [];
formGroupDir.ngSubmit.subscribe((e: Event) => submitted.push(e));

dispatchEvent(fixture.debugElement.query(By.css('form')).nativeElement, 'submit');
fixture.detectChanges();

expect(submitted.length)
.withContext('Expected ngSubmit to fire immediately when form is not PENDING')
.toBe(1);
});
});
});

describe('value changes and status changes', () => {
Expand Down
Loading
Loading