perf(plugin-chart-table): Add memoization to avoid rerenders (#19976)

* perf(plugin-chart-table): Add memoization to avoid rerenders * Fix typing
apache · May 13, 2022 · 0f68dee · 0f68dee
1 parent 35e6e27
commit 0f68dee
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 45 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/packages/superset-ui-chart-controls/package.json b/superset-frontend/packages/superset-ui-chart-controls/package.json
@@ -41,6 +41,7 @@
     "ace-builds": "^1.4.14",
     "antd": "^4.9.4",
     "brace": "^0.11.1",
+    "memoize-one": "^5.1.1",
     "react": "^16.13.1",
     "react-ace": "^9.4.4",
     "react-dom": "^16.13.1"

diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts b/superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts
@@ -16,6 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import memoizeOne from 'memoize-one';
 import { DataRecord } from '@superset-ui/core';
 import {
   ColorFormatters,
@@ -180,30 +181,32 @@ export const getColorFunction = (
   };
 };
 
-export const getColorFormatters = (
-  columnConfig: ConditionalFormattingConfig[] | undefined,
-  data: DataRecord[],
-) =>
-  columnConfig?.reduce(
-    (acc: ColorFormatters, config: ConditionalFormattingConfig) => {
-      if (
-        config?.column !== undefined &&
-        (config?.operator === COMPARATOR.NONE ||
-          (config?.operator !== undefined &&
-            (MULTIPLE_VALUE_COMPARATORS.includes(config?.operator)
-              ? config?.targetValueLeft !== undefined &&
-                config?.targetValueRight !== undefined
-              : config?.targetValue !== undefined)))
-      ) {
-        acc.push({
-          column: config?.column,
-          getColorFromValue: getColorFunction(
-            config,
-            data.map(row => row[config.column!] as number),
-          ),
-        });
-      }
-      return acc;
-    },
-    [],
-  ) ?? [];
+export const getColorFormatters = memoizeOne(
+  (
+    columnConfig: ConditionalFormattingConfig[] | undefined,
+    data: DataRecord[],
+  ) =>
+    columnConfig?.reduce(
+      (acc: ColorFormatters, config: ConditionalFormattingConfig) => {
+        if (
+          config?.column !== undefined &&
+          (config?.operator === COMPARATOR.NONE ||
+            (config?.operator !== undefined &&
+              (MULTIPLE_VALUE_COMPARATORS.includes(config?.operator)
+                ? config?.targetValueLeft !== undefined &&
+                  config?.targetValueRight !== undefined
+                : config?.targetValue !== undefined)))
+        ) {
+          acc.push({
+            column: config?.column,
+            getColorFromValue: getColorFunction(
+              config,
+              data.map(row => row[config.column!] as number),
+            ),
+          });
+        }
+        return acc;
+      },
+      [],
+    ) ?? [],
+);
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/index.ts b/superset-frontend/packages/superset-ui-core/src/utils/index.ts
@@ -28,3 +28,4 @@ export { default as logging } from './logging';
 export { default as removeDuplicates } from './removeDuplicates';
 export * from './featureFlags';
 export * from './random';
+export * from './typedMemo';
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/typedMemo.ts b/superset-frontend/packages/superset-ui-core/src/utils/typedMemo.ts
@@ -0,0 +1,21 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { memo } from 'react';
+
+export const typedMemo: <T>(c: T) => T = memo;
diff --git a/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/superset-frontend/plugins/plugin-chart-table/src/DataTable/DataTable.tsx
@@ -36,6 +36,7 @@ import {
   Row,
 } from 'react-table';
 import { matchSorter, rankings } from 'match-sorter';
+import { typedMemo } from '@superset-ui/core';
 import GlobalFilter, { GlobalFilterProps } from './components/GlobalFilter';
 import SelectPageSize, {
   SelectPageSizeProps,
@@ -74,7 +75,7 @@ const sortTypes = {
 };
 
 // Be sure to pass our updateMyData and the skipReset option
-export default function DataTable<D extends object>({
+export default typedMemo(function DataTable<D extends object>({
   tableClassName,
   columns,
   data,
@@ -124,7 +125,7 @@ export default function DataTable<D extends object>({
   const defaultGetTableSize = useCallback(() => {
     if (wrapperRef.current) {
       // `initialWidth` and `initialHeight` could be also parameters like `100%`
-      // `Number` reaturns `NaN` on them, then we fallback to computed size
+      // `Number` returns `NaN` on them, then we fallback to computed size
       const width = Number(initialWidth) || wrapperRef.current.clientWidth;
       const height =
         (Number(initialHeight) || wrapperRef.current.clientHeight) -
@@ -287,7 +288,7 @@ export default function DataTable<D extends object>({
   let resultCurrentPage = pageIndex;
   let resultOnPageChange: (page: number) => void = gotoPage;
   if (serverPagination) {
-    const serverPageSize = serverPaginationData.pageSize ?? initialPageSize;
+    const serverPageSize = serverPaginationData?.pageSize ?? initialPageSize;
     resultPageCount = Math.ceil(rowCount / serverPageSize);
     if (!Number.isFinite(resultPageCount)) {
       resultPageCount = 0;
@@ -299,7 +300,7 @@ export default function DataTable<D extends object>({
     if (foundPageSizeIndex === -1) {
       resultCurrentPageSize = 0;
     }
-    resultCurrentPage = serverPaginationData.currentPage ?? 0;
+    resultCurrentPage = serverPaginationData?.currentPage ?? 0;
     resultOnPageChange = (pageNumber: number) =>
       onServerPaginationChange(pageNumber, serverPageSize);
   }
@@ -354,4 +355,4 @@ export default function DataTable<D extends object>({
       ) : null}
     </div>
   );
-}
+});
diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
@@ -162,6 +162,9 @@ function SelectPageSize({
   );
 }
 
+const getNoResultsMessage = (filter: string) =>
+  t(filter ? 'No matching records found' : 'No records found');
+
 export default function TableChart<D extends DataRecord = DataRecord>(
   props: TableChartTransformedProps<D> & {
     sticky?: DataTableProps<D>['sticky'];
@@ -474,12 +477,12 @@ export default function TableChart<D extends DataRecord = DataRecord>(
     [columnsMeta, getColumnConfigs],
   );
 
-  const handleServerPaginationChange = (
-    pageNumber: number,
-    pageSize: number,
-  ) => {
-    updateExternalFormData(setDataMask, pageNumber, pageSize);
-  };
+  const handleServerPaginationChange = useCallback(
+    (pageNumber: number, pageSize: number) => {
+      updateExternalFormData(setDataMask, pageNumber, pageSize);
+    },
+    [setDataMask],
+  );
 
   return (
     <Styles>
@@ -497,9 +500,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
         onServerPaginationChange={handleServerPaginationChange}
         // 9 page items in > 340px works well even for 100+ pages
         maxPageItemCount={width > 340 ? 9 : 7}
-        noResults={(filter: string) =>
-          t(filter ? 'No matching records found' : 'No records found')
-        }
+        noResults={getNoResultsMessage}
         searchInput={includeSearch && SearchInput}
         selectPageSize={pageSize !== null && SelectPageSize}
         // not in use in Superset, but needed for unit tests

diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
@@ -31,7 +31,10 @@ import {
   TimeFormats,
   TimeFormatter,
 } from '@superset-ui/core';
-import { getColorFormatters } from '@superset-ui/chart-controls';
+import {
+  ColorFormatters,
+  getColorFormatters,
+} from '@superset-ui/chart-controls';
 
 import isEqualColumns from './utils/isEqualColumns';
 import DateWithFormatter from './utils/DateWithFormatter';
@@ -189,6 +192,8 @@ const getPageSize = (
   return numRecords * numColumns > 5000 ? 200 : 0;
 };
 
+const defaultServerPaginationData = {};
+const defaultColorFormatters = [] as ColorFormatters;
 const transformProps = (
   chartProps: TableChartProps,
 ): TableChartTransformedProps => {
@@ -198,7 +203,7 @@ const transformProps = (
     rawFormData: formData,
     queriesData = [],
     filterState,
-    ownState: serverPaginationData = {},
+    ownState: serverPaginationData,
     hooks: { onAddFilter: onChangeFilter, setDataMask = () => {} },
   } = chartProps;
 
@@ -237,7 +242,7 @@ const transformProps = (
       ? totalQuery?.data[0]
       : undefined;
   const columnColorFormatters =
-    getColorFormatters(conditionalFormatting, data) ?? [];
+    getColorFormatters(conditionalFormatting, data) ?? defaultColorFormatters;
 
   return {
     height,
@@ -249,7 +254,9 @@ const transformProps = (
     serverPagination,
     metrics,
     percentMetrics,
-    serverPaginationData,
+    serverPaginationData: serverPagination
+      ? serverPaginationData
+      : defaultServerPaginationData,
     setDataMask,
     alignPositiveNegative,
     colorPositiveNegative,