Skip to content

Commit

Permalink
refactor(core): Update CD traversal to use 'modes' (angular#50005)
Browse files Browse the repository at this point in the history 
Rather than maintaining separate traversal functions that act differently, this change
updates the change detection traversal to share more code and use different modes
to control the type of traversal being performed.

PR Close angular#50005
  • Loading branch information
@atscott @alxhub
atscott authored and alxhub committed May 8, 2023
1 parent a648e30 commit 3112352
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 134 deletions.
122 changes: 69 additions & 53 deletions packages/core/src/render3/instructions/change_detection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,37 @@ export function detectChanges(component: {}): void {
detectChangesInternal(view[TVIEW], view, component);
}

/**
* Different modes of traversing the logical view tree during change detection.
*
*
* The change detection traversal algorithm switches between these modes based on various
* conditions.
*/
const enum ChangeDetectionMode {
/**
* In `Global` mode, `Dirty` and `CheckAlways` views are refreshed as well as views with the
* `RefreshTransplantedView` flag.
*/
Global,
/**
* In `Targeted` mode, only views with the `RefreshTransplantedView`
* flag are refreshed.
*/
Targeted,
/**
* Used when refreshing a view to force a refresh of its embedded views. This mode
* refreshes views without taking into account their LView flags, i.e. non-dirty OnPush components
* will be refreshed in this mode.
*
* TODO: we should work to remove this mode. It's used in `refreshView` because that's how the
* code worked before introducing ChangeDetectionMode. Instead, it should pass `Global` to the
* `detectChangesInEmbeddedViews`. We should aim to fix this by v17 or, at the very least, prevent
* this flag from affecting signal views not specifically marked for refresh (currently, this flag
* would _also_ force signal views to be refreshed).
*/
BugToForceRefreshAndIgnoreViewFlags
}

/**
* Processes a view in update mode. This includes a number of steps in a specific order:
Expand Down Expand Up @@ -122,7 +153,7 @@ export function refreshView<T>(
// insertion points. This is needed to avoid the situation where the template is defined in this
// `LView` but its declaration appears after the insertion component.
markTransplantedViewsForRefresh(lView);
refreshEmbeddedViews(lView);
detectChangesInEmbeddedViews(lView, ChangeDetectionMode.BugToForceRefreshAndIgnoreViewFlags);

// Content query results must be refreshed before content hooks are called.
if (tView.contentQueries !== null) {
Expand Down Expand Up @@ -152,7 +183,7 @@ export function refreshView<T>(
// Refresh child component views.
const components = tView.components;
if (components !== null) {
refreshChildComponents(lView, components);
detectChangesInChildComponents(lView, components, ChangeDetectionMode.Global);
}

// View queries must execute after refreshing child components because a template in this view
Expand Down Expand Up @@ -208,16 +239,12 @@ export function refreshView<T>(
* Goes over embedded views (ones created through ViewContainerRef APIs) and refreshes
* them by executing an associated template function.
*/
export function refreshEmbeddedViews(lView: LView) {
function detectChangesInEmbeddedViews(lView: LView, mode: ChangeDetectionMode) {
for (let lContainer = getFirstLContainer(lView); lContainer !== null;
lContainer = getNextLContainer(lContainer)) {
for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
const embeddedLView = lContainer[i];
const embeddedTView = embeddedLView[TVIEW];
ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
if (viewAttachedToChangeDetector(embeddedLView)) {
refreshView(embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!);
}
detectChangesInView(embeddedLView, mode);
}
}
}
Expand All @@ -227,7 +254,7 @@ export function refreshEmbeddedViews(lView: LView) {
*
* @param lView The `LView` that may have transplanted views.
*/
export function markTransplantedViewsForRefresh(lView: LView) {
function markTransplantedViewsForRefresh(lView: LView) {
for (let lContainer = getFirstLContainer(lView); lContainer !== null;
lContainer = getNextLContainer(lContainer)) {
if (!lContainer[HAS_TRANSPLANTED_VIEWS]) continue;
Expand All @@ -244,68 +271,57 @@ export function markTransplantedViewsForRefresh(lView: LView) {
}

/**
* Refreshes components by entering the component view and processing its bindings, queries, etc.
* Detects changes in a component by entering the component view and processing its bindings,
* queries, etc. if it is CheckAlways, OnPush and Dirty, etc.
*
* @param componentHostIdx Element index in LView[] (adjusted for HEADER_OFFSET)
*/
function refreshComponent(hostLView: LView, componentHostIdx: number): void {
function detectChangesInComponent(
hostLView: LView, componentHostIdx: number, mode: ChangeDetectionMode): void {
ngDevMode && assertEqual(isCreationMode(hostLView), false, 'Should be run in update mode');
const componentView = getComponentLViewByIndex(componentHostIdx, hostLView);
// Only attached components that are CheckAlways or OnPush and dirty should be refreshed
if (viewAttachedToChangeDetector(componentView)) {
const tView = componentView[TVIEW];
if (componentView[FLAGS] & (LViewFlags.CheckAlways | LViewFlags.Dirty)) {
refreshView(tView, componentView, tView.template, componentView[CONTEXT]);
} else if (componentView[DESCENDANT_VIEWS_TO_REFRESH] > 0) {
// Only attached components that are CheckAlways or OnPush and dirty should be refreshed
refreshContainsDirtyView(componentView);
}
}
detectChangesInView(componentView, mode);
}

/**
* Refreshes all transplanted views marked with `LViewFlags.RefreshTransplantedView` that are
* children or descendants of the given lView.
* Visits a view as part of change detection traversal.
*
* - If the view is detached, no additional traversal happens.
*
* The view is refreshed if:
* - If the view is CheckAlways or Dirty and ChangeDetectionMode is `Global`
* - If the view has the `RefreshTransplantedView` flag
*
* The view is not refreshed, but descendants are traversed in `ChangeDetectionMode.Targeted` if the
* view has a non-zero TRANSPLANTED_VIEWS_TO_REFRESH counter.
*
* @param lView The lView which contains descendant transplanted views that need to be refreshed.
*/
function refreshContainsDirtyView(lView: LView) {
for (let lContainer = getFirstLContainer(lView); lContainer !== null;
lContainer = getNextLContainer(lContainer)) {
for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
const embeddedLView = lContainer[i];
if (viewAttachedToChangeDetector(embeddedLView)) {
if (embeddedLView[FLAGS] & LViewFlags.RefreshView) {
const embeddedTView = embeddedLView[TVIEW];
ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
refreshView(
embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!);

} else if (embeddedLView[DESCENDANT_VIEWS_TO_REFRESH] > 0) {
refreshContainsDirtyView(embeddedLView);
}
}
}
function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
if (!viewAttachedToChangeDetector(lView)) {
return;
}

const tView = lView[TVIEW];
// Refresh child component views.
const components = tView.components;
if (components !== null) {
for (let i = 0; i < components.length; i++) {
const componentView = getComponentLViewByIndex(components[i], lView);
// Only attached components that are CheckAlways or OnPush and dirty should be refreshed
if (viewAttachedToChangeDetector(componentView) &&
componentView[DESCENDANT_VIEWS_TO_REFRESH] > 0) {
refreshContainsDirtyView(componentView);
}
if ((lView[FLAGS] & (LViewFlags.CheckAlways | LViewFlags.Dirty) &&
mode === ChangeDetectionMode.Global) ||
lView[FLAGS] & LViewFlags.RefreshView ||
mode === ChangeDetectionMode.BugToForceRefreshAndIgnoreViewFlags) {
refreshView(tView, lView, tView.template, lView[CONTEXT]);
} else if (lView[DESCENDANT_VIEWS_TO_REFRESH] > 0) {
detectChangesInEmbeddedViews(lView, ChangeDetectionMode.Targeted);

const tView = lView[TVIEW];
const components = tView.components;
if (components !== null) {
detectChangesInChildComponents(lView, components, ChangeDetectionMode.Targeted);
}
}
}

/** Refreshes child components in the current view (update mode). */
export function refreshChildComponents(hostLView: LView, components: number[]): void {
function detectChangesInChildComponents(
hostLView: LView, components: number[], mode: ChangeDetectionMode): void {
for (let i = 0; i < components.length; i++) {
refreshComponent(hostLView, components[i]);
detectChangesInComponent(hostLView, components[i], mode);
}
}
21 changes: 12 additions & 9 deletions packages/core/test/bundling/animations/bundle.golden_symbols.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,18 @@
{
"name": "detachMovedView"
},
{
"name": "detectChangesInChildComponents"
},
{
"name": "detectChangesInComponent"
},
{
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
},
{
"name": "detectChangesInternal"
},
Expand Down Expand Up @@ -1355,12 +1367,6 @@
{
"name": "refCount"
},
{
"name": "refreshComponent"
},
{
"name": "refreshContainsDirtyView"
},
{
"name": "refreshContentQueries"
},
Expand Down Expand Up @@ -1526,9 +1532,6 @@
{
"name": "urlParsingNode"
},
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "visitDslNode"
},
Expand Down
21 changes: 12 additions & 9 deletions packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,18 @@
{
"name": "detachMovedView"
},
{
"name": "detectChangesInChildComponents"
},
{
"name": "detectChangesInComponent"
},
{
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
},
{
"name": "detectChangesInternal"
},
Expand Down Expand Up @@ -1070,12 +1082,6 @@
{
"name": "refCount"
},
{
"name": "refreshComponent"
},
{
"name": "refreshContainsDirtyView"
},
{
"name": "refreshContentQueries"
},
Expand Down Expand Up @@ -1208,9 +1214,6 @@
{
"name": "urlParsingNode"
},
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"
},
Expand Down
21 changes: 12 additions & 9 deletions packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,18 @@
{
"name": "detachView"
},
{
"name": "detectChangesInChildComponents"
},
{
"name": "detectChangesInComponent"
},
{
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
},
{
"name": "detectChangesInternal"
},
Expand Down Expand Up @@ -1505,12 +1517,6 @@
{
"name": "refCount"
},
{
"name": "refreshComponent"
},
{
"name": "refreshContainsDirtyView"
},
{
"name": "refreshContentQueries"
},
Expand Down Expand Up @@ -1697,9 +1703,6 @@
{
"name": "urlParsingNode"
},
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"
},
Expand Down
21 changes: 12 additions & 9 deletions packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,18 @@
{
"name": "detachView"
},
{
"name": "detectChangesInChildComponents"
},
{
"name": "detectChangesInComponent"
},
{
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
},
{
"name": "detectChangesInternal"
},
Expand Down Expand Up @@ -1469,12 +1481,6 @@
{
"name": "refCount"
},
{
"name": "refreshComponent"
},
{
"name": "refreshContainsDirtyView"
},
{
"name": "refreshContentQueries"
},
Expand Down Expand Up @@ -1673,9 +1679,6 @@
{
"name": "urlParsingNode"
},
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"
},
Expand Down
21 changes: 12 additions & 9 deletions packages/core/test/bundling/hello_world/bundle.golden_symbols.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,18 @@
{
"name": "detachMovedView"
},
{
"name": "detectChangesInChildComponents"
},
{
"name": "detectChangesInComponent"
},
{
"name": "detectChangesInEmbeddedViews"
},
{
"name": "detectChangesInView"
},
{
"name": "detectChangesInternal"
},
Expand Down Expand Up @@ -839,12 +851,6 @@
{
"name": "refCount"
},
{
"name": "refreshComponent"
},
{
"name": "refreshContainsDirtyView"
},
{
"name": "refreshContentQueries"
},
Expand Down Expand Up @@ -953,9 +959,6 @@
{
"name": "urlParsingNode"
},
{
"name": "viewAttachedToChangeDetector"
},
{
"name": "walkProviderTree"
},
Expand Down
Loading

0 comments on commit 3112352

Please sign in to comment.