perf(core): Avoid traversal when markForCheck is called on a view being refreshed

[atscott]

This commit updates the internal markViewDirty logic to avoid marking views dirty up to the root when we encounter a view that is already being refreshed. This is wasteful because we clear the dirty bit at the end of refreshing a view anyways so it really has no affect on that view.

Sometimes we can reach a view and refresh it when the parents have been skipped. For example, this happens with views that have updated signals. Ancestors are only traversed and not refreshed. Marking dirty all the way up to the root in this case can result in those ancestor views being marked with the dirty flag and it not being cleared.

This only became problematic when we started to support backwards referenced transplanted views and changed signal change detection to not mark ancestors for check. In addition to that, the loop in change detection support to remove ExpressionChanged... errors for these cases can potentially cause infinite loops in rare cases when these views are refreshed at a time when parents have only been traversed and markForCheck is called. Prior to the loop, if this markForCheck was combined with an actual change to binding values, this would cause ExpressionChangedAfterItWasCheckedError.

For regular cases where we are refreshing a view, update a binding, and call markForCheck, this is either ExpressionChanged... in views with default change detection or no error at all (but without any state update) in views that are OnPush since the Dirty flag is cleared at the end of refreshView and checkNoChanges uses the same Dirty bit for traversal (#45612).

The only case where this might cause different and unexpected behavior is if change detection starts in the middle of a view tree via ChangeDetectorRef.detectChanges and a state update happens where the state is read in a view above the root of the traversal along with markForCheck. This might be the case for updating host bindings during change detection. That said, we have never really intented to support updating state during change detection, especially in a location above the current spot in the current view tree. This violates unidirectional data flow, a principle we have always intented to enforce outside of using signals for state.

[atscott]

TGP shows a few failures. Might want to consider a different approach. Converting back to draft.

[atscott]

Closing, at least for now. An adjustment has been applied to AsyncPipe to avoid the infinite loops. We’ll have to see if other cases come up.

[angular-automatic-lock-bot]

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