fix(virtual-scroll): not removing view from container if it's outside the template cache (#13916)

Currently when detaching a view, we check whether it would fit in the cache, and if it doesn't, we destroy it. Since we destroy the view on it's own, the `ViewContainerRef` still has a reference to it, which means that we'll trigger change detection on it the next time the data changes. These changes switch to destroying the view through the view container.

Fixes #13901.

Author: crisbeto

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

@@ -324,7 +324,16 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
     if (this._templateCache.length < this.cdkVirtualForTemplateCacheSize) {
       this._templateCache.push(view);
     } else {
-      view.destroy();
+      const index = this._viewContainerRef.indexOf(view);
+
+      // It's very unlikely that the index will ever be -1, but just in case,
+      // destroy the view on its own, otherwise destroy it through the
+      // container to ensure that all the references are removed.
+      if (index === -1) {
+        view.destroy();
+      } else {
+        this._viewContainerRef.remove(index);
+      }
     }
   }
 

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

@@ -611,6 +611,24 @@ describe('CdkVirtualScrollViewport', () => {
       finishInit(fixture);
       expect(zoneTest).toHaveBeenCalledWith(true);
     }));
+
+    it('should not throw when disposing of a view that will not fit in the cache', fakeAsync(() => {
+      finishInit(fixture);
+      testComponent.items = new Array(200).fill(0);
+      testComponent.templateCacheSize = 1; // Reduce the cache size to something we can easily hit.
+      fixture.detectChanges();
+      flush();
+
+      expect(() => {
+        for (let i = 0; i < 50; i++) {
+          viewport.scrollToIndex(i);
+          triggerScroll(viewport);
+          fixture.detectChanges();
+          flush();
+        }
+      }).not.toThrow();
+    }));
+
   });
 
   describe('with RTL direction', () => {