perf(cdk/table): Use afterNextRender for sticky styling. Fixes a performance regression dating back to #28393 and removes need for coalesced sticky styler.

kseamon · kseamon · commit c7ca3b78f953 · 2024-12-27T13:16:18.000-05:00
diff --git a/src/cdk/table/sticky-styler.ts b/src/cdk/table/sticky-styler.ts
@@ -10,6 +10,7 @@
  * Directions that can be used when setting sticky positioning.
  * @docs-private
  */
+import {afterNextRender, Injector} from '@angular/core';
 import {Direction} from '@angular/cdk/bidi';
 import {_CoalescedStyleScheduler} from './coalesced-style-scheduler';
 import {StickyPositioningListener} from './sticky-position-listener';
@@ -41,6 +42,7 @@ export class StickyStyler {
   private _stickyColumnsReplayTimeout: number | null = null;
   private _cachedCellWidths: number[] = [];
   private readonly _borderCellCss: Readonly<{[d in StickyDirection]: string}>;
+  private _destroyed = false;
 
   /**
    * @param _isNativeHtmlTable Whether the sticky logic should be based on a table
@@ -64,6 +66,7 @@ export class StickyStyler {
     private _isBrowser = true,
     private readonly _needsPositionStickyOnElement = true,
     private readonly _positionListener?: StickyPositioningListener,
+    private readonly _tableInjector?: Injector,
   ) {
     this._borderCellCss = {
       'top': `${_stickCellCss}-border-elem-top`,
@@ -92,18 +95,20 @@ export class StickyStyler {
         continue;
       }
 
-      elementsToClear.push(row);
-      for (let i = 0; i < row.children.length; i++) {
-        elementsToClear.push(row.children[i] as HTMLElement);
-      }
+      elementsToClear.push(row, ...(row.children as HTMLCollectionOf<HTMLElement>));
     }
 
     // Coalesce with sticky row/column updates (and potentially other changes like column resize).
-    this._coalescedStyleScheduler.schedule(() => {
-      for (const element of elementsToClear) {
-        this._removeStickyStyle(element, stickyDirections);
-      }
-    });
+    afterNextRender(
+      {
+        write: () => {
+          for (const element of elementsToClear) {
+            this._removeStickyStyle(element, stickyDirections);
+          }
+        },
+      },
+      {injector: this._tableInjector},
+    );
   }
 
   /**
@@ -147,54 +152,67 @@ export class StickyStyler {
     }
 
     // Coalesce with sticky row updates (and potentially other changes like column resize).
-    this._coalescedStyleScheduler.schedule(() => {
-      const firstRow = rows[0];
-      const numCells = firstRow.children.length;
-      const cellWidths: number[] = this._getCellWidths(firstRow, recalculateCellWidths);
-
-      const startPositions = this._getStickyStartColumnPositions(cellWidths, stickyStartStates);
-      const endPositions = this._getStickyEndColumnPositions(cellWidths, stickyEndStates);
-
-      const lastStickyStart = stickyStartStates.lastIndexOf(true);
-      const firstStickyEnd = stickyEndStates.indexOf(true);
-
-      const isRtl = this.direction === 'rtl';
-      const start = isRtl ? 'right' : 'left';
-      const end = isRtl ? 'left' : 'right';
-
-      for (const row of rows) {
-        for (let i = 0; i < numCells; i++) {
-          const cell = row.children[i] as HTMLElement;
-          if (stickyStartStates[i]) {
-            this._addStickyStyle(cell, start, startPositions[i], i === lastStickyStart);
+    const firstRow = rows[0];
+    const numCells = firstRow.children.length;
+
+    const isRtl = this.direction === 'rtl';
+    const start = isRtl ? 'right' : 'left';
+    const end = isRtl ? 'left' : 'right';
+
+    const lastStickyStart = stickyStartStates.lastIndexOf(true);
+    const firstStickyEnd = stickyEndStates.indexOf(true);
+
+    let cellWidths: number[];
+    let startPositions: number[];
+    let endPositions: number[];
+
+    afterNextRender(
+      {
+        earlyRead: () => {
+          cellWidths = this._getCellWidths(firstRow, recalculateCellWidths);
+
+          startPositions = this._getStickyStartColumnPositions(cellWidths, stickyStartStates);
+          endPositions = this._getStickyEndColumnPositions(cellWidths, stickyEndStates);
+        },
+        write: () => {
+          for (const row of rows) {
+            for (let i = 0; i < numCells; i++) {
+              const cell = row.children[i] as HTMLElement;
+              if (stickyStartStates[i]) {
+                this._addStickyStyle(cell, start, startPositions[i], i === lastStickyStart);
+              }
+
+              if (stickyEndStates[i]) {
+                this._addStickyStyle(cell, end, endPositions[i], i === firstStickyEnd);
+              }
+            }
           }
 
-          if (stickyEndStates[i]) {
-            this._addStickyStyle(cell, end, endPositions[i], i === firstStickyEnd);
+          if (this._positionListener) {
+            this._positionListener.stickyColumnsUpdated({
+              sizes:
+                lastStickyStart === -1
+                  ? []
+                  : cellWidths
+                      .slice(0, lastStickyStart + 1)
+                      .map((width, index) => (stickyStartStates[index] ? width : null)),
+            });
+            this._positionListener.stickyEndColumnsUpdated({
+              sizes:
+                firstStickyEnd === -1
+                  ? []
+                  : cellWidths
+                      .slice(firstStickyEnd)
+                      .map((width, index) =>
+                        stickyEndStates[index + firstStickyEnd] ? width : null,
+                      )
+                      .reverse(),
+            });
           }
-        }
-      }
-
-      if (this._positionListener) {
-        this._positionListener.stickyColumnsUpdated({
-          sizes:
-            lastStickyStart === -1
-              ? []
-              : cellWidths
-                  .slice(0, lastStickyStart + 1)
-                  .map((width, index) => (stickyStartStates[index] ? width : null)),
-        });
-        this._positionListener.stickyEndColumnsUpdated({
-          sizes:
-            firstStickyEnd === -1
-              ? []
-              : cellWidths
-                  .slice(firstStickyEnd)
-                  .map((width, index) => (stickyEndStates[index + firstStickyEnd] ? width : null))
-                  .reverse(),
-        });
-      }
-    });
+        },
+      },
+      {injector: this._tableInjector},
+    );
   }
 
   /**
@@ -214,64 +232,70 @@ export class StickyStyler {
       return;
     }
 
-    // Coalesce with other sticky row updates (top/bottom), sticky columns updates
-    // (and potentially other changes like column resize).
-    this._coalescedStyleScheduler.schedule(() => {
-      // If positioning the rows to the bottom, reverse their order when evaluating the sticky
-      // position such that the last row stuck will be "bottom: 0px" and so on. Note that the
-      // sticky states need to be reversed as well.
-      const rows = position === 'bottom' ? rowsToStick.slice().reverse() : rowsToStick;
-      const states = position === 'bottom' ? stickyStates.slice().reverse() : stickyStates;
-
-      // Measure row heights all at once before adding sticky styles to reduce layout thrashing.
-      const stickyOffsets: number[] = [];
-      const stickyCellHeights: (number | undefined)[] = [];
-      const elementsToStick: HTMLElement[][] = [];
-
-      for (let rowIndex = 0, stickyOffset = 0; rowIndex < rows.length; rowIndex++) {
-        if (!states[rowIndex]) {
-          continue;
-        }
+    // If positioning the rows to the bottom, reverse their order when evaluating the sticky
+    // position such that the last row stuck will be "bottom: 0px" and so on. Note that the
+    // sticky states need to be reversed as well.
+    const rows = position === 'bottom' ? rowsToStick.slice().reverse() : rowsToStick;
+    const states = position === 'bottom' ? stickyStates.slice().reverse() : stickyStates;
 
-        stickyOffsets[rowIndex] = stickyOffset;
-        const row = rows[rowIndex];
-        elementsToStick[rowIndex] = this._isNativeHtmlTable
-          ? (Array.from(row.children) as HTMLElement[])
-          : [row];
-
-        const height = this._retrieveElementSize(row).height;
-        stickyOffset += height;
-        stickyCellHeights[rowIndex] = height;
-      }
+    // Measure row heights all at once before adding sticky styles to reduce layout thrashing.
+    const stickyOffsets: number[] = [];
+    const stickyCellHeights: (number | undefined)[] = [];
+    const elementsToStick: HTMLElement[][] = [];
 
-      const borderedRowIndex = states.lastIndexOf(true);
-
-      for (let rowIndex = 0; rowIndex < rows.length; rowIndex++) {
-        if (!states[rowIndex]) {
-          continue;
-        }
-
-        const offset = stickyOffsets[rowIndex];
-        const isBorderedRowIndex = rowIndex === borderedRowIndex;
-        for (const element of elementsToStick[rowIndex]) {
-          this._addStickyStyle(element, position, offset, isBorderedRowIndex);
-        }
-      }
+    // Coalesce with other sticky row updates (top/bottom), sticky columns updates
+    // (and potentially other changes like column resize).
+    afterNextRender(
+      {
+        earlyRead: () => {
+          for (let rowIndex = 0, stickyOffset = 0; rowIndex < rows.length; rowIndex++) {
+            if (!states[rowIndex]) {
+              continue;
+            }
+
+            stickyOffsets[rowIndex] = stickyOffset;
+            const row = rows[rowIndex];
+            elementsToStick[rowIndex] = this._isNativeHtmlTable
+              ? (Array.from(row.children) as HTMLElement[])
+              : [row];
+
+            const height = this._retrieveElementSize(row).height;
+            stickyOffset += height;
+            stickyCellHeights[rowIndex] = height;
+          }
+        },
+        write: () => {
+          const borderedRowIndex = states.lastIndexOf(true);
+
+          for (let rowIndex = 0; rowIndex < rows.length; rowIndex++) {
+            if (!states[rowIndex]) {
+              continue;
+            }
+
+            const offset = stickyOffsets[rowIndex];
+            const isBorderedRowIndex = rowIndex === borderedRowIndex;
+            for (const element of elementsToStick[rowIndex]) {
+              this._addStickyStyle(element, position, offset, isBorderedRowIndex);
+            }
+          }
 
-      if (position === 'top') {
-        this._positionListener?.stickyHeaderRowsUpdated({
-          sizes: stickyCellHeights,
-          offsets: stickyOffsets,
-          elements: elementsToStick,
-        });
-      } else {
-        this._positionListener?.stickyFooterRowsUpdated({
-          sizes: stickyCellHeights,
-          offsets: stickyOffsets,
-          elements: elementsToStick,
-        });
-      }
-    });
+          if (position === 'top') {
+            this._positionListener?.stickyHeaderRowsUpdated({
+              sizes: stickyCellHeights,
+              offsets: stickyOffsets,
+              elements: elementsToStick,
+            });
+          } else {
+            this._positionListener?.stickyFooterRowsUpdated({
+              sizes: stickyCellHeights,
+              offsets: stickyOffsets,
+              elements: elementsToStick,
+            });
+          }
+        },
+      },
+      {injector: this._tableInjector},
+    );
   }
 
   /**
@@ -286,17 +310,30 @@ export class StickyStyler {
     }
 
     // Coalesce with other sticky updates (and potentially other changes like column resize).
-    this._coalescedStyleScheduler.schedule(() => {
-      const tfoot = tableElement.querySelector('tfoot')!;
-
-      if (tfoot) {
-        if (stickyStates.some(state => !state)) {
-          this._removeStickyStyle(tfoot, ['bottom']);
-        } else {
-          this._addStickyStyle(tfoot, 'bottom', 0, false);
-        }
-      }
-    });
+    afterNextRender(
+      {
+        write: () => {
+          const tfoot = tableElement.querySelector('tfoot')!;
+
+          if (tfoot) {
+            if (stickyStates.some(state => !state)) {
+              this._removeStickyStyle(tfoot, ['bottom']);
+            } else {
+              this._addStickyStyle(tfoot, 'bottom', 0, false);
+            }
+          }
+        },
+      },
+      {injector: this._tableInjector},
+    );
+  }
+
+  destroy() {
+    if (this._stickyColumnsReplayTimeout) {
+      clearTimeout(this._stickyColumnsReplayTimeout);
+    }
+
+    this._destroyed = true;
   }
 
   /**
@@ -516,6 +553,10 @@ export class StickyStyler {
       }
 
       this._stickyColumnsReplayTimeout = setTimeout(() => {
+        if (this._destroyed) {
+          return;
+        }
+
         for (const update of this._updatedStickyColumnsParamsToReplay) {
           this.updateStickyColumns(
             update.rows,
diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts
@@ -654,6 +654,8 @@ export class CdkTable<T>
   }
 
   ngOnDestroy() {
+    this._stickyStyler?.destroy();
+
     [
       this._rowOutlet?.viewContainer,
       this._headerRowOutlet?.viewContainer,
@@ -727,14 +729,8 @@ export class CdkTable<T>
 
     this._updateNoDataRow();
 
-    afterNextRender(
-      () => {
-        this.updateStickyColumnStyles();
-      },
-      {injector: this._injector},
-    );
-
     this.contentChanged.next();
+    this.updateStickyColumnStyles();
   }
 
   /** Adds a column definition that was not included as part of the content children. */
@@ -1201,7 +1197,7 @@ export class CdkTable<T>
 
   /** Adds the sticky column styles for the rows according to the columns' stick states. */
   private _addStickyColumnStyles(rows: HTMLElement[], rowDef: BaseRowDef) {
-    const columnDefs = Array.from(rowDef.columns || []).map(columnName => {
+    const columnDefs = Array.from(rowDef?.columns || []).map(columnName => {
       const columnDef = this._columnDefsByName.get(columnName);
       if (!columnDef && (typeof ngDevMode === 'undefined' || ngDevMode)) {
         throw getTableUnknownColumnError(columnName);
@@ -1396,6 +1392,7 @@ export class CdkTable<T>
       this._platform.isBrowser,
       this.needsPositionStickyOnElement,
       this._stickyPositioningListener,
+      this._injector,
     );
     (this._dir ? this._dir.change : observableOf<Direction>())
       .pipe(takeUntil(this._onDestroy))
