fix(core): Reattached views that are dirty from a signal update shoul…

…d refresh (#53001)

Related to #52928 but `updateAncestorTraversalFlagsOnAttach` is called
on view insertion and _should_ have made that work for views dirty from
signals but it wasn't updated to read the `dirty` flag when we changed
it from sharing the `RefreshView` flag.

For #52928, we've traditionally worked under the assumption that this is working
as expected.  The created view is `CheckAlways`. There is a question of whether we
should automatically mark things for check when the attached view has
the `Dirty` flag and/or has the `FirstLViewPass` flag set (or other
flags that indicate it definitely needs to be prefreshed).

PR Close #53001

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, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
+import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, requiresRefreshOrTraversal, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils';
 
 import {executeTemplate, executeViewQueryFn, handleError, processHostBindingOpCodes, refreshContentQueries} from './shared';
 
@@ -72,8 +72,7 @@ function detectChangesInViewWhileDirty(lView: LView) {
   // descendants views that need to be refreshed due to re-dirtying during the change detection
   // run, detect changes on the view again. We run change detection in `Targeted` mode to only
   // refresh views with the `RefreshView` flag.
-  while (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) ||
-         lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty) {
+  while (requiresRefreshOrTraversal(lView)) {
     if (retries === MAXIMUM_REFRESH_RERUNS) {
       throw new RuntimeError(
           RuntimeErrorCode.INFINITE_CHANGE_DETECTION,

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

@@ -13,7 +13,7 @@ import {LContainer, LContainerFlags, TYPE} from '../interfaces/container';
 import {TConstants, TNode} from '../interfaces/node';
 import {RNode} from '../interfaces/renderer_dom';
 import {isLContainer, isLView} from '../interfaces/type_checks';
-import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TData, TView} from '../interfaces/view';
+import {DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, ON_DESTROY_HOOKS, PARENT, PREORDER_HOOK_FLAGS, PreOrderHookFlags, REACTIVE_TEMPLATE_CONSUMER, TData, TView} from '../interfaces/view';
 
 
 
@@ -195,21 +195,22 @@ export function walkUpViews(nestingLevel: number, currentView: LView): LView {
   return currentView;
 }
 
+export function requiresRefreshOrTraversal(lView: LView) {
+  return lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh) ||
+      lView[REACTIVE_TEMPLATE_CONSUMER]?.dirty;
+}
+
 
 /**
- * Updates the `DESCENDANT_VIEWS_TO_REFRESH` counter on the parents of the `LView` as well as the
- * parents above that whose
- *  1. counter goes from 0 to 1, indicating that there is a new child that has a view to refresh
- *  or
- *  2. counter goes from 1 to 0, indicating there are no more descendant views to refresh
- * When attaching/re-attaching an `LView` to the change detection tree, we need to ensure that the
- * views above it are traversed during change detection if this one is marked for refresh or has
- * some child or descendant that needs to be refreshed.
+ * Updates the `HasChildViewsToRefresh` flag on the parents of the `LView` as well as the
+ * parents above.
  */
 export function updateAncestorTraversalFlagsOnAttach(lView: LView) {
-  if (lView[FLAGS] & (LViewFlags.RefreshView | LViewFlags.HasChildViewsToRefresh)) {
-    markAncestorsForTraversal(lView);
+  if (!requiresRefreshOrTraversal(lView)) {
+    return;
   }
+
+  markAncestorsForTraversal(lView);
 }
 
 /**

packages/core/test/acceptance/change_detection_signals_in_zones_spec.ts

@@ -793,7 +793,9 @@ describe('OnPush components with signals', () => {
 
     @Component({
       selector: 'on-push-parent',
-      template: `<signal-component></signal-component>{{incrementChecks()}}`,
+      template: `
+      <signal-component></signal-component>
+      {{incrementChecks()}}`,
       changeDetection: ChangeDetectionStrategy.OnPush,
       standalone: true,
       imports: [SignalComponent],
@@ -827,6 +829,18 @@ describe('OnPush components with signals', () => {
       expect(trim(fixture.nativeElement.textContent)).toEqual('initial');
     });
 
+    it('refreshes when reattached if already dirty', () => {
+      const fixture = TestBed.createComponent(OnPushParent);
+      fixture.detectChanges();
+      fixture.componentInstance.signalChild.value.set('new');
+      fixture.componentInstance.signalChild.cdr.detach();
+      fixture.detectChanges();
+      expect(trim(fixture.nativeElement.textContent)).toEqual('initial');
+      fixture.componentInstance.signalChild.cdr.reattach();
+      fixture.detectChanges();
+      expect(trim(fixture.nativeElement.textContent)).toEqual('new');
+    });
+
     // Note: Design decision for signals because that's how the hooks work today
     // We have considered actually running a component's `afterViewChecked` hook if it's refreshed
     // in targeted mode (meaning the parent did not refresh) and could change this decision.

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

@@ -1328,6 +1328,9 @@
   {
     "name": "replacePostStylesAsPre"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1400,6 +1400,9 @@
   {
     "name": "replacePostStylesAsPre"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1112,6 +1112,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -2249,6 +2249,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1553,6 +1553,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1523,6 +1523,9 @@
   {
     "name": "requiredValidator"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -881,6 +881,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1220,6 +1220,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1880,6 +1880,9 @@
   {
     "name": "replaceSegment"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -977,6 +977,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },

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

@@ -1334,6 +1334,9 @@
   {
     "name": "renderView"
   },
+  {
+    "name": "requiresRefreshOrTraversal"
+  },
   {
     "name": "resetPreOrderHookFlags"
   },