Repository navigation
fix: avoid re-rendering unchanged Table rows and cells - #10816
pete-woods wants to merge 1 commit into
Conversation
56133fd to
ebcd3b2
Compare
Row and Cell read TableStateContext, whose value changes whenever the collection, selection or expanded keys change, so every row and cell re-ran its hooks on any table update, even when the rendered elements were cached. Row and Cell now render their existing bodies inside a memoised component that only re-renders when a render key, computed from the parts of the table state the row or cell displays, changes. The bodies read a stable proxy of the table state (useStableTableState) so event handlers in rows that skipped rendering still act on the latest collection, selection and callbacks. Table-wide values that affect every row are provided through a context that changes only when one of them does. Adds a test that runs the same interactions against Table and against a copy with memoisation disabled and checks the DOM always matches, and a "Tree Table render counts" story that flashes rows and cells as they render. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ebcd3b2 to
e9ad1b5
Compare
|
In your own words, how is this different from #10768? |
To be completely honest I didn't spot that PR. I set "the machine" on this performance tuning task before I realised others had the same issue. I'll have a look through to see how the implementations compare. |
|
So the fix here is more general. But both changes I think make sense combined:
With the two PRs combined I think we'd see even fewer re-renders. |
|
Getting opus to compare the performance of each. I'm also gonna look how the numbers compare if we combined both. Edit: Below is Opus's analysis, which seems to confirm what I expected (they combine well). Table render cost:
|
| Scenario | main |
#10768 | #10816 | Combined |
|---|---|---|---|---|
| Container resized (its own width state, #10759) | 20 / 40 | 0 / 0 | 0 / 0 | 0 / 0 |
| Parent re-renders with a new width | 40 / 80 | 40 / 80 | 0 / 0 | 0 / 0 |
| Parent re-renders, nothing changed | 40 / 80 | 40 / 80 | 0 / 0 | 0 / 0 |
| Click a cell (#10477) | 42 / 84 | 42 / 84 | 23 / 41 | 23 / 41 |
| Arrow key to next row (#10477) | 23 / 45 | 23 / 45 | 4 / 8 | 4 / 8 |
| Change one row's data | 40 / 80 | 40 / 80 | 1 / 2 | 1 / 2 |
| Tree: expand one row | 100 / 100 | 100 / 100 | 5 / 5 | 5 / 5 |
| Tree: add a child row | 101 / 101 | 101 / 101 | 6 / 6 | 6 / 6 |
Setup:
- Flat table: 20 rows, 2 columns,
selectionMode="multiple", inside aResizableTableContainer. - Tree table: 10 parents with 4 children each, 9 of the parents expanded.
- Counting: each
RowandCellcounts a render whenever itsclassNamerender prop runs. - Environment: Jest with jsdom and the repo's test setup. That setup uses StrictMode, so some counts include double renders.
- Container resize:
clientWidthis mocked and a windowresizeevent is fired, the same way Fix/table state identity #10768's tests do it. - "Click a cell" on fix: avoid re-rendering unchanged Table rows and cells #10816: the first selection flips
isEmpty, which re-renders every row. A later click renders about 2 rows.
Source: /tmp/CompareRenders.test.js.
Tree table timings (Chromium)
From Treeble.perf.browser.test.tsx: 50 parents with 5 children each, 3 columns, selectionMode="multiple", dev build, timed with flushSync. Each cell gives the median over 5–7 rounds in two separate runs, as "run 1 / run 2".
| Scenario | main |
#10768 | #10816 | Combined | Rows rendered (main → #10816) |
|---|---|---|---|---|---|
| Add 1 child row | 473 / 501 ms | 465 / 488 ms | 32 / 30 ms | 29 / 29 ms | 613 → 13 |
| Parent re-renders, nothing changed | 146 / 159 ms | 126 / 145 ms | 30 / 12 ms | 12 / 13 ms | 614 → 0 |
| Add 60 child rows | 294 / 247 ms | 262 / 243 ms | 82 / 95 ms | 98 / 100 ms | 1154 → 306 |
| Expand one row | 144 / 142 ms | 138 / 149 ms | 15 / 13 ms | 13 / 12 ms | 600 → 6 |
| Collapse one row | 226 / 143 ms | 138 / 141 ms | 13 / 12 ms | 11 / 12 ms | 590 → 1 |
#10768 renders the same rows as main in this benchmark, and combined renders the same rows as #10816.
Container resize timings (Chromium)
A ResizableTableContainer with 300 rows and 4 columns. Each round changes the container's width through the DOM, so the real ResizeObserver fires. The figures are total React render time per resize from <Profiler> actualDuration, as the median of 9 resizes in each of two runs.
| Version | Render time per resize | Cells rendered per resize |
|---|---|---|
main |
47.9 / 49.5 ms | 1,200 |
| #10768 | 0.7 / 1.1 ms | 0 |
| #10816 | 4.5 / 4.9 ms | 0 |
| Combined | 0.9 / 0.7 ms | 0 |
#10816 renders no cells here, but it still runs each row and cell's wrapper to compare render keys, which costs about 4 ms for 300 rows. #10768 avoids that because TableStateContext keeps its identity.
Source: /tmp/ResizeBench.browser.test.tsx.
Takeaways
- Fix/table state identity #10768 only helps when the table state really doesn't change. That's the container resizing itself, which is Table: TableStateContext changes identity every render, so ResizableTableContainer re-renders every cell on width changes #10759's exact case. A parent re-render rebuilds the collection from inline
<Column>elements, so the state is new and every row renders. So does any real change to focus, selection, data or expansion. - fix: avoid re-rendering unchanged Table rows and cells #10816 helps for every kind of update, because rows and cells skip rendering even when the state changes. What's left is the per-row render-key check, about 4 ms for 300 rows on a resize.
- Combined gives the best of both. It's as fast as Fix/table state identity #10768 on a container resize, and as good as fix: avoid re-rendering unchanged Table rows and cells #10816 everywhere else. Combining them causes no regressions: the counts and timings match fix: avoid re-rendering unchanged Table rows and cells #10816 apart from the resize case.
|
Were you running your own perf? It'd be good if we could run those combinations with our new PR @reidbarber has been working on to do benchmarking #10787 |
Yes, I got Opus to make a performance harness for this. Let me look into using the one from your other PR. |
|
Okay, I got it to performance profile the various combinations with the harness you mentioned. I had to get opus to add some more scenarios (which I can add in a PR if we move forward with this). It seems to back up my above findings. Table performance with the #10787 harness: summaryI compared
Scenarios added to the harnessThe harness had no tree table. Its So I added a
Each action waits for the expected end state before it counts as complete: the full row order, All other rows below come from #10787's existing scenarios:
ResultsTimes are medians of the time from input to two frames after the expected DOM and focus state.
Findings
|
| let stateRef = useRef(state); | ||
| // Rows and cells render after this in the same pass and must see the new state, so the ref is | ||
| // updated during render. The layout effect restores the committed state if a render is discarded. | ||
| // oxlint-disable-next-line react/react-compiler, rsp-rules/pure-render |
There was a problem hiding this comment.
this one we can't disable unfortunately, it's caused some nasty bugs before, no reading or writing refs in render
3 participants
Closes #10477, closes #10759
Intent
When one row of a large RAC
Tablechanges, for example a tree table gaining child rows or a row being expanded, the update should cost roughly that row's subtree, not the whole table. Today everyRowandCellre-runsuseTableRow/useTableCell, focus rings, hover, selection checkbox and so on for every update. On a tree table of about 300 rows that blocks the main thread for 150–500ms in a dev build, even when only one row changed.The same cause is behind two open issues, which this fixes. These counts come from a 20-row table in a
ResizableTableContainer, measured in Jest onmainand on this branch:mainIt also helps with #8617, but doesn't fully fix it: sorting and filtering replace most rows, so those rows still render.
Why it happens
useCachedChildrenalready returns the same element for unchanged collection nodes, butRowandCellconsumeTableStateContext. Its value is a new object whenever the collection, selection or expanded keys change, so React can't bail out. There are usually two passes per update: the parent render, then theuseSyncExternalStorerender once the hidden collection tree commits. Inline<Column>elements get new props on every parent render, which rebuildscollection.columns, so even a parent re-render that changes nothing triggers a full second pass.How this works
RowandCellare created with small table-local wrappers (createTableBranchComponent/createTableLeafComponent). These render the existing body, unchanged, inside amemo'd component. The wrapper still readsTableStateContextand computes a render key from exactly what that row or cell displays:isFocused/childFocusStrategyonly when focused), expanded, last-child type, its cell nodes (updating a cell doesn't clone the row node), and the last sibling's index (foraria-setsize).TableRowRenderContext, which changes only when one of them does: selection mode and behaviour,isEmpty, empty collection, column keys (not the array identity), row header keys, header row count,treeColumn, and whetheronRowAction/onCellActionare set.useStableTableState(new, inreact-aria/private/table). It returns a stable proxy that forwards to the latest table state, including theSelectionManager, keyboard delegate and grid actions, and it is registered withgridMap/gridIds. Without it, a row that skipped rendering would keep stale closures and act on an outdated selection, collection oronRowActioncallback.DragAndDropContextandRowFocusContextvalues are now memoised.I picked this over a selector-style subscription per row because it doesn't change the hooks or the collection internals. Everything stays inside RAC
Table, andTableStateContextbehaves the same for anything else that consumes it.Benchmark
packages/react-aria-components/test/Treeble.perf.browser.test.tsx: 50 parents × 5 children, 3 columns,selectionMode="multiple", dev build, Chromium, timed withflushSync. Medians over 3 runs:Adding 60 rows still re-renders every row in that group, because each one's
aria-setsizechanges.Behaviour changes and risks
RowRenderProps.stateis now the stable proxy. AclassNameorstylefunction that reads table-wide state not covered by the render keys won't update until its own row re-renders. Components that readTableStateContextdirectly still update as before.useTableRow,useGridRow,useSelectableItemanduseGridCellread during render. If those hooks start depending on new state, the keys need updating too.TableRenderKeys.test.js(below) catches most omissions.dragStateanddropStateare new objects on every render, so tables with drag and drop still re-render all rows. No regression, but no gain either.✅ Pull Request Checklist:
AI disclosure: the investigation, implementation, tests and benchmark were done with Claude Code (Claude Opus 5.5).
Adds a "Tree Table render counts" story. No docs changes. Rendered output is unchanged and the only API change is private (
react-aria/private/table/useStableTableState). ARIA output is unchanged and covered by the existing Treeble/Table tests.📝 Test Instructions:
New tests in
Treeble.test.jsunder "Treeble row memoization":onRowActionclosure;Each of these fails if the memoised rows are given non-stable state.
TableRenderKeys.test.jsruns about 60 interactions againstTableand against a copy loaded withmemodisabled (every row and cell always re-renders), with and without drag and drop. After each step it checks that the two DOMs match, including render props and the focused element. Interactions include selection, focus, expansion, row and column changes, disabled rows, actions and keyboard navigation modes. Removing render-key entries one at a time, and in groups that duplicate each other, makes it fail. The exceptions are node-identity entries: they are currently redundant becauseDocument.setPropsgives every sibling of a changed row new props. Also uncaught iskeyboardNavigationBehavior, which can't be reached from jsdom.The last Treeble drag-and-drop test now presses Escape at the end. It left a keyboard drag running, which hid the document from any test that ran after it.
yarn test(8140 passed, 16 skipped),yarn test:ssr(74 passed),yarn test-looseon RAC/S2/RSP table (1976 passed), and Chromium browser tests for RAC and S2 (147 passed).yarn lintis clean. The Firefox and WebKit browser projects were not run.npx vitest run --config=vitest.browser.config.ts --project chromium-desktop packages/react-aria-components/test/Treeble.perf.browser.test.tsx.main, and togglewithDragAndDropto see the drag-and-drop gap. Also try the Table tree stories in Storybook. Expand and collapse rows with mouse and keyboard (ArrowLeft/ArrowRight, including RTL), select with click and shift-click, and check focus restoration after expanding.🧢 Your Project:
CircleCI's web based UI.