Fix/ngsubmit await async validators#68661
Conversation
| if (this.awaitAsyncValidators && this.form.status === 'PENDING') { | ||
| this._pendingSubmitSub?.unsubscribe(); | ||
| this._pendingSubmitSub = this.form.statusChanges | ||
| .pipe( | ||
| filter((status) => status !== 'PENDING'), | ||
| take(1), | ||
| ) | ||
| .subscribe(() => { | ||
| this.ngSubmit.emit($event); | ||
| this.form._events.next(new FormSubmittedEvent(this.control)); | ||
| }); |
There was a problem hiding this comment.
This is generaly not a recommended practice to hide async work in a sync function.
Here you wouldn't handle consecutive submits that would cancel the pending one (if you'd like to cancel it).
There was a problem hiding this comment.
Thanks for the feedback! Refactored to address both concerns:
- Async in sync function: onSubmit() is now fully synchronous — it just pushes to a Subject. The async pipeline lives in the constructor where its nature is explicit.
- Consecutive submits: Replaced manual unsubscribe() with switchMap, which makes the "cancel previous pending submit" behavior idiomatic and explicit.
Let me know if you'd prefer a different approach for the consecutive-submit semantics (e.g. blocking subsequent submits instead of cancelling).
|
I have read the CLA Document and I hereby sign the CLA. |
The commits were co-authored with On the other hand, I see the tests are failing; could you please review them as well |
…sync validation completes Adds an opt-in `awaitAsyncValidators` boolean input to `NgForm` and `FormGroupDirective`. When set to `true`, the submit handler defers `ngSubmit` emission until all async validators have settled (transitioned from PENDING to VALID or INVALID). onSubmit() remains synchronous — it pushes to a Subject. The async pipeline lives in the constructor where its nature is explicit. Consecutive submits are handled via switchMap, which cancels any pending deferred submit when a new one arrives. Fixes angular#31021
44d8dcc to
c3b6cfd
Compare
| 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'; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
14225af to
c3b6cfd
Compare
…nt:false
When addControl sets up async validators via updateValueAndValidity({emitEvent:false}),
the validator subscription captures emitEvent:false in its closure. This meant
statusChanges would not emit when the validator resolved, so the ngSubmit
deferral never fired.
Fix: before subscribing to statusChanges in the pending-submit pipeline,
call _updateTreeValidity({emitEvent:true}) to restart validators with
emitEvent:true so statusChanges properly emits when validation completes.
Also add take(1) to test validator observables so subscriptions complete
cleanly after each emission, preventing stale subscriptions on re-validation.
d40a230 to
84e45d2
Compare
PR Checklist
https://github.com/angular/angular/blob/main/contributing-docs/commit-message-guidelines.md
PR Type
What is the current behavior?
NgFormandFormGroupDirectiveemitngSubmitsynchronously on form submission even when the form has pending async validators(
status === 'PENDING'). Handlers that checkform.validreceive a false negative because async validation hasn't resolved yet.Issue Number: #31021
What is the new behavior?
Adds an opt-in
awaitAsyncValidatorsboolean input (defaultfalse) to bothNgFormandAbstractFormDirective. When enabled and theform is
PENDINGat submission time,ngSubmitis deferred untilstatusChangesemits a non-PENDINGvalue.Template-driven:
Does this PR introduce a breaking change?