Use consumer dirtiness to drive whether a view should refresh by atscott · Pull Request #52476 · angular/angular

atscott · 2023-10-31T22:19:45Z

First commit is from #52475

commit 2:

refactor(core): Update LView consumer to use only 1 consumer for a component

This commit updates the reactive consumer used for LViews to be shared
between a component and its embedded views. This allows us to use the
consumer flag directly for a dirty indicator rather than needing to
find a component view for updating its flags.

In the future, this will also allow us to effectively poll producers to see if
they really changed before refreshing a view.

commit 3

refactor(core): Do not refresh view if producers did not actually change

Producers represent values which can deliver change notifications.
When a producer value is changed, a change notification is propagated through the graph,
notifying live consumers which depend on the producer of the potential update.
Note here that this is a potential update.

A producer may not have actually "changed" based on its equality function. With
this commit, before refreshing a view that is only marked for refresh
because its consumer is dirty, we poll producers for change to see if
they really have. If not, we can skip the refresh. The example test in this commit
shows that a computed which depends on a signal that is updated but
produces a value that is the same as before will not cause the
component's template to refresh.

fixes #51797

packages/core/src/render3/instructions/change_detection.ts

…tion The significance of the combination of angular#51854 and angular#52302 went mostly unnoticed. The first removed a unidirectional data flow constraint for transplanted views and the second updated the signal implementation to share transplanted view logic. The result is that we automatically get behavior that (mostly) removes `ExpressionChangedAfterItWasCheckedError` when signals are used to drive application state to DOM synchronization. fixes angular#50320

…mponent This commit updates the reactive consumer used for `LView`s to be shared between a component and its embedded views. This allows us to use the consumer flag directly for a dirty indicator rather than needing to find a component view for updating its flags. In the future, this will also allow us to effectively poll producers to see if they really changed before refreshing a view.

Producers represent values which can deliver change notifications. When a producer value is changed, a change notification is propagated through the graph, notifying live consumers which depend on the producer of the potential update. Note here that this is a _potential_ update. A producer may not have actually "changed" based on its equality function. With this commit, before refreshing a view that is only marked for refresh because its consumer is dirty, we poll producers for change to see if they really have. If not, we can skip the refresh. The example test in this commit shows that a `computed` which depends on a `signal` that is updated but produces a value that is the same as before will _not_ cause the component's template to refresh. fixes angular#51797

alxhub

Reviewed-for: global-approvers

atscott · 2023-11-02T20:12:35Z

caretaker note: presubmits green other than one test failing at HEAD already.

alxhub · 2023-11-02T20:23:52Z

This PR was merged into the repository by commit 83a3b85.

