From 3fa44915c1b2f814ddbec494c405221b921ab2f5 Mon Sep 17 00:00:00 2001 From: Sanaei Date: Tue, 15 Sep 2026 21:06:32 +0200 Subject: [PATCH] perf(nodes): keep the node table element across unrelated re-renders rc-table re-runs every cell renderer whenever the Table re-renders, and NodeList rebuilt its columns and table props on every render (the relative time formatter was a fresh function each time). Any re-render of the Nodes page therefore re-rendered all rows even when no node had changed: about 390ms per re-render for 150 nodes in jsdom. The formatter is now stable and the table element is memoized on its inputs, so a re-render that leaves the nodes untouched costs 0.5ms. A heartbeat push that does change the nodes still re-renders every row. --- frontend/src/pages/nodes/NodeList.tsx | 98 +++++++++++-------- frontend/src/test/node-list-rerender.test.tsx | 68 +++++++++++++ 2 files changed, 124 insertions(+), 42 deletions(-) create mode 100644 frontend/src/test/node-list-rerender.test.tsx diff --git a/frontend/src/pages/nodes/NodeList.tsx b/frontend/src/pages/nodes/NodeList.tsx index e3dfc4869..aa81fa652 100644 --- a/frontend/src/pages/nodes/NodeList.tsx +++ b/frontend/src/pages/nodes/NodeList.tsx @@ -145,17 +145,22 @@ function formatUptime(secs?: number): string { return `${mins}m`; } +// Stable per language: the columns memo depends on it, and a fresh function each +// render rebuilt every column, re-rendering all rows on each heartbeat push. function useRelativeTime() { const { t } = useTranslation(); - return (unixSeconds?: number) => { - if (!unixSeconds) return t('pages.nodes.never'); - const diffSec = Math.max(0, Math.floor(Date.now() / 1000 - unixSeconds)); - if (diffSec < 5) return t('pages.nodes.justNow'); - if (diffSec < 60) return `${diffSec}s`; - if (diffSec < 3600) return `${Math.floor(diffSec / 60)}m`; - if (diffSec < 86400) return `${Math.floor(diffSec / 3600)}h`; - return `${Math.floor(diffSec / 86400)}d`; - }; + return useMemo( + () => (unixSeconds?: number) => { + if (!unixSeconds) return t('pages.nodes.never'); + const diffSec = Math.max(0, Math.floor(Date.now() / 1000 - unixSeconds)); + if (diffSec < 5) return t('pages.nodes.justNow'); + if (diffSec < 60) return `${diffSec}s`; + if (diffSec < 3600) return `${Math.floor(diffSec / 60)}m`; + if (diffSec < 86400) return `${Math.floor(diffSec / 3600)}h`; + return `${Math.floor(diffSec / 86400)}d`; + }, + [t], + ); } export default function NodeList({ @@ -530,6 +535,47 @@ export default function NodeList({ ], ); + // rc-table re-runs every cell renderer whenever the Table re-renders, so keep the + // same element until its inputs change rather than re-rendering all rows each time. + const nodeTable = useMemo( + () => ( + + dataSource={dataSource} + columns={columns} + pagination={false} + loading={loading} + scroll={{ x: 'max-content' }} + size="middle" + rowKey="key" + rowSelection={ + dataSource.length > 1 + ? { + selectedRowKeys: selectedIds, + onChange: (keys) => + onSelectionChange(keys.filter((k) => typeof k === 'number') as number[]), + getCheckboxProps: (record) => ({ + disabled: !!record.transitive || !isUpdateEligible(record), + }), + } + : undefined + } + locale={{ + emptyText: ( +
+ +
{t('noData')}
+
+ ), + }} + expandable={{ + expandedRowRender: (record) => , + rowExpandable: (record) => !record.transitive, + }} + /> + ), + [dataSource, columns, loading, selectedIds, onSelectionChange, t], + ); + return (
@@ -806,39 +852,7 @@ export default function NodeList({ ) : ( - - dataSource={dataSource} - columns={columns} - pagination={false} - loading={loading} - scroll={{ x: 'max-content' }} - size="middle" - rowKey="key" - rowSelection={ - dataSource.length > 1 - ? { - selectedRowKeys: selectedIds, - onChange: (keys) => - onSelectionChange(keys.filter((k) => typeof k === 'number') as number[]), - getCheckboxProps: (record) => ({ - disabled: !!record.transitive || !isUpdateEligible(record), - }), - } - : undefined - } - locale={{ - emptyText: ( -
- -
{t('noData')}
-
- ), - }} - expandable={{ - expandedRowRender: (record) => , - rowExpandable: (record) => !record.transitive, - }} - /> + nodeTable )} ); diff --git a/frontend/src/test/node-list-rerender.test.tsx b/frontend/src/test/node-list-rerender.test.tsx new file mode 100644 index 000000000..376634ad6 --- /dev/null +++ b/frontend/src/test/node-list-rerender.test.tsx @@ -0,0 +1,68 @@ +import type { ReactNode } from 'react'; +import { render } from '@testing-library/react'; +import { QueryClientProvider } from '@tanstack/react-query'; +import { describe, expect, it, vi } from 'vitest'; + +import { ThemeProvider } from '@/hooks/useTheme'; +import NodeList from '@/pages/nodes/NodeList'; +import type { NodeRecord } from '@/schemas/node'; + +import { makeTestQueryClient } from './test-utils'; + +const updateChecks = vi.hoisted(() => ({ count: 0 })); + +vi.mock('@/lib/panel-version', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + isPanelUpdateAvailable: (...args: Parameters) => { + updateChecks.count++; + return actual.isPanelUpdateAvailable(...args); + }, + }; +}); + +// Every heartbeat push re-rendered all rows, unchanged ones too: the columns and +// table props were rebuilt on each render, so every cell re-ran its renderer. +describe('NodeList re-render', () => { + it('leaves the rows alone when its parent re-renders with the same nodes', () => { + const queryClient = makeTestQueryClient(); + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + const nodes: NodeRecord[] = [1, 2, 3].map((id) => ({ + id, + name: `node-${id}`, + guid: `g${id}`, + transitive: false, + enable: true, + status: 'online', + panelVersion: '3.0.0', + })); + const noop = () => {}; + const props = { + nodes, + isMobile: false, + latestVersion: '3.0.1', + selectedIds: [] as number[], + onSelectionChange: noop, + onAdd: noop, + onMtls: noop, + onEdit: noop, + onDelete: noop, + onProbe: noop, + onToggleEnable: noop, + onUpdateNode: noop, + onUpdateSelected: noop, + }; + const view = render(, { wrapper }); + expect(updateChecks.count).toBeGreaterThan(0); + + updateChecks.count = 0; + view.rerender(); + + expect(updateChecks.count).toBe(0); + }); +});