From b4ef07d28e64233f30ec5233cc45d2418566aca8 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 2 Oct 2026 03:16:37 +0200 Subject: [PATCH] perf(ui): a long table page paints its first rows at once and the rest in steps The channel manager's All page (~400 rows) blocked one frame for 0.7-0.9 s on prod to mount, and every sort and search key re-rendered every row (300-400 ms). No single cell is to blame, measured column by column. DataTable now mounts the first 30 rows at once and 40 more per background transition; a new page, size, sort, filter or caller-side search (narrowedBy) starts over from the first rows. A row is its own component, so the compiler keeps the rows a step already holds. --- frontend/src/components/ChannelDiscovery.tsx | 1 + frontend/src/components/Channels.tsx | 1 + .../ui/DataTable.progressive.dom.test.tsx | 103 ++++++++++++++++++ frontend/src/components/ui/DataTable.tsx | 79 ++++++++++---- frontend/src/components/ui/progressiveRows.ts | 33 ++++++ 5 files changed, 198 insertions(+), 19 deletions(-) create mode 100644 frontend/src/components/ui/DataTable.progressive.dom.test.tsx create mode 100644 frontend/src/components/ui/progressiveRows.ts diff --git a/frontend/src/components/ChannelDiscovery.tsx b/frontend/src/components/ChannelDiscovery.tsx index 3e0896f8..96164322 100644 --- a/frontend/src/components/ChannelDiscovery.tsx +++ b/frontend/src/components/ChannelDiscovery.tsx @@ -242,6 +242,7 @@ export default function ChannelDiscovery({ onSortChange={setSort} emptyText={t("channels.discovery.empty")} keyboardNav + narrowedBy={q} /> ) : ( ) : ( <> diff --git a/frontend/src/components/ui/DataTable.progressive.dom.test.tsx b/frontend/src/components/ui/DataTable.progressive.dom.test.tsx new file mode 100644 index 00000000..db15fa1c --- /dev/null +++ b/frontend/src/components/ui/DataTable.progressive.dom.test.tsx @@ -0,0 +1,103 @@ +// @vitest-environment jsdom +// +// E31 S4d: the channel manager's "All" page (~400 rows) blocked one frame for 0.7–0.9 s on prod to +// mount, and every key in its search re-rendered every row. A long page now mounts its first rows at +// once and the rest in small background steps (progressiveRows), and a step leaves the rows it holds +// alone (a row is its own component, whose cells the React Compiler caches). +// +// These tests drive React without act(): act flushes the background steps along with the first render, +// and the first render on its own is the property under test. +import { describe, it, expect, afterEach, beforeAll, afterAll, vi } from "vitest"; +import { flushSync } from "react-dom"; +import { createRoot, type Root } from "react-dom/client"; +import { waitFor } from "@testing-library/react"; +import DataTable, { type Column } from "./DataTable"; +import { FIRST_ROWS } from "./progressiveRows"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useTranslation: () => ({ i18n: { language: "en" }, t: (key: string) => key }), + }; +}); + +const g = globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }; +let wasAct: boolean | undefined; +beforeAll(() => { + wasAct = g.IS_REACT_ACT_ENVIRONMENT; + g.IS_REACT_ACT_ENVIRONMENT = false; +}); +afterAll(() => { + g.IS_REACT_ACT_ENVIRONMENT = wasAct; +}); + +type Row = { id: string; name: string }; +const many = (n: number, prefix = "r"): Row[] => + Array.from({ length: n }, (_, i) => ({ id: `${prefix}${i}`, name: `${prefix} ${i}` })); + +let root: Root | null = null; +let host: HTMLElement | null = null; +afterEach(() => { + root?.unmount(); + host?.remove(); + root = null; + host = null; +}); + +const bodyRows = () => host!.querySelectorAll("tbody tr").length; + +// Mounts synchronously, so what the DOM holds right after is the first render alone. +function mount(rows: Row[], columns: Column[], narrowedBy = "") { + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + const draw = (r: Row[], n: string) => + flushSync(() => + root!.render( + x.id} pageSize={0} narrowedBy={n} />, + ), + ); + draw(rows, narrowedBy); + return draw; +} + +const plain: Column[] = [{ key: "name", header: "Name", render: (r) => r.name }]; + +describe("DataTable — a long page mounts in steps", () => { + it("paints its first rows at once and the rest after", async () => { + mount(many(200), plain); + expect(bodyRows()).toBe(FIRST_ROWS); + await waitFor(() => expect(bodyRows()).toBe(200)); + }); + + it("does not re-render the rows it already holds as it grows", async () => { + const renders = new Map(); + const counted: Column[] = [ + { + key: "name", + header: "Name", + render: (r) => { + renders.set(r.id, (renders.get(r.id) ?? 0) + 1); + return r.name; + }, + }, + ]; + mount(many(200), counted); + await waitFor(() => expect(bodyRows()).toBe(200)); + // The first row was drawn by the first render, not again by each step after it. + expect(renders.get("r0")).toBe(1); + }); + + it("starts over from the first rows when the caller narrows them, not on new data", async () => { + const draw = mount(many(200), plain, ""); + await waitFor(() => expect(bodyRows()).toBe(200)); + // New data for the same view (a refetch): the page keeps what it shows. + draw(many(200), ""); + expect(bodyRows()).toBe(200); + // A search the caller applied: the urgent render is small again. + draw(many(150, "s"), "s"); + expect(bodyRows()).toBe(FIRST_ROWS); + await waitFor(() => expect(bodyRows()).toBe(150)); + }); +}); diff --git a/frontend/src/components/ui/DataTable.tsx b/frontend/src/components/ui/DataTable.tsx index ce2d6296..db8c02a9 100644 --- a/frontend/src/components/ui/DataTable.tsx +++ b/frontend/src/components/ui/DataTable.tsx @@ -11,6 +11,7 @@ import { navItemAttrs, useListNav } from "../../lib/listNav"; import { useMediaQuery } from "../../lib/useMediaQuery"; import { SURFACE } from "./surfaces"; import { HiddenFilterCue } from "./HiddenFilterCue"; +import { useProgressiveRows } from "./progressiveRows"; // A reusable, client-side data table: per-column sort + filter (in the header) + pagination, // with its sort/filter/page state optionally persisted to localStorage so a reload (F5) keeps @@ -48,6 +49,45 @@ function loadPersist(key?: string): Persisted { } } +interface TableRowProps { + row: T; + index: number; + columns: Column[]; + rowProps?: (row: T) => Record; + rowClassName?: (row: T) => string; + keyboardNav: boolean; +} + +// A row is its own component so the React Compiler caches its cells by (row, columns, …): a long page +// grows in steps (progressiveRows), and a step must not draw the rows it already holds again. Inline in +// the body's map they all were, and the steps grew from ~70 to ~200 ms (dev build) as the page filled. +function TableRow({ + row, + index, + columns, + rowProps, + rowClassName, + keyboardNav, +}: TableRowProps) { + return ( + + {columns.map((col) => ( + + {col.render(row)} + + ))} + + ); +} + export default function DataTable({ rows, columns, @@ -72,6 +112,7 @@ export default function DataTable({ keyboardNav = false, reveal, surfacedFilters, + narrowedBy = "", }: { rows: T[]; columns: Column[]; @@ -131,6 +172,10 @@ export default function DataTable({ // Filter keys the caller shows somewhere besides their column (the channel manager's tag chips): // the hidden-filter cue leaves those out, it would name them twice. surfacedFilters?: string[]; + // What the caller narrowed `rows` by before passing them in (the channel manager's name search lives + // in the shared top bar): a change starts a long page over from its first rows, as the table's own + // sort and filter changes do (progressiveRows). Without it every key re-rendered the whole page. + narrowedBy?: string; }) { const { t, i18n } = useTranslation(); const lang = i18n.language; @@ -229,6 +274,11 @@ export default function DataTable({ if (at >= 0 && !allRows) setPage(Math.floor(at / size)); } const paged = allRows ? sorted : sorted.slice(safePage * size, safePage * size + size); + // A long page ("All", 100) mounts its first rows at once and the rest in the background. + const shown = useProgressiveRows( + paged, + `${safePage}:${size}:${JSON.stringify(sort)}:${JSON.stringify(filters)}:${narrowedBy}`, + ); function toggleSort(key: string) { const next = cycleSort(sort, key); @@ -327,25 +377,16 @@ export default function DataTable({ onFilter={applyFilter} /> - {paged.map((row, i) => ( - ( + - {columns.map((col) => ( - - {col.render(row)} - - ))} - + row={row} + index={i} + columns={columns} + rowProps={rowProps} + rowClassName={rowClassName} + keyboardNav={keyboardNav} + /> ))} @@ -356,7 +397,7 @@ export default function DataTable({ <> {cue("cards")}
- {paged.map((row, i) => ( + {shown.map((row, i) => (
(rows: T[], view: string): T[] { + const [seen, setSeen] = useState(view); + const [limit, setLimit] = useState(FIRST_ROWS); + if (seen !== view) { + setSeen(view); + setLimit(FIRST_ROWS); + } + const more = rows.length > limit; + useEffect(() => { + if (more) startTransition(() => setLimit(limit + STEP_ROWS)); + }, [more, limit]); + return more ? rows.slice(0, limit) : rows; +}