fix(core): Avoid refreshing a host view twice when using transplanted…

… views

This change fixes and issue where the expectation was that change
detection always goes through `detectChangesInView`. In reality,
`detectChangesInternal` directly calls `refreshView`
and refreshes a view directly without checking if it was dirty (to my discontent).

This update clears the refresh flags at the top of `refreshView` as well
as before traversing child views in `Targeted` mode. The issue only
partially exists with signals in that the flags aren't cleared but we
don't end up refreshing the view a second time because the
`consumerPollProducersForChange` returns `false` the second time we
enter `detectChangesInView`.

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

@@ -18,7 +18,7 @@ import {CONTEXT, EFFECTS_TO_SCHEDULE, ENVIRONMENT, FLAGS, InitPhaseState, LView,
 import {getOrBorrowReactiveLViewConsumer, maybeReturnReactiveLViewConsumer, ReactiveLViewConsumer} from '../reactive_lview_consumer';
 import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state';
 import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils';
-import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, requiresRefreshOrTraversal, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
+import {clearRefreshAndTraversalFlags, getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, requiresRefreshOrTraversal, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
 
 import {executeTemplate, executeViewQueryFn, handleError, processHostBindingOpCodes, refreshContentQueries} from './shared';
 
@@ -129,6 +129,8 @@ const enum ChangeDetectionMode {
 export function refreshView<T>(
     tView: TView, lView: LView, templateFn: ComponentTemplate<{}>|null, context: T) {
   ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode');
+  // Remove refresh and traversal flags so that they can be re-dirtied during the refresh process.
+  clearRefreshAndTraversalFlags(lView);
   const flags = lView[FLAGS];
   if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
 
@@ -399,16 +401,11 @@ function detectChangesInView(lView: LView, mode: ChangeDetectionMode) {
   // Refresh views when they have a dirty reactive consumer, regardless of mode.
   shouldRefreshView ||= !!(consumer?.dirty && consumerPollProducersForChange(consumer));
 
-  // Mark the Flags and `ReactiveNode` as not dirty before refreshing the component, so that they
-  // can be re-dirtied during the refresh process.
-  if (consumer) {
-    consumer.dirty = false;
-  }
-  lView[FLAGS] &= ~(LViewFlags.HasChildViewsToRefresh | LViewFlags.RefreshView);
-
   if (shouldRefreshView) {
     refreshView(tView, lView, tView.template, lView[CONTEXT]);
   } else if (flags & LViewFlags.HasChildViewsToRefresh) {
+    // Remove refresh and traversal flags so that they can be re-dirtied during the refresh process.
+    clearRefreshAndTraversalFlags(lView);
     detectChangesInEmbeddedViews(lView, ChangeDetectionMode.Targeted);
     const components = tView.components;
     if (components !== null) {

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

@@ -213,6 +213,20 @@ export function updateAncestorTraversalFlagsOnAttach(lView: LView) {
   markAncestorsForTraversal(lView);
 }
 
+/**
+ * Clears flags related to view refresh and traversal (excluding `LViewFlags.Dirty`).
+ *
+ * This helper method is meant to be used before refreshing a view. Since `LViewFlags.Dirty` is not
+ * cleared until the end of view refresh, it is not cleared in this function.
+ */
+export function clearRefreshAndTraversalFlags(lView: LView) {
+  const consumer = lView[REACTIVE_TEMPLATE_CONSUMER];
+  if (consumer) {
+    consumer.dirty = false;
+  }
+  lView[FLAGS] &= ~(LViewFlags.HasChildViewsToRefresh | LViewFlags.RefreshView);
+}
+
 /**
  * Ensures views above the given `lView` are traversed during change detection even when they are
  * not dirty.

packages/core/test/acceptance/change_detection_transplanted_view_spec.ts

@@ -734,8 +734,7 @@ describe('change detection for transplanted views', () => {
       // Detach view, manually call `detectChanges`, and verify the template was refreshed
       component.rootViewContainerRef.detach();
       viewRef.detectChanges();
-      // This view is a backwards reference so it's refreshed twice
-      expect(component.checks).toEqual(2);
+      expect(component.checks).toEqual(1);
     });
 
     it('should work when change detecting detached transplanted view already marked for refresh',
@@ -751,8 +750,7 @@ describe('change detection for transplanted views', () => {
            // should not affect parent counters.
            viewRef.detectChanges();
          }).not.toThrow();
-         // This view is a backwards reference so it's refreshed twice
-         expect(component.checks).toEqual(2);
+         expect(component.checks).toEqual(1);
        });
 
     it('should work when re-inserting a previously detached transplanted view marked for refresh',

packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json

@@ -698,6 +698,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "cloakAndComputeStyles"
   },

packages/core/test/bundling/animations/bundle.golden_symbols.json

@@ -758,6 +758,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "cloakAndComputeStyles"
   },

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

@@ -572,6 +572,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/defer/bundle.golden_symbols.json

@@ -650,6 +650,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

@@ -776,6 +776,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

@@ -755,6 +755,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -437,6 +437,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/hydration/bundle.golden_symbols.json

@@ -656,6 +656,9 @@
   {
     "name": "clearElementContents"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -989,6 +989,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json

@@ -512,6 +512,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -680,6 +680,9 @@
   {
     "name": "cleanUpView"
   },
+  {
+    "name": "clearRefreshAndTraversalFlags"
+  },
   {
     "name": "collectNativeNodes"
   },