[v4] [table] feat: experimental cellRendererDependencies prop (#5162)

packages/table/src/common/errors.ts

@@ -39,3 +39,5 @@ export const TABLE_NUM_COLUMNS_COLUMN_WIDTHS_MISMATCH =
     ns + ` Table requires columnWidths.length to equal the number of <Column>s if provided.`;
 
 export const TABLE_UNMOUNTED_RESIZE_WARNING = `${ns} Table resize method called while component is unmounted, this is a no-op.`;
+
+export const TABLE_INVALID_CELL_RENDERER_DEPS = `${ns} cellRendererDependencies should either always be defined or undefined; this feature cannot be enabled during the component lifecycle`;

packages/table/src/index.ts

@@ -91,4 +91,4 @@ export { ITableProps, TableProps } from "./tableProps";
 
 export { Table } from "./table";
 
-export { Table2 } from "./table2";
+export { Table2, Table2Props } from "./table2";

packages/table/src/table2.tsx

@@ -59,7 +59,18 @@ import type { TableProps, TablePropsDefaults, TablePropsWithDefaults } from "./t
 import type { TableState, TableSnapshot } from "./tableState";
 import { clampNumFrozenColumns, clampNumFrozenRows, hasLoadingOption } from "./tableUtils";
 
-export class Table2 extends AbstractComponent2<TableProps, TableState, TableSnapshot> {
+export interface Table2Props extends TableProps {
+    /**
+     * Warning: experimental feature!
+     *
+     * This dependency list may be used to trigger a re-render of all cells when one of its elements changes
+     * (compared using shallow equality checks). This is done by invalidating the grid, which forces
+     * TableQuadrantStack to re-render.
+     */
+    cellRendererDependencies?: React.DependencyList;
+}
+
+export class Table2 extends AbstractComponent2<Table2Props, TableState, TableSnapshot> {
     public static displayName = `${DISPLAYNAME_PREFIX}.Table2`;
 
     public static defaultProps: TablePropsDefaults = {
@@ -419,7 +430,7 @@ export class Table2 extends AbstractComponent2<TableProps, TableState, TableSnap
         };
     }
 
-    public shouldComponentUpdate(nextProps: TableProps, nextState: TableState) {
+    public shouldComponentUpdate(nextProps: Table2Props, nextState: TableState) {
         const propKeysDenylist = { exclude: Table2.SHALLOW_COMPARE_PROP_KEYS_DENYLIST };
         const stateKeysDenylist = { exclude: Table2.SHALLOW_COMPARE_STATE_KEYS_DENYLIST };
 
@@ -542,7 +553,7 @@ export class Table2 extends AbstractComponent2<TableProps, TableState, TableSnap
         this.didCompletelyMount = false;
     }
 
-    public componentDidUpdate(prevProps: TableProps, prevState: TableState) {
+    public componentDidUpdate(prevProps: Table2Props, prevState: TableState) {
         super.componentDidUpdate(prevProps, prevState);
         this.hotkeysImpl.setState(this.state);
         this.hotkeysImpl.setProps(this.props);
@@ -552,10 +563,28 @@ export class Table2 extends AbstractComponent2<TableProps, TableState, TableSnap
             this.state.childrenArray,
         );
 
+        if (this.props.cellRendererDependencies !== undefined && prevProps.cellRendererDependencies === undefined) {
+            console.error(Errors.TABLE_INVALID_CELL_RENDERER_DEPS);
+        }
+
+        const didCellRendererDependenciesChange =
+            this.props.cellRendererDependencies !== undefined &&
+            this.props.cellRendererDependencies.some(
+                (dep, index) => dep !== (prevProps.cellRendererDependencies ?? [])[index],
+            );
+
+        const didColumnWidthsChange =
+            this.props.columnWidths !== undefined &&
+            !Utils.compareSparseArrays(this.props.columnWidths, prevState.columnWidths);
+        const didRowHeightsChange =
+            this.props.rowHeights !== undefined &&
+            !Utils.compareSparseArrays(this.props.rowHeights, prevState.rowHeights);
+
         const shouldInvalidateGrid =
             didChildrenChange ||
-            !Utils.compareSparseArrays(this.props.columnWidths, prevState.columnWidths) ||
-            !Utils.compareSparseArrays(this.props.rowHeights, prevState.rowHeights) ||
+            didCellRendererDependenciesChange ||
+            didColumnWidthsChange ||
+            didRowHeightsChange ||
             this.props.numRows !== prevProps.numRows ||
             (this.props.forceRerenderOnSelectionChange && this.props.selectedRegions !== prevProps.selectedRegions);
 
@@ -577,6 +606,11 @@ export class Table2 extends AbstractComponent2<TableProps, TableState, TableSnap
         if (shouldInvalidateHotkeys) {
             this.hotkeys = getHotkeysFromProps(this.props as TablePropsWithDefaults, this.hotkeysImpl);
         }
+
+        if (didCellRendererDependenciesChange) {
+            // force an update with the new grid
+            this.forceUpdate();
+        }
     }
 
     protected validateProps(props: TableProps) {
@@ -1078,11 +1112,18 @@ export class Table2 extends AbstractComponent2<TableProps, TableState, TableSnap
     };
 
     private handleCompleteRender = () => {
-        // the first onCompleteRender is triggered before the viewportRect is
-        // defined and the second after the viewportRect has been set. the cells
+        // The first onCompleteRender is triggered before the viewportRect is
+        // defined and the second after the viewportRect has been set. The cells
         // will only actually render once the viewportRect is defined though, so
         // we defer invoking onCompleteRender until that check passes.
-        if (this.state.viewportRect != null && this.state.didHeadersMount) {
+
+        // Additional note: we run into an unfortunate race condition between the order of execution
+        // of this callback and this.handleHeaderMounted(...). The setState() call in the latter
+        // does not update this.state quickly enough for us to query for the new state here, so instead
+        // we read the private member variables which are the dependent parts of that "didHeadersMount"
+        // state.
+        const didHeadersMount = this.didColumnHeaderMount && this.didRowHeaderMount;
+        if (this.state.viewportRect != null && didHeadersMount) {
             this.props.onCompleteRender?.();
             this.didCompletelyMount = true;
         }

packages/table/test/table2Tests.tsx

@@ -703,7 +703,7 @@ describe("<Table2>", function (this) {
                     <Column cellRenderer={renderDummyCell} />
                 </Table2>,
             );
-            expect(onCompleteRenderSpy.callCount, "call count on mount").to.equal(1);
+            expect(onCompleteRenderSpy.callCount, "call count on mount").to.equal(2);
         });
 
         it("triggers immediately on mount/update with RenderMode.BATCH for very small batches", () => {
@@ -717,9 +717,9 @@ describe("<Table2>", function (this) {
                 </Table2>,
             );
 
-            expect(onCompleteRenderSpy.callCount, "call count on mount").to.equal(1);
+            expect(onCompleteRenderSpy.callCount, "call count on mount").to.equal(2);
             table.setProps({ numRows: 2 }); // still small enough to fit in one batch
-            expect(onCompleteRenderSpy.callCount, "call count on update").to.equal(2);
+            expect(onCompleteRenderSpy.callCount, "call count on update").to.equal(3);
         });
     });
 
@@ -2068,6 +2068,54 @@ describe("<Table2>", function (this) {
         }
     });
 
+    describe("EXPERIMENTAL: cellRendererDependencies", () => {
+        it("Does not re-render cells when dependencies don't change", () => {
+            const dependencies: any[] = ["stable"];
+            const cellRenderer = sinon.stub().returns(<Cell>foo</Cell>);
+            const table = mount(
+                <Table2 numRows={1} cellRendererDependencies={dependencies}>
+                    <Column name="Column0" cellRenderer={cellRenderer} />
+                </Table2>,
+            );
+            const originalCallCount = cellRenderer.callCount;
+
+            // replace the single dependency with a shallow-equal value
+            dependencies[0] = "stable";
+            table.setProps({ cellRendererDependencies: dependencies });
+
+            expect(cellRenderer.callCount).to.be.equal(
+                originalCallCount,
+                "cellRenderer should not have been called again when cellRendererDependencies are the same",
+            );
+        });
+
+        it("Does re-render cells when dependencies change", () => {
+            const dependencies: string[] = ["some data from external store"];
+            const cellRenderer = () => <Cell>{dependencies[0]}</Cell>;
+            const table = mount(
+                <Table2 numRows={1} cellRendererDependencies={dependencies.slice()}>
+                    <Column name="Column0" cellRenderer={cellRenderer} />
+                </Table2>,
+            );
+            expect(getFirstCellText()).to.equal(dependencies[0]);
+
+            // replace the single dependency with a shallow-equal value
+            dependencies[0] = "new data from external store";
+            table.setProps({ cellRendererDependencies: dependencies.slice() });
+            expect(getFirstCellText()).to.equal(
+                dependencies[0],
+                "cellRenderer should have been called again when cellRendererDependencies changed",
+            );
+
+            function getFirstCellText() {
+                return table
+                    .find(`.${Classes.rowCellIndexClass(0)}.${Classes.columnCellIndexClass(0)}`)
+                    .hostNodes()
+                    .text();
+            }
+        });
+    });
+
     function renderDummyCell() {
         return <Cell>gg</Cell>;
     }