From c68f9d7fa8990d2010b3a824de912e43ace1bf58 Mon Sep 17 00:00:00 2001 From: Conglei Date: Thu, 31 Oct 2019 15:33:54 -0700 Subject: [PATCH] fix(table): fixed performance issue (#241) * fix(table): fixed performance issue * refactor(table): adress comment * fix(table): address comment * test(table): fix test --- .../package.json | 3 +- .../src/Table.tsx | 74 +++++++++++++------ .../src/legacy/transformProps.ts | 10 ++- .../src/processColumns.ts | 34 ++++++--- .../src/processData.ts | 29 ++++++-- .../src/processMetrics.ts | 25 +++++-- .../src/renderer.tsx | 6 +- .../src/transformProps.ts | 10 ++- .../test/processData.test.ts | 3 +- .../test/processMetrics.test.ts | 3 +- 10 files changed, 143 insertions(+), 54 deletions(-) diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/package.json b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/package.json index a8c0937873..e56af7b38d 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/package.json +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/package.json @@ -29,7 +29,8 @@ "@airbnb/lunar": "^2.35.0", "@airbnb/lunar-icons": "^2.1.4", "@types/dompurify": "^0.0.33", - "dompurify": "^2.0.6" + "dompurify": "^2.0.6", + "reselect": "^4.0.0" }, "peerDependencies": { "@superset-ui/chart": "^0.12.0", diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/Table.tsx b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/Table.tsx index 9643210d5f..58309b424e 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/Table.tsx +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/Table.tsx @@ -5,6 +5,7 @@ import Input from '@airbnb/lunar/lib/components/Input'; import withStyles, { WithStylesProps } from '@airbnb/lunar/lib/composers/withStyles'; import { Renderers, ParentRow, ColumnMetadata } from '@airbnb/lunar/lib/components/DataTable/types'; import dompurify from 'dompurify'; +import { createSelector } from 'reselect'; import { getRenderer, ColumnType, Cell } from './renderer'; type Props = { @@ -71,7 +72,38 @@ function getText(value: unknown) { return String(value); } +type columnWidthMetaDataType = { + [key: string]: { + maxWidth: number; + width: number; + }; +}; + class TableVis extends React.PureComponent { + columnWidthSelector = createSelector( + (data: ParentRow[]) => data, + data => { + const keys = data && data.length > 0 ? Object.keys(data[0].data) : []; + let totalWidth = 0; + const columnWidthMetaData: columnWidthMetaDataType = {}; + + keys.forEach(key => { + const maxLength = Math.max(...data.map(d => getText(d.data[key]).length), key.length); + const stringWidth = maxLength * CHAR_WIDTH + CELL_PADDING; + columnWidthMetaData[key] = { + maxWidth: MAX_COLUMN_WIDTH, + width: stringWidth, + }; + totalWidth += Math.min(stringWidth, MAX_COLUMN_WIDTH); + }); + + return { + columnWidthMetaData, + totalWidth, + }; + }, + ); + static defaultProps = defaultProps; constructor(props: InternalTableProps) { @@ -176,9 +208,8 @@ class TableVis extends React.PureComponent { const { filteredRows, searchKeyword } = this.state; - const renderers: Renderers = {}; - const dataToRender = searchKeyword === '' ? data : filteredRows; + const renderers: Renderers = {}; const columnMetadata: ColumnMetadata = {}; columns.forEach(column => { @@ -198,16 +229,13 @@ class TableVis extends React.PureComponent { }); const keys = dataToRender && dataToRender.length > 0 ? Object.keys(dataToRender[0].data) : []; - let calculatedWidth = 0; + const columnWidthInfo = this.columnWidthSelector(data); + keys.forEach(key => { - const maxLength = Math.max(...data.map(d => getText(d.data[key]).length), key.length); - const stringWidth = maxLength * CHAR_WIDTH + CELL_PADDING; columnMetadata[key] = { - maxWidth: MAX_COLUMN_WIDTH, - width: stringWidth, + ...columnWidthInfo.columnWidthMetaData[key], ...columnMetadata[key], }; - calculatedWidth += Math.min(stringWidth, MAX_COLUMN_WIDTH); if (!renderers[key]) { renderers[key] = getRenderer({ @@ -228,7 +256,7 @@ class TableVis extends React.PureComponent { const tableHeight = includeSearch ? height - SEARCH_BAR_HEIGHT : height; return ( -
+ <> {includeSearch && (
@@ -242,21 +270,24 @@ class TableVis extends React.PureComponent { />
- Showing {dataToRender.length} out of {data.length} rows + Showing {dataToRender.length}/{data.length} rows
)} - -
+
+ +
+ ); } } @@ -264,6 +295,7 @@ class TableVis extends React.PureComponent { export default withStyles(({ unit }) => ({ container: { display: 'grid', + overflowX: 'scroll', }, searchBar: { alignItems: 'baseline', diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/legacy/transformProps.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/legacy/transformProps.ts index 2fe712f83c..82d5c85d1d 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/legacy/transformProps.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/legacy/transformProps.ts @@ -19,9 +19,13 @@ /* eslint-disable sort-keys */ import { ChartProps } from '@superset-ui/chart'; -import processColumns from '../processColumns'; -import processMetrics from '../processMetrics'; -import processData from '../processData'; +import getProcessColumnsFunction from '../processColumns'; +import getProcessMetricsFunction from '../processMetrics'; +import getProcessDataFunction from '../processData'; + +const processColumns = getProcessColumnsFunction(); +const processMetrics = getProcessMetricsFunction(); +const processData = getProcessDataFunction(); const NOOP = () => {}; diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processColumns.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processColumns.ts index a29832eb5c..6bc7f78a9b 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processColumns.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processColumns.ts @@ -1,22 +1,25 @@ import { getNumberFormatter, NumberFormats, NumberFormatter } from '@superset-ui/number-format'; import { getTimeFormatter, TimeFormatter } from '@superset-ui/time-format'; +import { createSelector } from 'reselect'; import { PlainObject } from './types'; const DTTM_ALIAS = '__timestamp'; -export default function processColumns({ - columns, - metrics, - records, - tableTimestampFormat, - datasource, -}: { +type inputType = { columns: string[]; metrics: string[]; records: any[]; tableTimestampFormat: string; datasource: PlainObject; -}) { +}; + +function processColumns( + columns: string[], + metrics: string[], + records: any[], + tableTimestampFormat: string, + datasource: PlainObject, +) { const { columnFormats, verboseMap } = datasource; const dataArray: { @@ -51,7 +54,7 @@ export default function processColumns({ let label = verboseMap[key]; const formatString = columnFormats && columnFormats[key]; let formatFunction: NumberFormatter | TimeFormatter | undefined; - let type = 'string'; + let type: 'string' | 'metric' = 'string'; if (key === DTTM_ALIAS) { formatFunction = tsFormatter; @@ -90,3 +93,16 @@ export default function processColumns({ return processedColumns; } + +const getCreateSelectorFunction = () => + createSelector( + (data: inputType) => data.columns, + data => data.metrics, + data => data.records, + data => data.tableTimestampFormat, + data => data.datasource, + (columns, metrics, records, tableTimestampFormat, datasource) => + processColumns(columns, metrics, records, tableTimestampFormat, datasource), + ); + +export default getCreateSelectorFunction; diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processData.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processData.ts index 8c0a295896..67a30ad9b6 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processData.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processData.ts @@ -1,17 +1,20 @@ import { QueryFormDataMetric, AdhocMetric } from '@superset-ui/query'; +import { createSelector } from 'reselect'; import { PlainObject } from './types'; -export default function processData({ - timeseriesLimitMetric, - orderDesc, - records, - metrics, -}: { +type inputType = { timeseriesLimitMetric: QueryFormDataMetric; orderDesc: boolean; records: PlainObject[]; metrics: string[]; -}) { +}; + +function processData( + timeseriesLimitMetric: QueryFormDataMetric, + orderDesc: boolean, + records: PlainObject[], + metrics: string[], +) { const sortByKey = timeseriesLimitMetric && ((timeseriesLimitMetric as AdhocMetric).label || (timeseriesLimitMetric as string)); @@ -37,3 +40,15 @@ export default function processData({ : row => ({ data: row }), ); } + +const getCreateSelectorFunction = () => + createSelector( + (data: inputType) => data.timeseriesLimitMetric, + data => data.orderDesc, + data => data.records, + data => data.metrics, + (timeseriesLimitMetric, orderDesc, records, metrics) => + processData(timeseriesLimitMetric, orderDesc, records, metrics), + ); + +export default getCreateSelectorFunction; diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processMetrics.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processMetrics.ts index 91f7be2f4d..53f41dd8b1 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processMetrics.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/processMetrics.ts @@ -1,15 +1,18 @@ import { QueryFormDataMetric, AdhocMetric } from '@superset-ui/query'; +import { createSelector } from 'reselect'; import { PlainObject } from './types'; -export default function processMetrics({ - metrics, - percentMetrics, - records, -}: { +type inputType = { metrics: QueryFormDataMetric[]; percentMetrics: QueryFormDataMetric[]; records: PlainObject[]; -}) { +}; + +function processMetrics( + metrics: QueryFormDataMetric[], + percentMetrics: QueryFormDataMetric[], + records: PlainObject[], +) { const processedMetrics = (metrics || []).map(m => (m as AdhocMetric).label || (m as string)); const processedPercentMetrics = (percentMetrics || []) @@ -20,3 +23,13 @@ export default function processMetrics({ .concat(processedPercentMetrics) .filter(m => typeof records[0][m] === 'number'); } + +const getCreateSelectorFunction = () => + createSelector( + (data: inputType) => data.metrics, + data => data.percentMetrics, + data => data.records, + (metrics, percentMetrics, records) => processMetrics(metrics, percentMetrics, records), + ); + +export default getCreateSelectorFunction; diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/renderer.tsx b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/renderer.tsx index bb3e75426b..bccc2e95ed 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/renderer.tsx +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/renderer.tsx @@ -5,6 +5,8 @@ import React, { CSSProperties } from 'react'; import { HEIGHT_TO_PX } from '@airbnb/lunar/lib/components/DataTable/constants'; import { RendererProps } from '@airbnb/lunar/lib/components/DataTable/types'; +import { NumberFormatter } from '@superset-ui/number-format'; +import { TimeFormatter } from '@superset-ui/time-format'; import dompurify from 'dompurify'; const NEGATIVE_COLOR = '#FFA8A8'; @@ -16,7 +18,7 @@ export const heightType = 'micro'; export type ColumnType = { key: string; label: string; - format?: (value: any) => string; + format?: NumberFormatter | TimeFormatter | undefined; type: 'metric' | 'string'; maxValue?: number; minValue?: number; @@ -132,7 +134,7 @@ export const getRenderer = ({ > {column.format ? ( - column.format(value) + column.format.format(value as any) ) : ( // eslint-disable-next-line react/no-danger
diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/transformProps.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/transformProps.ts index f5dc535cca..79a2208110 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/transformProps.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/src/transformProps.ts @@ -19,9 +19,13 @@ import { ChartProps } from '@superset-ui/chart'; import { QueryFormDataMetric, AdhocMetric } from '@superset-ui/query'; -import processColumns from './processColumns'; -import processMetrics from './processMetrics'; -import processData from './processData'; +import getProcessColumnsFunction from './processColumns'; +import getProcessMetricsFunction from './processMetrics'; +import getProcessDataFunction from './processData'; + +const processColumns = getProcessColumnsFunction(); +const processMetrics = getProcessMetricsFunction(); +const processData = getProcessDataFunction(); const DTTM_ALIAS = '__timestamp'; diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processData.test.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processData.test.ts index 97661ba10e..7540e33e35 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processData.test.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processData.test.ts @@ -1,6 +1,7 @@ -import processData from '../src/processData'; +import getProcessDataFunction from '../src/processData'; describe('processData', () => { + const processData = getProcessDataFunction(); const timeseriesLimitMetric = 'a'; const orderDesc = true; const records = [ diff --git a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processMetrics.test.ts b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processMetrics.test.ts index 448880ffc7..2c6a7c2e38 100644 --- a/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processMetrics.test.ts +++ b/plugins/superset-ui-plugins/packages/superset-ui-plugin-chart-table/test/processMetrics.test.ts @@ -1,6 +1,7 @@ -import processMetrics from '../src/processMetrics'; +import getProcessMetricsFunction from '../src/processMetrics'; describe('processData', () => { + const processMetrics = getProcessMetricsFunction(); const records = [ { a: 1,