Skip to content

fix: reuse layout nodes built while scrolling in ListLayout - #10707

Open
romansndlr wants to merge 1 commit into
adobe:mainfrom
romansndlr:fix/list-layout-reuse-scrolled-nodes
Open

romansndlr wants to merge 1 commit into
adobe:mainfrom
romansndlr:fix/list-layout-reuse-scrolled-nodes

Conversation

@romansndlr

@romansndlr romansndlr commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Closes #10706

Problem

ListLayout.layoutIfNeeded unions each new visible rect into requestedRect and rebuilds the collection, but validRect is only set in update(). Every node laid out while scrolling therefore stays outside validRect, isValid() rejects it on the next rebuild, and it is laid out again. In TableLayout that means buildRow runs for every row from the last update() down to the viewport on each scroll frame, so frame cost grows with scroll distance. Details and a repro are in #10706.

Intent

Layout work per scroll frame should depend on the rows coming into view, not on how far the user has scrolled. Which rows get laid out, and where, should not change.

How

After layoutIfNeeded rebuilds for a new rect, set validRect to requestedRect, the same thing update() does at the end of a rebuild. The nodes it just built are current, so later rebuilds can reuse them. isValid still requires the same node and the same offset, so a row whose node or position changed is laid out again as before. update() and updateItemSize keep managing validRect as they do today.

Measurements

The #10706 repro: 5000 rows, production build, headless Chromium 145, 4× CPU throttling, 120px per frame. Average frame time per range of rows, median of 3 runs:

Rows scrolled Before After
0–499 9.2 ms 8.5 ms
2000–2499 18.9 ms 8.5 ms
4000–4499 29.0 ms 8.6 ms

On a variant of the repro with estimatedRowHeight and rows of six different measured heights (18–92px), I recorded every rendered row's position and cell height at 66 checkpoints while scrolling 2000 rows down and back up. Before and after were identical at every checkpoint, and so was the total scroll height.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests). Added a RAC-level test that counts the rows laid out on a scroll; it fails without the change (rows 4–89 are laid out again instead of the rows coming into view). No story, since there is no visual change.
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component). Not applicable, no API change.
  • Looked at the Accessibility Practices for this feature - Aria Practices. Not applicable, layout-only change.
  • I understand every change in this PR and can explain why it's there.
  • If AI-assisted, I followed our AI contribution guidance and pointed my assistant at AGENTS.md.

📝 Test Instructions:

  1. Run yarn jest packages/react-aria-components/test/Table.test.js -t "only lay out rows that come into view". It passes with this change and fails without it.
  2. Render the component from Virtualized Table: scroll frame cost grows with the number of rows scrolled past #10706 in a production build, enable 4× CPU throttling in DevTools, and record while scrolling from the top to around row 4000. Before this change frame times climb and buildRow time grows with scroll distance; after it, frame times stay flat.
  3. Scroll virtualized tables, lists and grid lists with variable row heights down and back up, and check that rows don't overlap or leave gaps.

What I tested:

  • yarn test: 384 suites, 8439 passed, 16 skipped
  • yarn test:ssr: 60 suites, 74 passed
  • yarn lint: passes
  • yarn test:browser: not run to completion yet
  • Scrolling by programmatic scrollTop steps in headless Chromium, with fixed and measured row heights, LTR only
  • Keyboard navigation only through the existing Jest suites; no manual keyboard, screen reader, touch, RTL, Firefox or Safari testing

🧢 Your Project:

groundcover

layoutIfNeeded unions each new visible rect into requestedRect and rebuilds,
but only update() set validRect, so every node laid out while scrolling failed
isValid on the next rebuild and was laid out again. In TableLayout that re-ran
buildRow for every row scrolled past on each frame, so scroll cost grew with
scroll distance. Mark the freshly built rect valid, as update() does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0119dQGPf1YcHgA29G1CfxqP
@romansndlr
romansndlr marked this pull request as ready for review October 3, 2026 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Virtualized Table: scroll frame cost grows with the number of rows scrolled past

1 participant