…mponent (#52476) This commit updates the reactive consumer used for `LView`s to be shared between a component and its embedded views. This allows us to use the consumer flag directly for a dirty indicator rather than needing to find a component view for updating its flags. In the future, this will also allow us to effectively poll producers to see if they really changed before refreshing a view. PR Close #52476

…nge (#52476) Producers represent values which can deliver change notifications. When a producer value is changed, a change notification is propagated through the graph, notifying live consumers which depend on the producer of the potential update. Note here that this is a _potential_ update. A producer may not have actually "changed" based on its equality function. With this commit, before refreshing a view that is only marked for refresh because its consumer is dirty, we poll producers for change to see if they really have. If not, we can skip the refresh. The example test in this commit shows that a `computed` which depends on a `signal` that is updated but produces a value that is the same as before will _not_ cause the component's template to refresh. fixes #51797 PR Close #52476

…tion (#52476) The significance of the combination of #51854 and #52302 went mostly unnoticed. The first removed a unidirectional data flow constraint for transplanted views and the second updated the signal implementation to share transplanted view logic. The result is that we automatically get behavior that (mostly) removes `ExpressionChangedAfterItWasCheckedError` when signals are used to drive application state to DOM synchronization. fixes #50320 PR Close #52476

…mponent (#52476) This commit updates the reactive consumer used for `LView`s to be shared between a component and its embedded views. This allows us to use the consumer flag directly for a dirty indicator rather than needing to find a component view for updating its flags. In the future, this will also allow us to effectively poll producers to see if they really changed before refreshing a view. PR Close #52476

…nge (#52476) Producers represent values which can deliver change notifications. When a producer value is changed, a change notification is propagated through the graph, notifying live consumers which depend on the producer of the potential update. Note here that this is a _potential_ update. A producer may not have actually "changed" based on its equality function. With this commit, before refreshing a view that is only marked for refresh because its consumer is dirty, we poll producers for change to see if they really have. If not, we can skip the refresh. The example test in this commit shows that a `computed` which depends on a `signal` that is updated but produces a value that is the same as before will _not_ cause the component's template to refresh. fixes #51797 PR Close #52476

…tion (angular#52476) The significance of the combination of angular#51854 and angular#52302 went mostly unnoticed. The first removed a unidirectional data flow constraint for transplanted views and the second updated the signal implementation to share transplanted view logic. The result is that we automatically get behavior that (mostly) removes `ExpressionChangedAfterItWasCheckedError` when signals are used to drive application state to DOM synchronization. fixes angular#50320 PR Close angular#52476

…mponent (angular#52476) This commit updates the reactive consumer used for `LView`s to be shared between a component and its embedded views. This allows us to use the consumer flag directly for a dirty indicator rather than needing to find a component view for updating its flags. In the future, this will also allow us to effectively poll producers to see if they really changed before refreshing a view. PR Close angular#52476

…nge (angular#52476) Producers represent values which can deliver change notifications. When a producer value is changed, a change notification is propagated through the graph, notifying live consumers which depend on the producer of the potential update. Note here that this is a _potential_ update. A producer may not have actually "changed" based on its equality function. With this commit, before refreshing a view that is only marked for refresh because its consumer is dirty, we poll producers for change to see if they really have. If not, we can skip the refresh. The example test in this commit shows that a `computed` which depends on a `signal` that is updated but produces a value that is the same as before will _not_ cause the component's template to refresh. fixes angular#51797 PR Close angular#52476

angular-automatic-lock-bot · 2023-12-03T00:09:55Z

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

…tion (angular#52476) The significance of the combination of angular#51854 and angular#52302 went mostly unnoticed. The first removed a unidirectional data flow constraint for transplanted views and the second updated the signal implementation to share transplanted view logic. The result is that we automatically get behavior that (mostly) removes `ExpressionChangedAfterItWasCheckedError` when signals are used to drive application state to DOM synchronization. fixes angular#50320 PR Close angular#52476

…mponent (angular#52476) This commit updates the reactive consumer used for `LView`s to be shared between a component and its embedded views. This allows us to use the consumer flag directly for a dirty indicator rather than needing to find a component view for updating its flags. In the future, this will also allow us to effectively poll producers to see if they really changed before refreshing a view. PR Close angular#52476

…nge (angular#52476) Producers represent values which can deliver change notifications. When a producer value is changed, a change notification is propagated through the graph, notifying live consumers which depend on the producer of the potential update. Note here that this is a _potential_ update. A producer may not have actually "changed" based on its equality function. With this commit, before refreshing a view that is only marked for refresh because its consumer is dirty, we poll producers for change to see if they really have. If not, we can skip the refresh. The example test in this commit shows that a `computed` which depends on a `signal` that is updated but produces a value that is the same as before will _not_ cause the component's template to refresh. fixes angular#51797 PR Close angular#52476

atscott added the target: rc This PR is targeted for the next release-candidate label Oct 31, 2023

atscott requested a review from alxhub October 31, 2023 22:25

alxhub approved these changes Oct 31, 2023

View reviewed changes

packages/core/src/render3/instructions/change_detection.ts Outdated Show resolved Hide resolved

atscott force-pushed the workingCombineConsumers branch 4 times, most recently from 13f9a67 to 7ba4481 Compare November 1, 2023 02:40

pullapprove bot requested review from AndrewKushnir, alxhub and jessicajaniuk November 1, 2023 02:40

atscott added 3 commits November 2, 2023 11:27

alxhub approved these changes Nov 2, 2023

View reviewed changes

atscott force-pushed the workingCombineConsumers branch from 7ba4481 to 7bd221f Compare November 2, 2023 18:44

pullapprove bot requested a review from alxhub November 2, 2023 18:44

alxhub approved these changes Nov 2, 2023

View reviewed changes

atscott mentioned this pull request Nov 2, 2023

test(core): Ensure signals can be read after view creation during cha… #52495

Closed

atscott 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 labels Nov 2, 2023

alxhub closed this in 58d74a2 Nov 2, 2023

angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use consumer dirtiness to drive whether a view should refresh#52476

Use consumer dirtiness to drive whether a view should refresh#52476
atscott wants to merge 3 commits intoangular:mainfrom
atscott:workingCombineConsumers

atscott commented Oct 31, 2023 •

edited

Loading

Uh oh!

Uh oh!

alxhub left a comment

Uh oh!

atscott commented Nov 2, 2023

Uh oh!

alxhub commented Nov 2, 2023

Uh oh!

angular-automatic-lock-bot bot commented Dec 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

atscott commented Oct 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

commit 2:

commit 3

Uh oh!

Uh oh!

alxhub left a comment

Choose a reason for hiding this comment

Uh oh!

atscott commented Nov 2, 2023

Uh oh!

alxhub commented Nov 2, 2023

Uh oh!

angular-automatic-lock-bot bot commented Dec 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

atscott commented Oct 31, 2023 •

edited

Loading