Skip to content

fix(forms): Make NgControlStatus host bindings OnPush compatible - #55720

Closed
atscott wants to merge 2 commits into
angular:mainfrom
atscott:formshostbindingsonpush
Closed

atscott wants to merge 2 commits into
angular:mainfrom
atscott:formshostbindingsonpush

Conversation

@atscott

@atscott atscott commented May 7, 2024

Copy link
Copy Markdown
Contributor

This commit makes the host bindings of NgControlStatus[Group] compatible with OnPush components. Note that this intentionally does not expose any new APIs in the forms module. The goal is only to remove unpreventable ExpressionChangedAfterItHasBeenCheckedError in the forms code that developers do not have control over.

@atscott atscott added area: forms requires: TGP This PR requires a passing TGP before merging is allowed labels May 7, 2024
@ngbot ngbot Bot added this to the Backlog milestone May 7, 2024
@atscott atscott added the target: patch This PR is targeted for the next patch release label May 7, 2024
@atscott
atscott force-pushed the formshostbindingsonpush branch 4 times, most recently from 5e83ed8 to 90a620a Compare May 10, 2024 00:59
@atscott
atscott force-pushed the formshostbindingsonpush branch 5 times, most recently from dbb8263 to 2911043 Compare May 23, 2024 19:52
@atscott
atscott marked this pull request as ready for review May 23, 2024 19:52
@pullapprove
pullapprove Bot requested a review from AndrewKushnir May 23, 2024 19:52
@pullapprove pullapprove Bot removed the requires: TGP This PR requires a passing TGP before merging is allowed label May 23, 2024
@atscott
atscott requested a review from dylhunn May 23, 2024 20:24

@JeanMeche JeanMeche left a comment

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.

Fwiw, LGTM !

@AndrewKushnir
AndrewKushnir removed their request for review May 23, 2024 22:48
@atscott
atscott force-pushed the formshostbindingsonpush branch 2 times, most recently from a4d7c44 to 9afe197 Compare May 29, 2024 15:58

@dylhunn dylhunn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the PR that had issues when groups are passed into OnPush sibling components, right?

@thePunderWoman thePunderWoman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove
pullapprove Bot requested a review from AndrewKushnir May 30, 2024 13:59
@atscott

atscott commented May 30, 2024

Copy link
Copy Markdown
Contributor Author

This is the PR that had issues when groups are passed into OnPush sibling components, right?

Yes, that's right. It's maybe on the border of what might be considered a breaking change.

@AndrewKushnir AndrewKushnir left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@atscott
atscott force-pushed the formshostbindingsonpush branch 2 times, most recently from 8b65e7e to a87a3cb Compare June 6, 2024 16:48
@atscott atscott self-assigned this Jun 11, 2024
This commit makes the host bindings of `NgControlStatus[Group]`
compatible with `OnPush` components. Note that this intentionally _does not_
expose any new APIs in the forms module. The goal is only to remove
unpreventable `ExpressionChangedAfterItHasBeenCheckedError` in the forms
code that developers do not have control over.
@atscott

atscott commented Jun 18, 2024

Copy link
Copy Markdown
Contributor Author

@atscott
atscott force-pushed the formshostbindingsonpush branch from 91a35e9 to 237b6f2 Compare June 18, 2024 17:02
@AndrewKushnir AndrewKushnir 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: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 18, 2024
@AndrewKushnir

Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 00bde8b.

The changes were merged into the following branches: main

@angular-automatic-lock-bot

Copy link
Copy Markdown

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 Jul 19, 2024
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: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants