diff --git a/packages/react/src/__tests__/pretable-surface.test.tsx b/packages/react/src/__tests__/pretable-surface.test.tsx index faf85bf7..d649362f 100644 --- a/packages/react/src/__tests__/pretable-surface.test.tsx +++ b/packages/react/src/__tests__/pretable-surface.test.tsx @@ -1006,6 +1006,8 @@ const gridColumns = [ { id: "c", header: "C", widthPx: 100 }, ]; +const getGridRowId = (row: GridRow) => row.id; + const gridRows: GridRow[] = [ { id: "r1", a: "a1", b: "b1", c: "c1" }, { id: "r2", a: "a2", b: "b2", c: "c2" }, @@ -1473,6 +1475,65 @@ describe("keyboard contract", () => { }); describe("controlled-mode round-trips", () => { + it("does not emit to the external store during render when a controlled prop changes (no React 'update while rendering' warning)", () => { + // Repro of the website-hero bug: the headless `usePretable` grid is shared + // between the surface and a sibling control panel (both subscribe to the + // grid's external store). A controlled prop (`state.filters`) is driven by + // sibling state. Applying the grid mutation *during render* emits to the + // store synchronously, which notifies the *already-mounted sibling* + // subscriber mid-render — tripping React's "Cannot update a component + // while rendering a different component" warning on every keystroke. + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + // A sibling that independently subscribes to the same grid store — stands + // in for the hero's control-panel / result-count readout. + function ResultCount({ grid }: { grid: PretableGrid }) { + const snap = React.useSyncExternalStore(grid.subscribe, grid.getSnapshot); + return {snap.visibleRows.length}; + } + + function HeadlessHarness() { + const [query, setQuery] = React.useState(""); + const { grid, renderSnapshot } = usePretable({ + columns: gridColumns, + getRowId: getGridRowId, + overscan: 0, + rows: gridRows, + state: { filters: query ? { a: query } : {} }, + viewportHeight: 300, + }); + + return ( +
+ setQuery(e.target.value)} + /> + + {renderSnapshot.rows.length} +
+ ); + } + + const view = render(); + const input = view.getByLabelText("filter"); + + fireEvent.change(input, { target: { value: "a1" } }); + fireEvent.change(input, { target: { value: "a12" } }); + + const renderWhileRenderingWarning = errorSpy.mock.calls.some((call) => + String(call[0]).includes("Cannot update a component"), + ); + expect(renderWhileRenderingWarning).toBe(false); + + // Filtering still works: query "a1" matches only r1 (column a = "a1"). + fireEvent.change(input, { target: { value: "a1" } }); + expect(view.getByTestId("count")).toHaveTextContent("1"); + + errorSpy.mockRestore(); + }); + it("state.selection: arrow does not mutate rendered selection when controlled and consumer ignores callback", () => { const onSelectionChange = vi.fn(); const controlledSelection: PretableSelectionState = { @@ -4291,7 +4352,11 @@ describe("cell renderers", () => { const view = render(); const initialCalls = renderFn.mock.calls.length; - expect(initialCalls).toBe(gridRows.length); + // Controlled focus is applied in a layout effect (post-commit, pre-paint) + // rather than during render, so the focused cell (r1/a) re-renders once + // after mount when focus lands on it — one extra call beyond the per-row + // baseline. The non-focused rows still render exactly once. + expect(initialCalls).toBe(gridRows.length + 1); // Re-render with focus moved from r1 -> r2 (column "a"). r1 loses focus, // r2 gains focus, both isFocused props change -> both should re-render. diff --git a/packages/react/src/use-pretable.ts b/packages/react/src/use-pretable.ts index fdd21ab4..9d4dfa20 100644 --- a/packages/react/src/use-pretable.ts +++ b/packages/react/src/use-pretable.ts @@ -172,7 +172,31 @@ export function usePretable({ void onSelectionChange; void onFocusChange; - if (state) { + const snapshot = useSyncExternalStore( + grid.subscribe, + grid.getSnapshot, + grid.getSnapshot, + ); + + // Apply controlled state in a layout effect rather than during render: the + // grid mutators emit to the external store synchronously, and emitting while + // rendering trips React's "Cannot update a component while rendering a + // different component" warning (see useSyncExternalStore). Running it post- + // commit (but before paint) keeps the controlled value authoritative without + // the during-render emit. + // + // The effect depends on `snapshot` so it re-runs after *internal* grid events + // (keyboard, click) as well as prop changes: when an internal event tries to + // change a controlled slice and the consumer ignores the callback, the engine + // has diverged from the prop, and this re-assert forces it back. Every grid + // mutator self-guards against equal values (no emit when unchanged), so the + // effect converges — the re-assert after our own emit is a no-op — and never + // loops. + useLayoutEffect(() => { + if (!state) { + return; + } + if (state.sort !== undefined) { grid.setSort(state.sort?.columnId ?? null, state.sort?.direction ?? null); } @@ -233,13 +257,9 @@ export function usePretable({ grid.setFocus(null); } } - } - - const snapshot = useSyncExternalStore( - grid.subscribe, - grid.getSnapshot, - grid.getSnapshot, - ); + // `snapshot` is an intentional dependency: it makes the effect re-assert the + // controlled value after internal grid mutations, not just prop changes. + }, [grid, state, snapshot]); useLayoutEffect(() => { if (