Skip to content
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

fix(core): exhaustive checkNoChanges should only do a single pass #55839

Closed
wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented May 16, 2024

Because exhaustive checks traverse the whole tree regardless of the dirty state, it breaks some expectations around how change detection should be running. When a view has transplanted views, it unconditionally marks all ancestors for traversal, assuming this is fine because the loop will just traverse them and find nothing dirty. However, exhaustive checkNoChanages actually refreshes everything during traversal.

This update ensures the exhaustive check only does a single pass and also prevents some unnecessary marking of transplanted views for refresh since we know they're going to be reached.

Because exhaustive checks traverse the whole tree regardless of the
dirty state, it breaks some expectations around how change detection
should be running. When a view has transplanted views, it
unconditionally marks all ancestors for traversal, assuming this is fine
because the loop will just traverse them and find nothing dirty.
However, exhaustive checkNoChanages actually refreshes everything during
traversal.

This update ensures the exhaustive check only does a single pass and
also prevents some unnecessary marking of transplanted views for
refresh since we know they're going to be reached.
@atscott atscott added target: rc This PR is targeted for the next release-candidate area: core Issues related to the framework runtime labels May 16, 2024
@ngbot ngbot bot added this to the Backlog milestone May 16, 2024
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label May 16, 2024
@dylhunn
Copy link
Contributor

dylhunn commented May 17, 2024

This PR was merged into the repository by commit cae0d31.

@dylhunn dylhunn closed this in cae0d31 May 17, 2024
dylhunn pushed a commit that referenced this pull request May 17, 2024
…5839)

Because exhaustive checks traverse the whole tree regardless of the
dirty state, it breaks some expectations around how change detection
should be running. When a view has transplanted views, it
unconditionally marks all ancestors for traversal, assuming this is fine
because the loop will just traverse them and find nothing dirty.
However, exhaustive checkNoChanages actually refreshes everything during
traversal.

This update ensures the exhaustive check only does a single pass and
also prevents some unnecessary marking of transplanted views for
refresh since we know they're going to be reached.

PR Close #55839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants