From 9d02b51fa0367f2de2ec997dd241b58135437c2c Mon Sep 17 00:00:00 2001
From: PeterYurkovich <pyurkovi@redhat.com>
Date: Tue, 13 Jan 2026 14:47:51 -0500
Subject: [PATCH] fix: remove top level dispatch; refactor to avoid late
 useEffect

a
---
 web/src/components/MetricsPage.tsx | 281 ++++++++++++++++-------------
 1 file changed, 158 insertions(+), 123 deletions(-)

diff --git a/web/src/components/MetricsPage.tsx b/web/src/components/MetricsPage.tsx
index b412c4f4..397dea5e 100644
--- a/web/src/components/MetricsPage.tsx
+++ b/web/src/components/MetricsPage.tsx
@@ -710,135 +710,170 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
     setSortBy({});
   }, [namespace, query]);
 
-  if (!isEnabled || !isExpanded || !query) {
-    return null;
-  }
-
-  if (error) {
-    return <Error error={error} title={t('Error loading values')} />;
-  }
-
-  if (!data) {
-    return <LoadingInline />;
-  }
-
-  // Add any data series from `series` (those displayed in the graph) that are not in `data.result`.
-  // This happens for queries that exclude a series currently, but included that same series at some
-  // point during the graph's range.
-  const expiredSeries = _.differenceWith(series, data.result, (s, r) => _.isEqual(s, r.metric));
-  const result = expiredSeries.length
-    ? [...data.result, ...expiredSeries.map((metric) => ({ metric }))]
-    : data.result;
-
-  if (!result || result.length === 0) {
-    return (
-      <div data-test={DataTestIDs.MetricsPageYellowNoDatapointsFound}>
-        <YellowExclamationTriangleIcon /> {t('No datapoints found.')}
-      </div>
-    );
-  }
+  const isUnused = !isEnabled || !isExpanded || !query;
+  const isError = !!error;
+  const isLoading = !data;
+  const result = useMemo(() => {
+    if (isUnused || isError || isLoading) {
+      return [];
+    }
+    // Add any data series from `series` (those displayed in the graph) that are not
+    // in `data.result`.This happens for queries that exclude a series currently, but
+    // included that same series at some point during the graph's range.
+    const expiredSeries = _.differenceWith(series, data.result, (s, r) => _.isEqual(s, r.metric));
+    return expiredSeries.length
+      ? [...data.result, ...expiredSeries.map((metric) => ({ metric }))]
+      : data.result;
+  }, [data?.result, series, isUnused, isError, isLoading]);
+  const isEmptyGraph = !result || result.length === 0;
+
+  const tableData = useMemo(() => {
+    if (isUnused || isError || isLoading || isEmptyGraph) {
+      return {};
+    }
+    const transforms: ITransform[] = [sortable, wrappable];
 
-  const transforms: ITransform[] = [sortable, wrappable];
-
-  const buttonCell = (labels) => ({ title: <SeriesButton index={index} labels={labels} /> });
-
-  let columns, rows;
-  if (data.resultType === 'scalar') {
-    columns = [
-      '',
-      {
-        title: t('Value'),
-        transforms,
-        cellTransforms: [
-          (data: IFormatterValueType) => {
-            const val = data?.title ? data.title : data;
-            return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
-          },
-        ],
-      },
-    ];
-    rows = [[buttonCell({}), _.get(result, '[1]')]];
-  } else if (data.resultType === 'string') {
-    columns = [
-      {
-        title: t('Value'),
-        transforms,
-        cellTransforms: [
-          (data: IFormatterValueType) => {
-            const val = data?.title ? data.title : data;
-            return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
-          },
-        ],
-      },
-    ];
-    rows = [[result?.[1]]];
-  } else {
-    const allLabelKeys = _.uniq(_.flatMap(result, ({ metric }) => Object.keys(metric))).sort();
-
-    columns = [
-      '',
-      ...allLabelKeys.map((k) => ({
-        title: <span>{k === '__name__' ? t('Name') : k}</span>,
-        transforms,
-      })),
-      {
-        title: t('Value'),
-        transforms,
-        cellTransforms: [
-          (data: IFormatterValueType) => {
-            const val = data?.title ? data.title : data;
-            return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
-          },
-        ],
-      },
-    ];
+    const buttonCell = (labels) => ({ title: <SeriesButton index={index} labels={labels} /> });
 
-    let rowMapper;
-    if (data.resultType === 'matrix') {
-      rowMapper = ({ metric, values }) => [
+    let columns, rows;
+    if (data.resultType === 'scalar') {
+      columns = [
         '',
-        ..._.map(allLabelKeys, (k) => metric[k]),
         {
-          title: (
-            <>
-              {_.map(values, ([time, v]) => (
-                <div key={time}>
-                  {v}&nbsp;@{time}
-                </div>
-              ))}
-            </>
-          ),
+          title: t('Value'),
+          transforms,
+          cellTransforms: [
+            (data: IFormatterValueType) => {
+              const val = data?.title ? data.title : data;
+              return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
+            },
+          ],
+        },
+      ];
+      rows = [[buttonCell({}), _.get(result, '[1]')]];
+    } else if (data.resultType === 'string') {
+      columns = [
+        {
+          title: t('Value'),
+          transforms,
+          cellTransforms: [
+            (data: IFormatterValueType) => {
+              const val = data?.title ? data.title : data;
+              return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
+            },
+          ],
         },
       ];
+      rows = [[result?.[1]]];
     } else {
-      rowMapper = ({ metric, value }) => [
-        buttonCell(metric),
-        ..._.map(allLabelKeys, (k) => metric[k]),
-        _.get(value, '[1]', { title: <span>{t('None')}</span> }),
+      const allLabelKeys = _.uniq(_.flatMap(result, ({ metric }) => Object.keys(metric))).sort();
+
+      columns = [
+        '',
+        ...allLabelKeys.map((k) => ({
+          title: <span>{k === '__name__' ? t('Name') : k}</span>,
+          transforms,
+        })),
+        {
+          title: t('Value'),
+          transforms,
+          cellTransforms: [
+            (data: IFormatterValueType) => {
+              const val = data?.title ? data.title : data;
+              return !Number.isNaN(Number(val)) ? valueFormat(Number(val)) : val;
+            },
+          ],
+        },
       ];
-    }
 
-    rows = _.map(result, rowMapper);
-    if (sortBy) {
-      // Sort Values column numerically and sort all the other columns alphabetically
-      const valuesColIndex = allLabelKeys.length + 1;
-      const sort =
-        sortBy.index === valuesColIndex
-          ? (cells) => {
-              const v = Number(cells[valuesColIndex]);
-              return Number.isNaN(v) ? 0 : v;
-            }
-          : `${sortBy.index}`;
-      rows = _.orderBy(rows, [sort], [sortBy.direction]);
+      let rowMapper;
+      if (data.resultType === 'matrix') {
+        rowMapper = ({ metric, values }) => [
+          '',
+          ..._.map(allLabelKeys, (k) => metric[k]),
+          {
+            title: (
+              <>
+                {_.map(values, ([time, v]) => (
+                  <div key={time}>
+                    {v}&nbsp;@{time}
+                  </div>
+                ))}
+              </>
+            ),
+          },
+        ];
+      } else {
+        rowMapper = ({ metric, value }) => [
+          buttonCell(metric),
+          ..._.map(allLabelKeys, (k) => metric[k]),
+          _.get(value, '[1]', { title: <span>{t('None')}</span> }),
+        ];
+      }
+
+      rows = _.map(result, rowMapper);
+      if (sortBy) {
+        // Sort Values column numerically and sort all the other columns alphabetically
+        const valuesColIndex = allLabelKeys.length + 1;
+        const sort =
+          sortBy.index === valuesColIndex
+            ? (cells) => {
+                const v = Number(cells[valuesColIndex]);
+                return Number.isNaN(v) ? 0 : v;
+              }
+            : `${sortBy.index}`;
+        rows = _.orderBy(rows, [sort], [sortBy.direction]);
+      }
     }
-  }
 
-  // Dispatch query table result so QueryKebab can access it for data export
-  dispatch(queryBrowserPatchQuery(index, { queryTableData: { columns, rows } }));
+    const onSort = (e, i, direction) => setSortBy({ index: i, direction });
 
-  const onSort = (e, i, direction) => setSortBy({ index: i, direction });
+    const tableRows = rows.slice((page - 1) * perPage, page * perPage).map((cells) => ({ cells }));
 
-  const tableRows = rows.slice((page - 1) * perPage, page * perPage).map((cells) => ({ cells }));
+    return {
+      onSort,
+      tableRows,
+      columns,
+      rows,
+    };
+  }, [
+    data?.resultType,
+    isEmptyGraph,
+    index,
+    isUnused,
+    isError,
+    isLoading,
+    page,
+    perPage,
+    result,
+    sortBy,
+    t,
+    valueFormat,
+  ]);
+
+  useEffect(() => {
+    if (tableData.columns && tableData.rows) {
+      dispatch(
+        queryBrowserPatchQuery(index, {
+          queryTableData: { columns: tableData.columns, rows: tableData.rows },
+        }),
+      );
+    }
+  }, [dispatch, index, tableData?.columns, tableData?.rows]);
+
+  if (isUnused) {
+    return null;
+  } else if (isError) {
+    return <Error error={error} title={t('Error loading values')} />;
+  } else if (isLoading) {
+    return <LoadingInline />;
+  } else if (isEmptyGraph) {
+    return (
+      <div data-test={DataTestIDs.MetricsPageYellowNoDatapointsFound}>
+        <YellowExclamationTriangleIcon /> {t('No datapoints found.')}
+      </div>
+    );
+  }
 
   return (
     <>
@@ -854,19 +889,19 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
         <Table
           aria-label={t('query results table')}
           gridBreakPoint={TableGridBreakpoint.none}
-          rows={tableRows.length}
+          rows={tableData?.tableRows.length}
           variant={TableVariant.compact}
           data-test={DataTestIDs.MetricsPageQueryTable}
         >
           <Thead>
             <Tr>
-              {columns.map((col, columnIndex) => {
+              {tableData?.columns.map((col, columnIndex) => {
                 const sortParams =
                   columnIndex !== 0
                     ? {
                         sort: {
                           sortBy,
-                          onSort,
+                          onSort: tableData?.onSort,
                           columnIndex,
                         },
                       }
@@ -880,15 +915,15 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
             </Tr>
           </Thead>
           <Tbody>
-            {tableRows.map((row, rowIndex) => (
+            {tableData?.tableRows.map((row, rowIndex) => (
               <Tr key={`row-${rowIndex}`}>
                 {row.cells?.map((cell, cellIndex) => (
                   <Td
                     style={{ fontFamily: t_global_font_family_mono.var }}
                     key={`cell-${rowIndex}-${cellIndex}`}
                   >
-                    {columns[cellIndex].cellTransforms
-                      ? columns[cellIndex].cellTransforms[0](
+                    {tableData?.columns[cellIndex].cellTransforms
+                      ? tableData?.columns[cellIndex].cellTransforms[0](
                           typeof cell === 'string' ? cell : cell?.title,
                         )
                       : typeof cell === 'string'
@@ -902,7 +937,7 @@ export const QueryTable: FC<QueryTableProps> = ({ index, namespace, customDataso
         </Table>
       </InnerScrollContainer>
       <TablePagination
-        itemCount={rows.length}
+        itemCount={tableData?.rows.length}
         page={page}
         perPage={perPage}
         setPage={setPage}