fix(cdk/scrolling): virtual scroll not picking up trackBy function when items come in after init (#21335)

The virtual scroll only creates its `IterableDiffer` once, which means that if the
`trackBy` function comes in at a later point (e.g. the input changed or the
initialization order is different), it won't be picked up.

These changes make it so the differ always has a `trackBy` which then delegates
to the user-provided one or falls back to the default.

Fixes #21281.

Author: crisbeto

src/cdk/scrolling/BUILD.bazel

@@ -45,6 +45,7 @@ ng_test_library(
         "//src/cdk/bidi",
         "//src/cdk/collections",
         "//src/cdk/testing/private",
+        "@npm//@angular/common",
         "@npm//rxjs",
     ],
 )

src/cdk/scrolling/virtual-for-of.ts

@@ -288,7 +288,11 @@ export class CdkVirtualForOf<T> implements
     }
     this._renderedItems = this._data.slice(this._renderedRange.start, this._renderedRange.end);
     if (!this._differ) {
-      this._differ = this._differs.find(this._renderedItems).create(this.cdkVirtualForTrackBy);
+      // Use a wrapper function for the `trackBy` so any new values are
+      // picked up automatically without having to recreate the differ.
+      this._differ = this._differs.find(this._renderedItems).create((index, item) => {
+        return this.cdkVirtualForTrackBy ? this.cdkVirtualForTrackBy(index, item) : item;
+      });
     }
     this._needsUpdate = true;
   }
@@ -310,7 +314,7 @@ export class CdkVirtualForOf<T> implements
     const count = this._data.length;
     let i = this._viewContainerRef.length;
     while (i--) {
-      let view = this._viewContainerRef.get(i) as EmbeddedViewRef<CdkVirtualForOfContext<T>>;
+      const view = this._viewContainerRef.get(i) as EmbeddedViewRef<CdkVirtualForOfContext<T>>;
       view.context.index = this._renderedRange.start + i;
       view.context.count = count;
       this._updateComputedContextProperties(view.context);
@@ -324,7 +328,7 @@ export class CdkVirtualForOf<T> implements
         changes,
         this._viewContainerRef,
         (record: IterableChangeRecord<T>,
-         adjustedPreviousIndex: number | null,
+         _adjustedPreviousIndex: number | null,
          currentIndex: number | null) => this._getEmbeddedViewArgs(record, currentIndex!),
         (record) => record.item);
 

src/cdk/scrolling/virtual-scroll-viewport.spec.ts

@@ -5,10 +5,10 @@ import {
   ScrollDispatcher,
   ScrollingModule
 } from '@angular/cdk/scrolling';
