From 31123520ce912b70149a437a13be62e653766358 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 25 Apr 2023 15:02:34 -0700 Subject: [PATCH] refactor(core): Update CD traversal to use 'modes' (#50005) 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 #50005 --- .../render3/instructions/change_detection.ts | 122 ++++++++++-------- .../animations/bundle.golden_symbols.json | 21 +-- .../cyclic_import/bundle.golden_symbols.json | 21 +-- .../forms_reactive/bundle.golden_symbols.json | 21 +-- .../bundle.golden_symbols.json | 21 +-- .../hello_world/bundle.golden_symbols.json | 21 +-- .../hydration/bundle.golden_symbols.json | 21 +-- .../router/bundle.golden_symbols.json | 21 +-- .../bundle.golden_symbols.json | 21 +-- .../bundling/todo/bundle.golden_symbols.json | 21 +-- 10 files changed, 177 insertions(+), 134 deletions(-) diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts index f385a358bf385..5fafd2981b884 100644 --- a/packages/core/src/render3/instructions/change_detection.ts +++ b/packages/core/src/render3/instructions/change_detection.ts @@ -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: @@ -122,7 +153,7 @@ export function refreshView( // 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) { @@ -152,7 +183,7 @@ export function refreshView( // 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 @@ -208,16 +239,12 @@ export function refreshView( * 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); } } } @@ -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; @@ -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); } } diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json index bea88152e3121..c4b17509c374d 100644 --- a/packages/core/test/bundling/animations/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations/bundle.golden_symbols.json @@ -872,6 +872,18 @@ { "name": "detachMovedView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1355,12 +1367,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1526,9 +1532,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "visitDslNode" }, diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 5371eff4ee172..406eb344eb439 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -653,6 +653,18 @@ { "name": "detachMovedView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1070,12 +1082,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1208,9 +1214,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index 51b3066a326f1..da52e7ef6a102 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -893,6 +893,18 @@ { "name": "detachView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1505,12 +1517,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1697,9 +1703,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index bbf8af334c549..331404c773386 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -863,6 +863,18 @@ { "name": "detachView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1469,12 +1481,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1673,9 +1679,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index fd8d2d4aed6fd..1d84f806b864c 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -509,6 +509,18 @@ { "name": "detachMovedView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -839,12 +851,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -953,9 +959,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 7ca4dac32a10c..005d01ba8f9c0 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -719,6 +719,18 @@ { "name": "detachMovedView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1145,12 +1157,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1283,9 +1289,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index d73ca756efc62..a9428daf4ba67 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1133,6 +1133,18 @@ { "name": "detachView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1823,12 +1835,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -2054,9 +2060,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 7428c2a342197..573ee1952795e 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -587,6 +587,18 @@ { "name": "detachMovedView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -941,12 +953,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1061,9 +1067,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 92fd67e56a629..9d29d7a251810 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -779,6 +779,18 @@ { "name": "detachView" }, + { + "name": "detectChangesInChildComponents" + }, + { + "name": "detectChangesInComponent" + }, + { + "name": "detectChangesInEmbeddedViews" + }, + { + "name": "detectChangesInView" + }, { "name": "detectChangesInternal" }, @@ -1286,12 +1298,6 @@ { "name": "refCount" }, - { - "name": "refreshComponent" - }, - { - "name": "refreshContainsDirtyView" - }, { "name": "refreshContentQueries" }, @@ -1451,9 +1457,6 @@ { "name": "urlParsingNode" }, - { - "name": "viewAttachedToChangeDetector" - }, { "name": "walkProviderTree" },