Skip to content

fix(useGridState): prevent infinite loop restoring focus when no focusable row remains#10241

Open
RobHannay wants to merge 6 commits into
adobe:mainfrom
RobHannay:rob/fix-usegridstate-focus-restoration-infinite-loop
Open

fix(useGridState): prevent infinite loop restoring focus when no focusable row remains#10241
RobHannay wants to merge 6 commits into
adobe:mainfrom
RobHannay:rob/fix-usegridstate-focus-restoration-infinite-loop

Conversation

@RobHannay

Copy link
Copy Markdown
Contributor

Description

useGridState's focus-restoration effect (which runs when the focused row is removed from the collection) searches collection.rows for a non-disabled, non-headerrow row to move focus to. It's meant to scan forward from the deleted position to the end, then fall back to scanning backward — but the forward/backward handoff is broken:

while (index >= 0) {
  if (!selectionManager.isDisabled(rows[index].key) && rows[index].type !== 'headerrow') {
    newRow = rows[index];
    break;
  }
  if (index < rows.length - 1) {
    index++;                                  // forward
  } else {
    if (index > parentNode.index) index = parentNode.index;
    index--;                                  // "backward"
  }
}

After the forward pass reaches the end and resets to parentNode.index / decrements, the next iteration's index < rows.length - 1 check sends it forward again. So when no focusable row remains (every row in range is disabled or a headerrow), index bounces between parentNode.index and the end and never reaches index < 0 — an infinite loop that hangs the main thread.

This is reachable whenever the focused row is removed and the new collection has no focusable row — e.g. a table whose rows are momentarily all-disabled loading skeletons while a row was focused. In our app it hard-froze the browser tab on filter/search/tab changes (Firefox "page is slowing down", Chrome unresponsive), independent of row count.

Fix

Replace the single bouncing loop with two bounded passes — forward from the start index, then backward — mirroring the already-correct walk in useListState (getKeyAfter/getKeyBefore). Math.max(0, index) keeps an empty rows array (index === -1) from reading rows[-1], matching the old while (index >= 0) no-op.

Only useGridState is affected; useListState and useTreeState already handle this case safely.

How I tested

Added a regression test in react-aria-components/test/Table.test.js: focus a row, then re-render with that row removed and every remaining row disabled.

  • Verified that without the fix, the new test hangs indefinitely — the infinite loop blocks the main thread so jest never completes (had to kill it).
  • With the fix it passes (~35 ms): focus is cleared and the render completes.
  • The existing supports removing rows test still passes, so focus restoration on normal removal is unchanged.

🧢 Your Project

react-aria-components / @react-stately/grid (Table)

RobHannay and others added 6 commits June 22, 2026 15:52
When the focused row is removed from the collection, the focus-restoration
effect searches collection.rows for a non-disabled, non-headerrow row to
move focus to. The single loop walked forward to the end, then jumped back
to parentNode.index and decremented — but the next iteration's
`index < rows.length - 1` check sent it forward again, so when every row in
the search range was disabled (or a headerrow) it bounced between
parentNode.index and the end forever and never reached `index < 0`. This
hangs the main thread (e.g. a table whose only rows are disabled loading
skeletons while a row is focused).

Replace the bouncing loop with two bounded passes — forward from the start
index, then backward — mirroring the already-correct walk in useListState.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Start the forward scan at Math.max(0, index) so an empty rows array
(index === -1 from Math.min(..., rows.length - 1)) no longer reads
rows[-1], matching the old `while (index >= 0)` no-op.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removing the focused row while every remaining row is disabled previously
infinite-looped in useGridState's focus restoration. Without the fix this
test hangs; with it, focus is cleared and the render completes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant