fix(core): Prevent markForCheck during change detection from causin…
#54900
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atscott
force-pushed
the
refreshloop
branch
5 times, most recently
from
004f259
to
de04163
Compare
|
2 test-only failures. One pre-existing failure (with the fix already submitted). |
atscott
added
area: core
…g infinite loops This change updates the approach to the loop in `ApplicationRef.tick` for allowing state updates in `afterRender` hooks. It is valid to update state in render hooks and we need to ensure we refresh views that may be marked for check in these hooks (this can happen simply as a result of focusing an element). This change ensures that the behavior of `markForCheck` with respect to this loop does not change while we are actively running change detection on a view tree. This approach also has the benefit of preventing a regression for angular#18917, where updating state in animation listeners can cause `ExpressionChanged...Error` This should be allowed - there is nothing wrong with respect to unidirectional data flow in this case. There may be other cases in the future where it is valid to update state. Rather than wrapping the render hooks and the animation flushing in something which flips a global state flag, the idea here is that `markForCheck` is safe and valid in all cases whenever change detection is not actively running.
atscott
added
action: merge
|
caretaker note: green TGP (other than mentioned failures that have been/are being fixed) |
alxhub
approved these changes
Mar 18, 2024
|
This PR was merged into the repository by commit 314112d. |
ilirbeqirii
pushed a commit
to ilirbeqirii/angular
that referenced
this pull request
Apr 6, 2024
…g infinite loops (angular#54900) This change updates the approach to the loop in `ApplicationRef.tick` for allowing state updates in `afterRender` hooks. It is valid to update state in render hooks and we need to ensure we refresh views that may be marked for check in these hooks (this can happen simply as a result of focusing an element). This change ensures that the behavior of `markForCheck` with respect to this loop does not change while we are actively running change detection on a view tree. This approach also has the benefit of preventing a regression for angular#18917, where updating state in animation listeners can cause `ExpressionChanged...Error` This should be allowed - there is nothing wrong with respect to unidirectional data flow in this case. There may be other cases in the future where it is valid to update state. Rather than wrapping the render hooks and the animation flushing in something which flips a global state flag, the idea here is that `markForCheck` is safe and valid in all cases whenever change detection is not actively running. PR Close angular#54900
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…g infinite loops
This change updates the approach to the loop in
ApplicationRef.tickfor allowing state updates inafterRenderhooks. It is valid to update state in render hooks and we need to ensure we refresh views that may be marked for check in these hooks (this can happen simply as a result of focusing an element). This change ensures that the behavior ofmarkForCheckwith respect to this loop does not change while we are actively running change detection on a view tree.This approach also has the benefit of preventing a regression for #18917, where updating state in animation listeners can cause
ExpressionChanged...ErrorThis should be allowed - there is nothing wrong with respect to unidirectional data flow in this case.There may be other cases in the future where it is valid to update state. Rather than wrapping the render hooks and the animation flushing in something which flips a global state flag, the idea here is that
markForCheckis safe and valid in all cases whenever change detection is not actively running.