From dcad4222a6720ffaac2f39817d6703df181ccad2 Mon Sep 17 00:00:00 2001 From: mcintyret Date: Fri, 2 Nov 2018 19:51:41 +0000 Subject: [PATCH] [table] Perf improvements with selections (#2005) * table performance improvements - reducing rerendering where possible * reinstate enum strings * fix compile errors * fix lint errors * fix test * cell: revert changes * code review suggestions; add forceRerenderOnSelectionChange flag * lint * default to false --- packages/table-dev-app/src/mutableTable.tsx | 4 ++ packages/table/src/common/batcher.ts | 4 +- packages/table/src/table.tsx | 75 +++++++++++++++------ 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/packages/table-dev-app/src/mutableTable.tsx b/packages/table-dev-app/src/mutableTable.tsx index b66306e4b8..df0408d18a 100644 --- a/packages/table-dev-app/src/mutableTable.tsx +++ b/packages/table-dev-app/src/mutableTable.tsx @@ -237,6 +237,7 @@ export interface IMutableTableState { scrollToRowIndex?: number; selectedFocusStyle?: FocusStyle; selectedRegionTransformPreset?: SelectedRegionTransformPreset; + selectedRegions?: IRegion[]; showCallbackLogs?: boolean; showCellsLoading?: boolean; showColumnHeadersLoading?: boolean; @@ -283,6 +284,7 @@ const DEFAULT_STATE: IMutableTableState = { scrollToRowIndex: 0, selectedFocusStyle: FocusStyle.TAB, selectedRegionTransformPreset: SelectedRegionTransformPreset.CELL, + selectedRegions: [], showCallbackLogs: true, showCellsLoading: false, showColumnHeadersLoading: false, @@ -403,6 +405,7 @@ export class MutableTable extends React.Component<{}, IMutableTableState> { rowHeaderCellRenderer={this.renderRowHeader} selectedRegionTransform={this.getSelectedRegionTransform()} selectionModes={this.getEnabledSelectionModes()} + selectedRegions={this.state.selectedRegions} styledRegionGroups={this.getStyledRegionGroups()} > {this.renderColumns()} @@ -944,6 +947,7 @@ export class MutableTable extends React.Component<{}, IMutableTableState> { private onSelection = (selectedRegions: IRegion[]) => { this.maybeLogCallback(`[onSelection] selectedRegions =`, ...selectedRegions); + this.setState({ selectedRegions }); }; private onColumnsReordered = (oldIndex: number, newIndex: number, length: number) => { diff --git a/packages/table/src/common/batcher.ts b/packages/table/src/common/batcher.ts index 8b85967b5d..216696da42 100644 --- a/packages/table/src/common/batcher.ts +++ b/packages/table/src/common/batcher.ts @@ -129,14 +129,14 @@ export class Batcher { const keysToAdd = this.setKeysDifference(this.batchArgs, this.currentObjects, addNewLimit); keysToAdd.forEach(key => (this.currentObjects[key] = callback.apply(undefined, this.batchArgs[key]))); - // set `done` to true of sets match exactly after add/remove and there + // set `done` to true if sets match exactly after add/remove and there // are no "old objects" remaining this.done = this.setHasSameKeys(this.batchArgs, this.currentObjects) && Object.keys(this.oldObjects).length === 0; } /** - * Returns true of the "current" set matches the "batch" set. + * Returns true if the "current" set matches the "batch" set. */ public isDone() { return this.done; diff --git a/packages/table/src/table.tsx b/packages/table/src/table.tsx index b870e9a1a5..bb1ff9f32e 100644 --- a/packages/table/src/table.tsx +++ b/packages/table/src/table.tsx @@ -175,6 +175,13 @@ export interface ITableProps extends IProps, IRowHeights, IColumnWidths { */ focusedCell?: IFocusedCellCoordinates; + /** + * If `true`, selection state changes will cause the component to re-render. + * If `false`, selection state is ignored when deciding to re-render. + * @default false + */ + forceRerenderOnSelectionChange?: boolean; + /** * If defined, this callback will be invoked for each cell when the user * attempts to copy a selection via `mod+c`. The returned data will be copied @@ -420,6 +427,7 @@ export class Table extends AbstractComponent { enableGhostCells: false, enableMultipleSelection: true, enableRowHeader: true, + forceRerenderOnSelectionChange: false, loadingOptions: [], minColumnWidth: 50, minRowHeight: 20, @@ -691,34 +699,54 @@ export class Table extends AbstractComponent { defaultRowHeight, enableFocusedCell, focusedCell, + forceRerenderOnSelectionChange, numRows, rowHeights, selectedRegions, selectionModes, } = nextProps; - const newChildArray = React.Children.toArray(children) as Array>; + const didChildrenChange = this.props.children !== nextProps.children; + const newChildArray = didChildrenChange + ? (React.Children.toArray(children) as Array>) + : this.childrenArray; const numCols = newChildArray.length; - // Try to maintain widths of columns by looking up the width of the - // column that had the same `ID` prop. If none is found, use the - // previous width at the same index. - const previousColumnWidths = newChildArray.map((child: React.ReactElement, index: number) => { - const mappedIndex = this.columnIdToIndex[child.props.id]; - return this.state.columnWidths[mappedIndex != null ? mappedIndex : index]; - }); - - // Make sure the width/height arrays have the correct length, but keep - // as many existing widths/heights when possible. Also, apply the - // sparse width/heights from props. + let shouldInvalidateGrid = false; let newColumnWidths = this.state.columnWidths; - newColumnWidths = Utils.arrayOfLength(newColumnWidths, numCols, defaultColumnWidth); - newColumnWidths = Utils.assignSparseValues(newColumnWidths, previousColumnWidths); - newColumnWidths = Utils.assignSparseValues(newColumnWidths, columnWidths); + if ( + defaultColumnWidth !== this.props.defaultColumnWidth || + columnWidths !== this.props.columnWidths || + didChildrenChange + ) { + // Try to maintain widths of columns by looking up the width of the + // column that had the same `ID` prop. If none is found, use the + // previous width at the same index. + const previousColumnWidths = newChildArray.map((child: React.ReactElement, index: number) => { + const mappedIndex = this.columnIdToIndex[child.props.id]; + return this.state.columnWidths[mappedIndex != null ? mappedIndex : index]; + }); + + // Make sure the width/height arrays have the correct length, but keep + // as many existing widths/heights as possible. Also, apply the + // sparse width/heights from props. + newColumnWidths = Utils.arrayOfLength(newColumnWidths, numCols, defaultColumnWidth); + newColumnWidths = Utils.assignSparseValues(newColumnWidths, previousColumnWidths); + newColumnWidths = Utils.assignSparseValues(newColumnWidths, columnWidths); + + shouldInvalidateGrid = true; + } let newRowHeights = this.state.rowHeights; - newRowHeights = Utils.arrayOfLength(newRowHeights, numRows, defaultRowHeight); - newRowHeights = Utils.assignSparseValues(newRowHeights, rowHeights); + if ( + defaultRowHeight !== this.props.defaultRowHeight || + rowHeights !== this.props.rowHeights || + numRows !== this.props.numRows + ) { + newRowHeights = Utils.arrayOfLength(newRowHeights, numRows, defaultRowHeight); + newRowHeights = Utils.assignSparseValues(newRowHeights, rowHeights); + shouldInvalidateGrid = true; + } if ( !CoreUtils.arraysEqual(newColumnWidths, this.state.columnWidths) || @@ -730,6 +758,9 @@ export class Table extends AbstractComponent { } let newSelectedRegions = selectedRegions; + if (forceRerenderOnSelectionChange && newSelectedRegions !== this.props.selectedRegions) { + shouldInvalidateGrid = true; + } if (selectedRegions == null) { // if we're in uncontrolled mode, filter out all selected regions that don't // fit in the current new table dimensions @@ -749,9 +780,13 @@ export class Table extends AbstractComponent { newSelectedRegions, ); - this.childrenArray = newChildArray; - this.columnIdToIndex = Table.createColumnIdIndex(this.childrenArray); - this.invalidateGrid(); + if (didChildrenChange) { + this.childrenArray = newChildArray; + this.columnIdToIndex = Table.createColumnIdIndex(this.childrenArray); + } + if (shouldInvalidateGrid) { + this.invalidateGrid(); + } this.setState({ columnWidths: newColumnWidths, focusedCell: newFocusedCell,