fix(core): ensure @for iteration over field is reactive (#64113)

When working with a proxy object such as signal forms' `Field`,
accessing the `lenght` or `Symbol.iterator` may trgger a reactive read.
This change ensures that `@for` properly captrues this before clearing
the active consumer.

PR Close #64113

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

@@ -39,15 +39,15 @@ import {NO_CHANGE} from '../tokens';
 import {getConstant, getTNode} from '../util/view_utils';
 import {createAndRenderEmbeddedLView, shouldAddViewToDom} from '../view_manipulation';
 
-import {declareNoDirectiveHostTemplate} from './template';
+import {AnimationLViewData} from '../../animation/interfaces';
+import {removeDehydratedViews} from '../../hydration/cleanup';
 import {
   addLViewToLContainer,
   detachView,
   getLViewFromLContainer,
   removeLViewFromLContainer,
 } from '../view/container';
-import {removeDehydratedViews} from '../../hydration/cleanup';
-import {AnimationLViewData} from '../../animation/interfaces';
+import {declareNoDirectiveHostTemplate} from './template';
 
 /**
  * Creates an LContainer for an ng-template representing a root node
@@ -495,7 +495,7 @@ export function ɵɵrepeater(collection: Iterable<unknown> | undefined | null):
     }
 
     const liveCollection = metadata.liveCollection;
-    reconcile(liveCollection, collection, metadata.trackByFn);
+    reconcile(liveCollection, collection, metadata.trackByFn, prevConsumer);
 
     // Warn developers about situations where the entire collection was re-created as part of the
     // reconciliation pass. Note that this warning might be "overreacting" and report cases where

packages/core/src/render3/list_reconciliation.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
+import {setActiveConsumer, type ReactiveNode} from '@angular/core/primitives/signals';
 import {TrackByFunction} from '../change_detection';
 import {formatRuntimeError, RuntimeErrorCode} from '../errors';
 
@@ -103,11 +104,15 @@ function recordDuplicateKeys(keyToIdx: Map<unknown, Set<number>>, key: unknown,
  * @param newCollection the new, incoming collection;
  * @param trackByFn key generation function that determines equality between items in the life and
  *     incoming collection;
+ * @param reactiveConsumer the reactive consumer to be used when accessing the length or iterator of
+ *     the {@link newCollection}. This ensures that if the object is a proxy, reactive reads that
+ *     occur when accessing the length or iterator are tracked.
  */
 export function reconcile<T, V>(
   liveCollection: LiveCollection<T, V>,
   newCollection: Iterable<V> | undefined | null,
   trackByFn: TrackByFunction<V>,
+  reactiveConsumer: ReactiveNode | null,
 ): void {
   let detachedItems: UniqueValueMultiKeyMap<unknown, T> | undefined = undefined;
   let liveKeysInTheFuture: Set<unknown> | undefined = undefined;
@@ -118,7 +123,9 @@ export function reconcile<T, V>(
   const duplicateKeys = ngDevMode ? new Map<unknown, Set<number>>() : undefined;
 
   if (Array.isArray(newCollection)) {
+    setActiveConsumer(reactiveConsumer);
     let newEndIdx = newCollection.length - 1;
+    setActiveConsumer(null);
 
     while (liveStartIdx <= liveEndIdx && liveStartIdx <= newEndIdx) {
       // compare from the beginning
@@ -235,7 +242,9 @@ export function reconcile<T, V>(
     }
   } else if (newCollection != null) {
     // iterable - immediately fallback to the slow path
+    setActiveConsumer(reactiveConsumer);
     const newCollectionIterator = newCollection[Symbol.iterator]();
+    setActiveConsumer(null);
     let newIterationResult = newCollectionIterator.next();
     while (!newIterationResult.done && liveStartIdx <= liveEndIdx) {
       const liveValue = liveCollection.at(liveStartIdx);

packages/core/test/acceptance/control_flow_for_spec.ts

@@ -8,6 +8,7 @@
 
 import {NgIf} from '@angular/common';
 import {
+  ChangeDetectionStrategy,
   ChangeDetectorRef,
   Component,
   Directive,
@@ -17,6 +18,7 @@ import {
   Pipe,
   PipeTransform,
   provideZoneChangeDetection,
+  signal,
   TemplateRef,
   ViewContainerRef,
 } from '../../src/core';
@@ -1103,4 +1105,61 @@ describe('control flow - for', () => {
       expect(fixture.nativeElement.textContent).toBe('Main: Before  After Slot: 3');
     });
   });
+
+  describe('reactivity', () => {
+    it('should do a reactive read of length', () => {
+      const len = signal(2);
+
+      @Component({
+        template: `@for (item of items; track item) {{{item}}|}`,
+        changeDetection: ChangeDetectionStrategy.OnPush,
+      })
+      class TestComponent {
+        items = new Proxy([1, 2, 3, 4, 5], {
+          get(target, prop) {
+            if (prop === 'length') {
+              return len();
+            }
+            return target[prop as keyof number[]];
+          },
+        });
+      }
+
+      const fixture = TestBed.createComponent(TestComponent);
+      TestBed.tick();
+      expect(fixture.nativeElement.textContent).toBe('1|2|');
+      len.set(4);
+      TestBed.tick();
+      expect(fixture.nativeElement.textContent).toBe('1|2|3|4|');
+    });
+  });
+
+  it('should do a reactive read of iterator', () => {
+    const iter = signal(() => [1, 2][Symbol.iterator]());
+
+    @Component({
+      template: `@for (item of items; track $index) {{{item}}|}`,
+      changeDetection: ChangeDetectionStrategy.OnPush,
+    })
+    class TestComponent {
+      items = new Proxy(
+        {},
+        {
+          get(target, prop) {
+            if (prop === Symbol.iterator) {
+              return iter();
+            }
+            return target[prop as keyof {}];
+          },
+        },
+      );
+    }
+
+    const fixture = TestBed.createComponent(TestComponent);
+    TestBed.tick();
+    expect(fixture.nativeElement.textContent).toBe('1|2|');
+    iter.set(() => [1, 2, 3, 4][Symbol.iterator]());
+    TestBed.tick();
+    expect(fixture.nativeElement.textContent).toBe('1|2|3|4|');
+  });
 });

packages/core/test/render3/list_reconciliation_spec.ts

@@ -100,7 +100,7 @@ describe('list reconciliation', () => {
   describe('fast path', () => {
     it('should do nothing if 2 lists are the same', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, ['a', 'b', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'b', 'c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([]);
@@ -108,7 +108,7 @@ describe('list reconciliation', () => {
 
     it('should add items at the end', () => {
       const pc = new LoggingLiveCollection(['a', 'b']);
-      reconcile(pc, ['a', 'b', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'b', 'c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -119,7 +119,7 @@ describe('list reconciliation', () => {
 
     it('should swap items', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, ['c', 'b', 'a'], trackByIdentity);
+      reconcile(pc, ['c', 'b', 'a'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['c', 'b', 'a']);
       // TODO: think of expressing as swap
@@ -133,7 +133,7 @@ describe('list reconciliation', () => {
 
     it('should should optimally swap adjacent items', () => {
       const pc = new LoggingLiveCollection(['a', 'b']);
-      reconcile(pc, ['b', 'a'], trackByIdentity);
+      reconcile(pc, ['b', 'a'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['b', 'a']);
       expect(pc.getLogs()).toEqual([
@@ -144,7 +144,7 @@ describe('list reconciliation', () => {
 
     it('should detect moves to the front', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']);
-      reconcile(pc, ['a', 'd', 'b', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'd', 'b', 'c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'd', 'b', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -155,7 +155,7 @@ describe('list reconciliation', () => {
 
     it('should delete items in the middle', () => {
       const pc = new LoggingLiveCollection(['a', 'x', 'b', 'c']);
-      reconcile(pc, ['a', 'b', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'b', 'c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -166,7 +166,7 @@ describe('list reconciliation', () => {
 
     it('should delete items from the beginning', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, ['c'], trackByIdentity);
+      reconcile(pc, ['c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['c']);
       expect(pc.getLogs()).toEqual([
@@ -179,7 +179,7 @@ describe('list reconciliation', () => {
 
     it('should delete items from the end', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, ['a'], trackByIdentity);
+      reconcile(pc, ['a'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a']);
       expect(pc.getLogs()).toEqual([
@@ -192,7 +192,7 @@ describe('list reconciliation', () => {
 
     it('should work with duplicated items', () => {
       const pc = new LoggingLiveCollection(['a', 'a', 'a']);
-      reconcile(pc, ['a', 'a', 'a'], trackByIdentity);
+      reconcile(pc, ['a', 'a', 'a'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'a', 'a']);
       expect(pc.getLogs()).toEqual([]);
@@ -202,7 +202,7 @@ describe('list reconciliation', () => {
   describe('slow path', () => {
     it('should delete multiple items from the middle', () => {
       const pc = new LoggingLiveCollection(['a', 'x1', 'b', 'x2', 'c']);
-      reconcile(pc, ['a', 'b', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'b', 'c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -215,7 +215,7 @@ describe('list reconciliation', () => {
 
     it('should add multiple items in the middle', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, ['a', 'n1', 'b', 'n2', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'n1', 'b', 'n2', 'c'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'n1', 'b', 'n2', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -228,7 +228,7 @@ describe('list reconciliation', () => {
 
     it('should go back to the fast path when start / end is different', () => {
       const pc = new LoggingLiveCollection(['s1', 'a', 'b', 'c', 'e1']);
-      reconcile(pc, ['s2', 'a', 'b', 'c', 'e2'], trackByIdentity);
+      reconcile(pc, ['s2', 'a', 'b', 'c', 'e2'], trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['s2', 'a', 'b', 'c', 'e2']);
       expect(pc.getLogs()).toEqual([
@@ -250,7 +250,7 @@ describe('list reconciliation', () => {
 
     it('should detect moves to the back', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']);
-      reconcile(pc, ['b', 'c', 'n1', 'n2', 'n3', 'a', 'd'], trackByIdentity);
+      reconcile(pc, ['b', 'c', 'n1', 'n2', 'n3', 'a', 'd'], trackByIdentity, null);
       expect(pc.getCollection()).toEqual(['b', 'c', 'n1', 'n2', 'n3', 'a', 'd']);
       expect(pc.getLogs()).toEqual([
         ['detach', 0, 'a'],
@@ -272,7 +272,7 @@ describe('list reconciliation', () => {
         {k: 2},
         {k: 3},
       ]);
-      reconcile(pc, [{k: 2}, {k: 3}, {k: 1}, {k: 1}, {k: 1}, {k: 4}], trackByKey);
+      reconcile(pc, [{k: 2}, {k: 3}, {k: 1}, {k: 1}, {k: 1}, {k: 4}], trackByKey, null);
 
       expect(pc.getCollection()).toEqual([{k: 2}, {k: 3}, {k: 1}, {k: 1}, {k: 1}, {k: 4}]);
       expect(pc.getLogs()).toEqual([
@@ -291,7 +291,7 @@ describe('list reconciliation', () => {
   describe('iterables', () => {
     it('should do nothing if 2 lists represented as iterables are the same', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity);
+      reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([]);
@@ -299,7 +299,7 @@ describe('list reconciliation', () => {
 
     it('should add items at the end', () => {
       const pc = new LoggingLiveCollection(['a', 'b']);
-      reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity);
+      reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -310,7 +310,7 @@ describe('list reconciliation', () => {
 
     it('should add multiple items in the middle', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, new Set(['a', 'n1', 'b', 'n2', 'c']), trackByIdentity);
+      reconcile(pc, new Set(['a', 'n1', 'b', 'n2', 'c']), trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'n1', 'b', 'n2', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -323,7 +323,7 @@ describe('list reconciliation', () => {
 
     it('should delete items from the end', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
-      reconcile(pc, new Set(['a']), trackByIdentity);
+      reconcile(pc, new Set(['a']), trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a']);
       expect(pc.getLogs()).toEqual([
@@ -336,7 +336,7 @@ describe('list reconciliation', () => {
 
     it('should detect (slow) moves to the front', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']);
-      reconcile(pc, new Set(['a', 'd', 'b', 'c']), trackByIdentity);
+      reconcile(pc, new Set(['a', 'd', 'b', 'c']), trackByIdentity, null);
 
       expect(pc.getCollection()).toEqual(['a', 'd', 'b', 'c']);
       expect(pc.getLogs()).toEqual([
@@ -349,7 +349,7 @@ describe('list reconciliation', () => {
 
     it('should detect (fast) moves to the back', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c', 'd']);
-      reconcile(pc, new Set(['b', 'c', 'a', 'd']), trackByIdentity);
+      reconcile(pc, new Set(['b', 'c', 'a', 'd']), trackByIdentity, null);
       expect(pc.getCollection()).toEqual(['b', 'c', 'a', 'd']);
       expect(pc.getLogs()).toEqual([
         ['detach', 0, 'a'],
@@ -360,11 +360,11 @@ describe('list reconciliation', () => {
     it('should allow switching collection types', () => {
       const pc = new LoggingLiveCollection(['a', 'b', 'c']);
 
-      reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity);
+      reconcile(pc, new Set(['a', 'b', 'c']), trackByIdentity, null);
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([]);
 
-      reconcile(pc, ['a', 'b', 'c'], trackByIdentity);
+      reconcile(pc, ['a', 'b', 'c'], trackByIdentity, null);
       expect(pc.getCollection()).toEqual(['a', 'b', 'c']);
       expect(pc.getLogs()).toEqual([]);
     });
@@ -405,14 +405,14 @@ describe('list reconciliation', () => {
     it('should update when tracking by index - fast path from the start', () => {
       const pc = new LoggingLiveCollection([], new RepeaterLikeItemFactory());
 
-      reconcile(pc, ['a', 'b', 'c'], trackByIndex);
+      reconcile(pc, ['a', 'b', 'c'], trackByIndex, null);
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: 'a'},
         {index: 1, implicit: 'b'},
         {index: 2, implicit: 'c'},
       ]);
 
-      reconcile(pc, ['c', 'b', 'a'], trackByIndex);
+      reconcile(pc, ['c', 'b', 'a'], trackByIndex, null);
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: 'c'},
         {index: 1, implicit: 'b'},
@@ -426,7 +426,7 @@ describe('list reconciliation', () => {
         new RepeaterLikeItemFactory<KeyValueItem<string, string>>(),
       );
 
-      reconcile(pc, [{k: 'o', v: 'o'}], trackByKey);
+      reconcile(pc, [{k: 'o', v: 'o'}], trackByKey, null);
       expect(pc.getCollection()).toEqual([{index: 0, implicit: {k: 'o', v: 'o'}}]);
 
       reconcile(
@@ -436,6 +436,7 @@ describe('list reconciliation', () => {
           {k: 'o', v: 'oo'},
         ],
         trackByKey,
+        null,
       );
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: {k: 'n', v: 'n'}},
@@ -458,6 +459,7 @@ describe('list reconciliation', () => {
           {k: 2, v: 'c'},
         ],
         trackByKey as any,
+        null,
       );
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: {k: 0, v: 'a'}},
@@ -473,6 +475,7 @@ describe('list reconciliation', () => {
           {k: 0, v: 'aa'},
         ],
         trackByKey as any,
+        null,
       );
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: {k: 2, v: 'cc'}},
@@ -495,6 +498,7 @@ describe('list reconciliation', () => {
           {k: 3, v: 'd'},
         ],
         trackByKey as any,
+        null,
       );
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: {k: 0, v: 'a'}},
@@ -512,6 +516,7 @@ describe('list reconciliation', () => {
           {k: 2, v: 'cc'},
         ],
         trackByKey as any,
+        null,
       );
       expect(pc.getCollection()).toEqual([
         {index: 0, implicit: {k: 0, v: 'aa'}},