fix(table): fixed performance issue (#241)

* fix(table): fixed performance issue * refactor(table): adress comment * fix(table): address comment * test(table): fix test
apache-superset · Oct 31, 2019 · dfdf263 · dfdf263
1 parent 1865bd5
commit dfdf263
Show file tree

Hide file tree

Showing 10 changed files with 143 additions and 54 deletions.
diff --git a/packages/superset-ui-plugin-chart-table/package.json b/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/packages/superset-ui-plugin-chart-table/src/Table.tsx b/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<InternalTableProps, TableState> {
+  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<InternalTableProps, TableState> {
 
     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<InternalTableProps, TableState> {
     });
 
     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<InternalTableProps, TableState> {
     const tableHeight = includeSearch ? height - SEARCH_BAR_HEIGHT : height;
 
     return (
-      <div className={cx(styles.container)}>
+      <>
         {includeSearch && (
           <div className={cx(styles.searchBar)}>
             <div className={cx(styles.searchBox)}>
@@ -242,28 +270,32 @@ class TableVis extends React.PureComponent<InternalTableProps, TableState> {
               />
             </div>
             <Text small>
-              Showing {dataToRender.length} out of {data.length} rows
+              Showing {dataToRender.length}/{data.length} rows
             </Text>
           </div>
         )}
-        <DataTable
-          data={dataToRender}
-          keys={keys}
-          columnMetadata={columnMetadata}
-          zebra
-          dynamicRowHeight
-          renderers={renderers}
-          height={tableHeight}
-          width={Math.max(calculatedWidth, width)}
-        />
-      </div>
+        <div className={cx(styles.container)}>
+          <DataTable
+            data={dataToRender}
+            keys={keys}
+            columnMetadata={columnMetadata}
+            zebra
+            dynamicRowHeight
+            rowHeight="micro"
+            renderers={renderers}
+            height={tableHeight}
+            width={Math.max(columnWidthInfo.totalWidth, width)}
+          />
+        </div>
+      </>
     );
   }
 }
 
 export default withStyles(({ unit }) => ({
   container: {
     display: 'grid',
+    overflowX: 'scroll',
   },
   searchBar: {
     alignItems: 'baseline',

diff --git a/packages/superset-ui-plugin-chart-table/src/legacy/transformProps.ts b/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/packages/superset-ui-plugin-chart-table/src/processColumns.ts b/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/packages/superset-ui-plugin-chart-table/src/processData.ts b/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/packages/superset-ui-plugin-chart-table/src/processMetrics.ts b/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/packages/superset-ui-plugin-chart-table/src/renderer.tsx b/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 = ({
     >
       <Parent>
         {column.format ? (
-          column.format(value)
+          column.format.format(value as any)
         ) : (
           // eslint-disable-next-line react/no-danger
           <div dangerouslySetInnerHTML={{ __html: value as string }} />

diff --git a/packages/superset-ui-plugin-chart-table/src/transformProps.ts b/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/packages/superset-ui-plugin-chart-table/test/processData.test.ts b/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/packages/superset-ui-plugin-chart-table/test/processMetrics.test.ts b/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,