Skip to content

fix(forms): Allow canceled async validators to emit. #55134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Mar 30, 2024

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: #41519

@ngbot ngbot bot added this to the Backlog milestone Mar 30, 2024
@JeanMeche JeanMeche force-pushed the forms/emit-async-validator branch from 52881d0 to 530de42 Compare March 30, 2024 06:24
@JeanMeche JeanMeche marked this pull request as ready for review March 30, 2024 06:25
@pullapprove pullapprove bot requested a review from dylhunn March 30, 2024 06:25
@JeanMeche JeanMeche force-pushed the forms/emit-async-validator branch 6 times, most recently from dce644f to 51b700f Compare March 30, 2024 06:47
@crisbeto
Copy link
Member

TGP for future reference.

@dylhunn
Copy link
Contributor

dylhunn commented Apr 2, 2024

Matthieu and i spent some time yesterday looking at the failures, and they mostly originate with various teams using setTimeout trick to poll the status changes. We reached out to some of them to see if we can get them fixed, but if not, we will ship this enabled only for the new unified control state change API.

@dylhunn
Copy link
Contributor

dylhunn commented Apr 24, 2024

Talking to a possible owner of a failing test in g3, in order to get some help fixing the test.

@JeanMeche JeanMeche force-pushed the forms/emit-async-validator branch 2 times, most recently from 96fe757 to 5160e41 Compare April 24, 2024 07:30
@dylhunn dylhunn modified the milestones: Backlog, v18 final Apr 24, 2024
@JeanMeche
Copy link
Member Author

Since this change is a bit too breaking to land in G3, I'm looking at a solution that will only work with the new events observable.

@JeanMeche JeanMeche force-pushed the forms/emit-async-validator branch from 5160e41 to 3e1d02b Compare May 4, 2024 14:08
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
@JeanMeche JeanMeche force-pushed the forms/emit-async-validator branch from 3e1d02b to d61efeb Compare May 4, 2024 14:13
@JeanMeche JeanMeche changed the title fix(forms): Allow async validators to emit status change if a async v… fix(forms): Allow canceled async validators to emit. May 4, 2024
@dylhunn
Copy link
Contributor

dylhunn commented May 9, 2024

caretaker: this has a clean TGP (https://test.corp.google.com/OCL:630752820:BASE:630752881:1714884722829:6f76c7bc)

@dylhunn dylhunn added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: rc This PR is targeted for the next release-candidate labels May 9, 2024
@atscott
Copy link
Contributor

atscott commented May 15, 2024

This PR was merged into the repository by commit 53b0d6a.

@atscott atscott closed this in 53b0d6a May 15, 2024
atscott pushed a commit that referenced this pull request May 15, 2024
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: #41519

PR Close #55134
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 15, 2024
@JeanMeche JeanMeche deleted the forms/emit-async-validator branch July 23, 2024 09:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: forms merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Form statusChanges with async validators doesn't leave PENDING state
4 participants