From 562f63b37d723d0c61d504cfc890ae3ba284aa70 Mon Sep 17 00:00:00 2001 From: teja2 Date: Tue, 25 Nov 2025 12:25:47 -0500 Subject: [PATCH 1/3] fix-32253 --- src/cdk/table/table.spec.ts | 145 +++++++++++++++++++++++++++++++++++- src/cdk/table/table.ts | 2 + 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/src/cdk/table/table.spec.ts b/src/cdk/table/table.spec.ts index 291cc6037520..6d1db8204301 100644 --- a/src/cdk/table/table.spec.ts +++ b/src/cdk/table/table.spec.ts @@ -1,5 +1,11 @@ import {BidiModule, Direction} from '../bidi'; -import {CollectionViewer, DataSource} from '../collections'; +import { + CollectionViewer, + DataSource, + _RecycleViewRepeaterStrategy, + _VIEW_REPEATER_STRATEGY, + _ViewRepeaterItemContext, +} from '../collections'; import { AfterContentInit, AfterViewInit, @@ -375,6 +381,56 @@ describe('CdkTable', () => { expect(() => table.renderRows()).not.toThrow(); }); + describe('with recycled rows', () => { + beforeEach(() => { + setupTableTestApp(CdkTableRecycleRowsApp); + component = fixture.componentInstance as CdkTableRecycleRowsApp; + tableElement = fixture.nativeElement.querySelector('.cdk-table'); + }); + + it('should re-render cached rows when columns change', () => { + component.addExtraRows(3); + fixture.detectChanges(); + + component.setShortList(); + fixture.detectChanges(); + + component.showSingleColumn(); + fixture.detectChanges(); + + const rows = getRows(tableElement); + expect(rows.length).toBe(component.dataSource.data.length); + rows.forEach(row => { + expect(getCells(row).length).toBe(component.displayedColumns.length); + }); + }); + }); + + describe('view repeater cleanup', () => { + beforeEach(() => { + setupTableTestApp(CdkTableWithCustomStrategyApp); + component = fixture.componentInstance as CdkTableWithCustomStrategyApp; + }); + + it('should detach the view repeater when forcing a re-render', () => { + const strategy = TestRecycleViewRepeaterStrategy.lastInstance!; + expect(strategy.detachCallCount).toBe(0); + + component.table['_forceRenderDataRows'](); + + expect(strategy.detachCallCount).toBe(1); + }); + + it('should detach the view repeater when the table is destroyed', () => { + const destroyFixture = TestBed.createComponent(CdkTableWithCustomStrategyApp); + destroyFixture.detectChanges(); + const strategy = TestRecycleViewRepeaterStrategy.lastInstance!; + expect(strategy.detachCallCount).toBe(0); + destroyFixture.destroy(); + expect(strategy.detachCallCount).toBe(1); + }); + }); + describe('with different data inputs other than data source', () => { let baseData: TestData[] = [ {a: 'a_1', b: 'b_1', c: 'c_1'}, @@ -3176,6 +3232,93 @@ class WrapNativeHtmlTableAppOnPush { dataSource = new FakeDataSource(); } +@Component({ + template: ` + + + + + + + + + + + + + + + + + + +
Column A {{row.a}} Column B {{row.b}} Column C {{row.c}}
+ `, + imports: [CdkTableModule], +}) +class CdkTableRecycleRowsApp { + private _longColumnSet = ['column_a', 'column_b', 'column_c']; + private _shortColumnSet = ['column_a']; + private _shortListLength = 3; + dataSource = new FakeDataSource(); + displayedColumns = this._longColumnSet.slice(); + + addExtraRows(count: number) { + for (let i = 0; i < count; i++) { + this.dataSource.addData(); + } + } + + setShortList() { + this.dataSource.data = this.dataSource.data.slice(0, this._shortListLength); + } + + showSingleColumn() { + this.displayedColumns = this._shortColumnSet.slice(); + } +} + +class TestRecycleViewRepeaterStrategy< + T, + R, + C extends _ViewRepeaterItemContext, +> extends _RecycleViewRepeaterStrategy { + static lastInstance: TestRecycleViewRepeaterStrategy | null = null; + detachCallCount = 0; + + constructor() { + super(); + TestRecycleViewRepeaterStrategy.lastInstance = this; + } + + override detach() { + super.detach(); + this.detachCallCount++; + } +} + +@Component({ + template: ` + + + Column A + {{row.a}} + + + + + + `, + providers: [{provide: _VIEW_REPEATER_STRATEGY, useClass: TestRecycleViewRepeaterStrategy}], + imports: [CdkTableModule], +}) +class CdkTableWithCustomStrategyApp { + dataSource = new FakeDataSource(); + columnsToRender = ['column_a']; + + @ViewChild(CdkTable) table: CdkTable; +} + function getElements(element: Element, query: string): HTMLElement[] { return [].slice.call(element.querySelectorAll(query)); } diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts index c704690e1163..2189289022ea 100644 --- a/src/cdk/table/table.ts +++ b/src/cdk/table/table.ts @@ -640,6 +640,7 @@ export class CdkTable ngOnDestroy() { this._stickyStyler?.destroy(); + this._viewRepeater.detach(); [ this._rowOutlet?.viewContainer, @@ -1330,6 +1331,7 @@ export class CdkTable * `multiTemplateDataRows` or adding/removing row definitions. */ private _forceRenderDataRows() { + this._viewRepeater.detach(); this._dataDiffer.diff([]); this._rowOutlet.viewContainer.clear(); this.renderRows(); From 8e41f72573af23c4442e9e252aff322859672064 Mon Sep 17 00:00:00 2001 From: teja2 Date: Wed, 26 Nov 2025 13:48:02 -0500 Subject: [PATCH 2/3] fix-32253- detach fix --- .../recycle-view-repeater-strategy.ts | 18 ++++++++++++++---- src/cdk/table/table.spec.ts | 12 ++++++++++-- src/cdk/table/table.ts | 9 ++++++++- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/cdk/collections/recycle-view-repeater-strategy.ts b/src/cdk/collections/recycle-view-repeater-strategy.ts index 735e32aec9de..f759ed1b5d7a 100644 --- a/src/cdk/collections/recycle-view-repeater-strategy.ts +++ b/src/cdk/collections/recycle-view-repeater-strategy.ts @@ -106,10 +106,12 @@ export class _RecycleViewRepeaterStrategy { component = fixture.componentInstance as CdkTableWithCustomStrategyApp; }); - it('should detach the view repeater when forcing a re-render', () => { + it('should clear the recycled row cache when forcing a re-render', () => { const strategy = TestRecycleViewRepeaterStrategy.lastInstance!; + expect(strategy.clearCacheCallCount).toBe(0); expect(strategy.detachCallCount).toBe(0); component.table['_forceRenderDataRows'](); - expect(strategy.detachCallCount).toBe(1); + expect(strategy.clearCacheCallCount).toBe(1); + expect(strategy.detachCallCount).toBe(0); }); it('should detach the view repeater when the table is destroyed', () => { @@ -3285,6 +3287,7 @@ class TestRecycleViewRepeaterStrategy< > extends _RecycleViewRepeaterStrategy { static lastInstance: TestRecycleViewRepeaterStrategy | null = null; detachCallCount = 0; + clearCacheCallCount = 0; constructor() { super(); @@ -3295,6 +3298,11 @@ class TestRecycleViewRepeaterStrategy< super.detach(); this.detachCallCount++; } + + override clearCache() { + super.clearCache(); + this.clearCacheCallCount++; + } } @Component({ diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts index 2189289022ea..f6bd2120230b 100644 --- a/src/cdk/table/table.ts +++ b/src/cdk/table/table.ts @@ -1331,12 +1331,19 @@ export class CdkTable * `multiTemplateDataRows` or adding/removing row definitions. */ private _forceRenderDataRows() { - this._viewRepeater.detach(); + this._clearReusableRowCache(); this._dataDiffer.diff([]); this._rowOutlet.viewContainer.clear(); this.renderRows(); } + /** Clears any cached data rows that were being reused. */ + private _clearReusableRowCache() { + if (this._viewRepeater instanceof _RecycleViewRepeaterStrategy) { + this._viewRepeater.clearCache(); + } + } + /** * Checks if there has been a change in sticky states since last check and applies the correct * sticky styles. Since checking resets the "dirty" state, this should only be performed once From 1dc2b625b08a39fb7e4a17ff6cb4377e4a892ac3 Mon Sep 17 00:00:00 2001 From: teja2 Date: Thu, 27 Nov 2025 09:44:14 -0500 Subject: [PATCH 3/3] check --- src/cdk/table/table.spec.ts | 128 +++++++++++++++++------------------- 1 file changed, 59 insertions(+), 69 deletions(-) diff --git a/src/cdk/table/table.spec.ts b/src/cdk/table/table.spec.ts index 0a9627e77d78..9c88adb5b6f8 100644 --- a/src/cdk/table/table.spec.ts +++ b/src/cdk/table/table.spec.ts @@ -3,7 +3,6 @@ import { CollectionViewer, DataSource, _RecycleViewRepeaterStrategy, - _VIEW_REPEATER_STRATEGY, _ViewRepeaterItemContext, } from '../collections'; import { @@ -31,7 +30,7 @@ import { StickyPositioningListener, StickyUpdate, } from './index'; -import {CdkCellOutlet, CdkHeaderRowDef, CdkNoDataRow, CdkRowDef} from './row'; +import {BaseRowDef, CdkCellOutlet, CdkHeaderRowDef, CdkNoDataRow, CdkRowDef} from './row'; import {CdkTable} from './table'; import { getTableDuplicateColumnNameError, @@ -395,41 +394,63 @@ describe('CdkTable', () => { component.setShortList(); fixture.detectChanges(); - component.showSingleColumn(); + const newColumns = ['column_a']; + const tableInstance = component.table as CdkTable; + const forceColumns = (defs: BaseRowDef[]) => { + defs.forEach(def => { + def.columns = newColumns; + (def as any)._columnsDiffer?.diff(newColumns); + }); + }; + + forceColumns(tableInstance['_rowDefs']); + forceColumns(tableInstance['_headerRowDefs']); + + tableInstance['_forceRenderDataRows'](); + tableInstance['_forceRenderHeaderRows'](); fixture.detectChanges(); const rows = getRows(tableElement); expect(rows.length).toBe(component.dataSource.data.length); rows.forEach(row => { - expect(getCells(row).length).toBe(component.displayedColumns.length); + expect(getCells(row).length).toBe(newColumns.length); }); }); }); describe('view repeater cleanup', () => { beforeEach(() => { - setupTableTestApp(CdkTableWithCustomStrategyApp); - component = fixture.componentInstance as CdkTableWithCustomStrategyApp; + setupTableTestApp(CdkTableRecycleRowsSpyApp); + component = fixture.componentInstance as CdkTableRecycleRowsSpyApp; }); it('should clear the recycled row cache when forcing a re-render', () => { - const strategy = TestRecycleViewRepeaterStrategy.lastInstance!; - expect(strategy.clearCacheCallCount).toBe(0); - expect(strategy.detachCallCount).toBe(0); + const repeater = component.table['_viewRepeater'] as _RecycleViewRepeaterStrategy< + unknown, + unknown, + _ViewRepeaterItemContext + >; + spyOn(repeater, 'clearCache'); component.table['_forceRenderDataRows'](); - expect(strategy.clearCacheCallCount).toBe(1); - expect(strategy.detachCallCount).toBe(0); + expect(repeater.clearCache).toHaveBeenCalledTimes(1); }); it('should detach the view repeater when the table is destroyed', () => { - const destroyFixture = TestBed.createComponent(CdkTableWithCustomStrategyApp); + const destroyFixture = TestBed.createComponent(CdkTableRecycleRowsSpyApp); destroyFixture.detectChanges(); - const strategy = TestRecycleViewRepeaterStrategy.lastInstance!; - expect(strategy.detachCallCount).toBe(0); + const table = destroyFixture.componentInstance.table; + const repeater = table['_viewRepeater'] as _RecycleViewRepeaterStrategy< + unknown, + unknown, + _ViewRepeaterItemContext + >; + spyOn(repeater, 'detach'); + destroyFixture.destroy(); - expect(strategy.detachCallCount).toBe(1); + + expect(repeater.detach).toHaveBeenCalledTimes(1); }); }); @@ -3234,6 +3255,26 @@ class WrapNativeHtmlTableAppOnPush { dataSource = new FakeDataSource(); } +@Component({ + template: ` + + + + + + + + +
Column A {{row.a}}
+ `, + imports: [CdkTableModule], +}) +class CdkTableRecycleRowsSpyApp { + dataSource = new FakeDataSource(); + + @ViewChild(CdkTable) table: CdkTable; +} + @Component({ template: ` @@ -3259,11 +3300,11 @@ class WrapNativeHtmlTableAppOnPush { imports: [CdkTableModule], }) class CdkTableRecycleRowsApp { - private _longColumnSet = ['column_a', 'column_b', 'column_c']; - private _shortColumnSet = ['column_a']; + private _longColumnSet: string[] = ['column_a', 'column_b', 'column_c']; private _shortListLength = 3; dataSource = new FakeDataSource(); - displayedColumns = this._longColumnSet.slice(); + displayedColumns = this._longColumnSet; + @ViewChild(CdkTable) table: CdkTable; addExtraRows(count: number) { for (let i = 0; i < count; i++) { @@ -3274,57 +3315,6 @@ class CdkTableRecycleRowsApp { setShortList() { this.dataSource.data = this.dataSource.data.slice(0, this._shortListLength); } - - showSingleColumn() { - this.displayedColumns = this._shortColumnSet.slice(); - } -} - -class TestRecycleViewRepeaterStrategy< - T, - R, - C extends _ViewRepeaterItemContext, -> extends _RecycleViewRepeaterStrategy { - static lastInstance: TestRecycleViewRepeaterStrategy | null = null; - detachCallCount = 0; - clearCacheCallCount = 0; - - constructor() { - super(); - TestRecycleViewRepeaterStrategy.lastInstance = this; - } - - override detach() { - super.detach(); - this.detachCallCount++; - } - - override clearCache() { - super.clearCache(); - this.clearCacheCallCount++; - } -} - -@Component({ - template: ` - - - Column A - {{row.a}} - - - - - - `, - providers: [{provide: _VIEW_REPEATER_STRATEGY, useClass: TestRecycleViewRepeaterStrategy}], - imports: [CdkTableModule], -}) -class CdkTableWithCustomStrategyApp { - dataSource = new FakeDataSource(); - columnsToRender = ['column_a']; - - @ViewChild(CdkTable) table: CdkTable; } function getElements(element: Element, query: string): HTMLElement[] {