Skip to content

Commit ac2d0c6

Browse files
atscottalxhub
atscott
authored and
alxhub
committed
perf(core): Update LView consumer to only mark component for check (#52302)
This commit updates the reactive template and host binding consumers to only mark their declaration components for refresh, but not parents/ancestors. This also updates the `AfterViewChecked` hook to run when a component is refreshed during change detection but its host is not. It is reasonable to expect that the `ngAfterViewChecked` lifecycle hook will run when a signal updates and the component is refreshed. The hooks are typically run when the host is refreshed so without this change, the update to not mark ancestors dirty would have caused `ngAfterViewChecked` to not run. resolves #14628 resolves #22646 resolves #34347 - this is not the direct request of the issue but generally forcing change detection to run is necessary only because a value was updated that needs to be synced to the DOM. Values that use signals will mark the component for check automatically so accessing the `ChangeDetectorRef` of a child is not necessary. The other part of this request was to avoid the need to "mark all views for checking since it wouldn't affect anything but itself". This is directly addressed by this commit - updating a signal that's read in the view's template will not cause ancestors/"all views" to be refreshed. PR Close #52302
1 parent 8d52fa7 commit ac2d0c6

File tree

5 files changed

+573
-146
lines changed

5 files changed

+573
-146
lines changed

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {getComponentViewByInstance} from '../context_discovery';
1313
import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
1414
import {CONTAINER_HEADER_OFFSET, HAS_CHILD_VIEWS_TO_REFRESH, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container';
1515
import {ComponentTemplate, RenderFlags} from '../interfaces/definition';
16-
import {CONTEXT, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView} from '../interfaces/view';
16+
import {CONTEXT, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView, TViewType} from '../interfaces/view';
1717
import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state';
1818
import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils';
1919
import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
@@ -344,15 +344,22 @@ function detectChangesInViewIfAttached(lView: LView, mode: ChangeDetectionMode)
344344
* view HasChildViewsToRefresh flag is set.
345345
*/
346346
function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
347+
const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode();
347348
const tView = lView[TVIEW];
348349
const flags = lView[FLAGS];
349350

350351
// Flag cleared before change detection runs so that the view can be re-marked for traversal if
351352
// necessary.
352353
lView[FLAGS] &= ~(LViewFlags.HasChildViewsToRefresh | LViewFlags.RefreshView);
353354

354-
if ((flags & (LViewFlags.CheckAlways | LViewFlags.Dirty) &&
355-
mode === ChangeDetectionMode.Global) ||
355+
if ((flags & LViewFlags.CheckAlways && mode === ChangeDetectionMode.Global) ||
356+
(flags & LViewFlags.Dirty && mode === ChangeDetectionMode.Global &&
357+
// CheckNoChanges never worked with `OnPush` components because the `Dirty` flag was cleared
358+
// before checkNoChanges ran. Because there is now a loop for to check for backwards views,
359+
// it gives an opportunity for `OnPush` components to be marked `Dirty` before the
360+
// CheckNoChanges pass. We don't want existing errors that are hidden by the current
361+
// CheckNoChanges bug to surface when making unrelated changes.
362+
!isInCheckNoChangesPass) ||
356363
flags & LViewFlags.RefreshView) {
357364
refreshView(tView, lView, tView.template, lView[CONTEXT]);
358365
} else if (flags & LViewFlags.HasChildViewsToRefresh) {

‎packages/core/src/render3/reactive_lview_consumer.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,8 @@
88

99
import {REACTIVE_NODE, ReactiveNode} from '@angular/core/primitives/signals';
1010

11-
import {RuntimeError} from '../errors';
12-
import {assertDefined} from '../util/assert';
13-
14-
import {markViewDirty} from './instructions/mark_view_dirty';
1511
import {LView, REACTIVE_HOST_BINDING_CONSUMER, REACTIVE_TEMPLATE_CONSUMER} from './interfaces/view';
12+
import {markViewDirtyFromSignal} from './util/view_utils';
1613

1714
let currentConsumer: ReactiveLViewConsumer|null = null;
1815
export interface ReactiveLViewConsumer extends ReactiveNode {
@@ -43,7 +40,7 @@ const REACTIVE_LVIEW_CONSUMER_NODE: Omit<ReactiveLViewConsumer, 'lView'|'slot'>
4340
` in ExpressionChangedAfterItHasBeenChecked errors or future updates not working` +
4441
` entirely.`);
4542
}
46-
markViewDirty(node.lView);
43+
markViewDirtyFromSignal(node.lView);
4744
},
4845
consumerOnSignalRead(this: ReactiveLViewConsumer): void {
4946
if (currentConsumer !== this) {

‎packages/core/src/render3/util/view_utils.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {HAS_CHILD_VIEWS_TO_REFRESH, LContainer, TYPE} from '../interfaces/contai
1313
import {TConstants, TNode} from '../interfaces/node';
1414
import {RNode} from '../interfaces/renderer_dom';
1515
import {isLContainer, isLView} from '../interfaces/type_checks';
16-
import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view';
16+
import {DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view';
1717

1818

1919

@@ -241,6 +241,25 @@ export function markAncestorsForTraversal(lView: LView) {
241241
}
242242
}
243243

244+
/**
245+
* Marks the component or root view of an LView for refresh.
246+
*
247+
* This function locates the declaration component view of a given LView and marks it for refresh.
248+
* With this, we get component-level change detection granularity. Marking the `LView` itself for
249+
* refresh would be view-level granularity.
250+
*
251+
* Note that when an LView is a root view, the DECLARATION_COMPONENT_VIEW will be the root view
252+
* itself. This is a bit confusing since the TView.type is `Root`, rather than `Component`, but this
253+
* is actually what we need for host bindings in a root view.
254+
*/
255+
export function markViewDirtyFromSignal(lView: LView): void {
256+
const declarationComponentView = lView[DECLARATION_COMPONENT_VIEW];
257+
declarationComponentView[FLAGS] |= LViewFlags.RefreshView;
258+
if (viewAttachedToChangeDetector(declarationComponentView)) {
259+
markAncestorsForTraversal(declarationComponentView);
260+
}
261+
}
262+
244263
/**
245264
* Stores a LView-specific destroy callback.
246265
*/

0 commit comments

Comments
 (0)