From dba0ebe0f06c1ef298069724ff1119bb94444d40 Mon Sep 17 00:00:00 2001 From: alexd-bes <129009580+alexd-bes@users.noreply.github.com> Date: Fri, 16 Feb 2024 09:30:22 +1300 Subject: [PATCH] FIx issue with non-memoized table data --- .../dashboard-item/charts/common.ts | 4 +-- .../src/utils/getChartTableData.tsx | 28 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/types/src/types/models-extra/dashboard-item/charts/common.ts b/packages/types/src/types/models-extra/dashboard-item/charts/common.ts index 2e79b97777..a1de0e4df8 100644 --- a/packages/types/src/types/models-extra/dashboard-item/charts/common.ts +++ b/packages/types/src/types/models-extra/dashboard-item/charts/common.ts @@ -64,8 +64,8 @@ export type BaseChartConfig = BaseConfig & { renderLegendForOneItem?: boolean; }; -export const isChartConfig = (config: BaseConfig): config is BaseChartConfig => { - return 'type' in config && config.type === 'chart'; +export const isChartConfig = (config?: BaseConfig): config is BaseChartConfig => { + return (config && 'type' in config && config.type === 'chart') ?? false; }; export type CartesianChartPresentationOptions = ExportPresentationOptions & { diff --git a/packages/ui-chart-components/src/utils/getChartTableData.tsx b/packages/ui-chart-components/src/utils/getChartTableData.tsx index 1002b6d899..b83d43abb6 100644 --- a/packages/ui-chart-components/src/utils/getChartTableData.tsx +++ b/packages/ui-chart-components/src/utils/getChartTableData.tsx @@ -5,7 +5,7 @@ import React, { useMemo } from 'react'; import styled from 'styled-components'; import { formatDataValueByType } from '@tupaia/utils'; -import { ValueType, ChartType } from '@tupaia/types'; +import { ValueType, ChartType, isChartConfig } from '@tupaia/types'; import { DEFAULT_DATA_KEY } from '../constants'; import { ExportViewContent, LooseObject, TableAccessor, ChartViewContent } from '../types'; import { formatTimestampForChart, getIsTimeSeries } from './utils'; @@ -123,24 +123,24 @@ const processData = (viewContent: ChartViewContent) => { }; export const getChartTableData = (viewContent?: ExportViewContent) => { - // tables only work for charts - if (!viewContent || viewContent.type !== 'chart') { - return { - columns: [], - data: [], - }; - } // Because react-table wants its sort function to be memoized, it needs to live here, outside of - // the other useMemo hooks + // the other useMemo hooks. All values need to be memoized, even default values, otherwise it will + // cause a potentially infinite loop of re-renders. + // See: [https://github.com/TanStack/table/issues/2369](this issue on GitHub for more information) const sortByTimestamp = useMemo( () => (rowA: any, rowB: any) => sortDates(rowA.original.timestamp, rowB.original.timestamp), undefined, ); - const columns = useMemo( - () => processColumns(viewContent, sortByTimestamp), - [JSON.stringify(viewContent)], - ); - const data = useMemo(() => processData(viewContent), [JSON.stringify(viewContent)]); + + const isChart = isChartConfig(viewContent); + const columns = useMemo(() => { + // only process columns if it's a chart, otherwise return an empty array. It won't be used but we have to memoize default values + return isChart ? processColumns(viewContent, sortByTimestamp) : []; + }, [JSON.stringify(viewContent)]); + const data = useMemo(() => { + // only process columns if it's a chart, otherwise return an empty array. It won't be used but we have to memoize default values + return isChart ? processData(viewContent) : []; + }, [JSON.stringify(viewContent)]); return { columns, data,