+import {CommonModule} from '@angular/common';
 import {dispatchFakeEvent} from '@angular/cdk/testing/private';
 import {
   Component,
-  Input,
   NgZone,
   TrackByFunction,
   ViewChild,
@@ -29,7 +29,7 @@ import {animationFrameScheduler, Subject} from 'rxjs';
 
 
 describe('CdkVirtualScrollViewport', () => {
-  describe ('with FixedSizeVirtualScrollStrategy', () => {
+  describe('with FixedSizeVirtualScrollStrategy', () => {
     let fixture: ComponentFixture<FixedSizeVirtualScroll>;
     let testComponent: FixedSizeVirtualScroll;
     let viewport: CdkVirtualScrollViewport;
@@ -899,6 +899,35 @@ describe('CdkVirtualScrollViewport', () => {
           .toEqual(['0', '1', '2', '3', '4', '5', '6', '7']);
       }));
   });
+
+  describe('with delayed initialization', () => {
+    let fixture: ComponentFixture<DelayedInitializationVirtualScroll>;
+    let testComponent: DelayedInitializationVirtualScroll;
+    let viewport: CdkVirtualScrollViewport;
+
+    beforeEach(waitForAsync(() => {
+      TestBed.configureTestingModule({
+        imports: [ScrollingModule, CommonModule],
+        declarations: [DelayedInitializationVirtualScroll],
+      }).compileComponents();
+      fixture = TestBed.createComponent(DelayedInitializationVirtualScroll);
+      testComponent = fixture.componentInstance;
+      viewport = testComponent.viewport;
+    }));
+
+    it('should call custom trackBy when virtual for is added after init', fakeAsync(() => {
+      finishInit(fixture);
+      expect(testComponent.trackBy).not.toHaveBeenCalled();
+
+      testComponent.renderVirtualFor = true;
+      fixture.detectChanges();
+      triggerScroll(viewport, testComponent.itemSize * 5);
+      fixture.detectChanges();
+      flush();
+
+      expect(testComponent.trackBy).toHaveBeenCalled();
+    }));
+  });
 });
 
 
@@ -973,15 +1002,15 @@ class FixedSizeVirtualScroll {
   // Casting virtualForOf as any so we can spy on private methods
   @ViewChild(CdkVirtualForOf, {static: true}) virtualForOf: any;
 
-  @Input() orientation = 'vertical';
-  @Input() viewportSize = 200;
-  @Input() viewportCrossSize = 100;
-  @Input() itemSize = 50;
-  @Input() minBufferPx = 0;
-  @Input() maxBufferPx = 0;
-  @Input() items = Array(10).fill(0).map((_, i) => i);
-  @Input() trackBy: TrackByFunction<number>;
-  @Input() templateCacheSize = 20;
+  orientation = 'vertical';
+  viewportSize = 200;
+  viewportCrossSize = 100;
+  itemSize = 50;
+  minBufferPx = 0;
+  maxBufferPx = 0;
+  items = Array(10).fill(0).map((_, i) => i);
+  trackBy: TrackByFunction<number>;
+  templateCacheSize = 20;
 
   scrolledToIndex = 0;
   hasMargin = false;
@@ -1033,15 +1062,15 @@ class FixedSizeVirtualScroll {
 class FixedSizeVirtualScrollWithRtlDirection {
   @ViewChild(CdkVirtualScrollViewport, {static: true}) viewport: CdkVirtualScrollViewport;
 
-  @Input() orientation = 'vertical';
-  @Input() viewportSize = 200;
-  @Input() viewportCrossSize = 100;
-  @Input() itemSize = 50;
-  @Input() minBufferPx = 0;
-  @Input() maxBufferPx = 0;
-  @Input() items = Array(10).fill(0).map((_, i) => i);
-  @Input() trackBy: TrackByFunction<number>;
-  @Input() templateCacheSize = 20;
+  orientation = 'vertical';
+  viewportSize = 200;
+  viewportCrossSize = 100;
+  itemSize = 50;
+  minBufferPx = 0;
+  maxBufferPx = 0;
+  items = Array(10).fill(0).map((_, i) => i);
+  trackBy: TrackByFunction<number>;
+  templateCacheSize = 20;
 
   scrolledToIndex = 0;
 
@@ -1116,3 +1145,40 @@ class VirtualScrollWithItemInjectingViewContainer {
   items = Array(20000).fill(0).map((_, i) => i);
 }
 
+
+@Component({
+  template: `
+    <cdk-virtual-scroll-viewport [itemSize]="itemSize">
+      <ng-container *ngIf="renderVirtualFor">
+        <div class="item" *cdkVirtualFor="let item of items; trackBy: trackBy">{{item}}</div>
+      </ng-container>
+    </cdk-virtual-scroll-viewport>
+  `,
+  styles: [`
+    .cdk-virtual-scroll-content-wrapper {
+      display: flex;
+      flex-direction: column;
+    }
+
+    .cdk-virtual-scroll-viewport {
+      width: 200px;
+      height: 200px;
+      background-color: #f5f5f5;
+    }
+
+    .item {
+      width: 100%;
+      height: 50px;
+      box-sizing: border-box;
+      border: 1px dashed #ccc;
+    }
+  `],
+  encapsulation: ViewEncapsulation.None,
+})
+class DelayedInitializationVirtualScroll {
+  @ViewChild(CdkVirtualScrollViewport, {static: true}) viewport: CdkVirtualScrollViewport;
+  itemSize = 50;
+  items = Array(20000).fill(0).map((_, i) => i);
+  trackBy = jasmine.createSpy('trackBy').and.callFake((item: unknown) => item);
+  renderVirtualFor = false;
+}