fix: prevent unbounded itemRefs growth and ArrowUp crash in Accordion - #5406
Merged
Merged
Conversation
itemRefs was a plain array that getItems() only ever pushed to on every render, so refs from earlier renders (nulled out by React once the key's ref object changes) piled up forever and Home/keyboard nav could target stale, detached refs after a re-render. Switch to a Map keyed by panel key so refs are reused/dropped in sync with the current children, and guard ArrowUp so it no longer indexes itemRefs[-1] when focus is already on the first panel header.
…us bugs Covers pressing ArrowUp while focus is already on the first panel header (previously threw), and keyboard navigation (Home/End) after a re-render has occurred (previously targeted stale, detached refs).
zigzagdev
requested review from
DianaSuvorova,
dyesin,
hualf1995 and
lijim
as code owners
September 2, 2026 23:33
|
|
zigzagdev
changed the title
Collaborator
|
Nice, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1, Fixes #2Description
Accordion'sitemRefswas a plain array thatgetItems()(called on every render) only ever pushed to, without ever resetting it.Because of that, every re-render left the previous render's refs which React had already nulled out (
.current = null) once it swapped in new ref objects for the same keyed panels sitting in the array forever, growing it without bound.As a result, once any re-render happened (e.g. expanding a panel), pressing
Homewould silently stop moving focus because it kept reading a stale, detached ref.Also fixed a related crash: pressing
ArrowUpwhile focus was already on the first panel header indexed intoitemRefs[-1], throwing aTypeError.itemRefs(array) with aMapkeyed by each panel's key, so it's rebuilt in sync with the current children on every render and reuses the existing ref as long as the key hasn't changed.activeItemIdx > 0guard onArrowUpso it no longer crashes when focus is on the first panel.accordion.e2e.tscovering both fixes (@playwright/testwasn't installed in this environment, so I couldn't run them locally — please confirm they pass in CI)Scope