fix(forms): ensure to emit `statusChanges` on subsequent value update… by sonukapoor · Pull Request #38354 · angular/angular

sonukapoor · 2020-08-05T22:42:58Z

… / validations

This commit ensures that the updateValueAndValidity method takes the
asyncValidator into consideration to emit on the statusChanges observables.
This is necessary so that any subsequent changes are emitted properly to any
subscribers.

BREAKING CHANGE:

Previously if FormControl, FormGroup and FormArray class instances had async validators
defined at initialization time, the status change event was not emitted once async validator
completed. After this change, the status event is emitted into the statusChanges observable.
If your code relies on the old behaviour, you can filter/ignore this additional status change
event.

Closes #20424
Closes #14542

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

Yes
No

Other information

AndrewKushnir

Thanks for adding tests @sonukapoor 👍

I've left a few comments (most of them are applicable to many tests), please have a look when you get a chance. Once the feedback is addressed, I will have another look.

Thank you.

packages/forms/test/form_group_spec.ts

sonukapoor · 2020-08-08T13:28:40Z

@AndrewKushnir All changes are done. The PR is ready for the second review.

AndrewKushnir · 2020-08-11T01:52:21Z

Initial Presubmit + Initial Global Presubmit.

AndrewKushnir

Hi @sonukapoor, thanks for additional changes. I've left a few more comments, please have a look when you get a chance. Thank you.

packages/forms/test/form_group_spec.ts

sonukapoor · 2020-08-12T12:41:13Z

@AndrewKushnir It's ready for another review. Thanks

AndrewKushnir

Thanks for updating tests @sonukapoor, great work 👍

I've a few more comments, they are mostly around comments/test descriptions, so I think we should move this PR to the "Ready for Review" state and perform a final round of review.

Thank you.

packages/forms/test/form_group_spec.ts

AndrewKushnir

Thanks for updating descriptions in tests, this is super helpful 👍

I've just left a comment on the last test, everything else looks great!

Since this is a change in behavior, could you please include a draft of the breaking changes note into the commit description (we can review and update it)?

packages/forms/test/form_group_spec.ts

AndrewKushnir

LGTM, thanks Sonu 👍

AndrewKushnir · 2020-08-27T04:17:25Z

Global Presubmit #2.

…/validations This commit ensures that the `updateValueAndValidity` method takes the `asyncValidator` into consideration to emit on the `statusChanges` observables. This is necessary so that any subsequent changes are emitted properly to any subscribers. Closes angular#20424 Closes angular#14542 BREAKING CHANGE: Previously if FormControl, FormGroup and FormArray class instances had async validators defined at initialization time, the status change event was not emitted once async validator completed. After this change the status event is emitted into the `statusChanges` observable. If your code relies on the old behavior, you can filter/ignore this additional status change event.

AndrewKushnir · 2020-08-28T00:03:08Z

Note to Caretaker: the global presubmit went well (only pre-existing failures), however since it's technically a breaking change, it might make sense to merge it and sync as a separate change (in case a rollback is needed). Thank you.

…/validations (angular#38354) This commit ensures that the `updateValueAndValidity` method takes the `asyncValidator` into consideration to emit on the `statusChanges` observables. This is necessary so that any subsequent changes are emitted properly to any subscribers. Closes angular#20424 Closes angular#14542 BREAKING CHANGE: Previously if FormControl, FormGroup and FormArray class instances had async validators defined at initialization time, the status change event was not emitted once async validator completed. After this change the status event is emitted into the `statusChanges` observable. If your code relies on the old behavior, you can filter/ignore this additional status change event. PR Close angular#38354

angular-automatic-lock-bot · 2020-10-09T16:26:06Z

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.}

googlebot added the cla: yes label Aug 5, 2020

sonukapoor requested a review from AndrewKushnir August 5, 2020 22:43

sonukapoor added area: forms forms: validators labels Aug 5, 2020

ngbot bot added this to the needsTriage milestone Aug 5, 2020

sonukapoor force-pushed the fix/emit-status-change-events branch from 0fdc615 to 8544dd9 Compare August 5, 2020 23:01

AndrewKushnir reviewed Aug 6, 2020

View reviewed changes

sonukapoor force-pushed the fix/emit-status-change-events branch from 5997d3b to a300076 Compare August 8, 2020 13:28

sonukapoor requested a review from AndrewKushnir August 10, 2020 21:24

AndrewKushnir reviewed Aug 12, 2020

View reviewed changes

sonukapoor force-pushed the fix/emit-status-change-events branch from 85f007d to 3a84683 Compare August 13, 2020 12:32

sonukapoor requested a review from AndrewKushnir August 13, 2020 12:49

sonukapoor marked this pull request as ready for review August 13, 2020 12:49

AndrewKushnir reviewed Aug 15, 2020

View reviewed changes

packages/forms/test/form_group_spec.ts Outdated Show resolved Hide resolved

AndrewKushnir added target: major This PR is targeted for the next major release breaking changes labels Aug 15, 2020

This comment has been minimized.

Sign in to view

AndrewKushnir approved these changes Aug 25, 2020

View reviewed changes

sonukapoor force-pushed the fix/emit-status-change-events branch 2 times, most recently from a245226 to ba5f4dc Compare August 25, 2020 23:14

AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Aug 27, 2020

sonukapoor force-pushed the fix/emit-status-change-events branch from ba5f4dc to 9d59b7d Compare August 27, 2020 10:14

AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Aug 28, 2020

AndrewKushnir mentioned this pull request Aug 28, 2020

fix(forms): ensure to emit status change events on subsequent value-update / validations #20806

Closed

3 tasks

AndrewKushnir added the risk: medium label Aug 28, 2020

josephperrott closed this in d9fea85 Sep 1, 2020

sonukapoor deleted the fix/emit-status-change-events branch September 8, 2020 18:50

angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 9, 2020

Conversation

sonukapoor commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonukapoor commented Aug 8, 2020

Uh oh!

AndrewKushnir commented Aug 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonukapoor commented Aug 12, 2020

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Aug 27, 2020

Uh oh!

AndrewKushnir commented Aug 28, 2020

Uh oh!

angular-automatic-lock-bot bot commented Oct 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sonukapoor commented Aug 5, 2020 •

edited

Loading

AndrewKushnir commented Aug 11, 2020 •

edited

Loading