Conversation
ViewHolderCollection reads every render stack entry through the throwing getLayout accessor, so an entry that outlives a layout-table shrink crashes the render with "index out of bounds, not enough layouts". modifyChildrenLayout truncates the layout table on its first line, but the render stack is only pruned on some of its exit paths. When the engaged range is unchanged, recomputeEngagedIndices returns undefined and sync() never runs against the new data length, leaving keys that point past the last layout. Read layouts through tryGetLayout and skip the entries that have none, matching the bounds-safe pattern StickyHeaders already uses. Stale entries are dropped until the next sync restores the pairing. Fixes Shopify#2440
teoarjun
marked this pull request as ready for review
September 25, 2026 11:17
Author
|
I have signed the CLA! |
This branch has not been deployed
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
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.
Description
Fixes #2440
ViewHolderCollectionreads every render stack entry through the throwinggetLayoutaccessor, so an entry that outlives a layout-table shrink crashes the render withindex out of bounds, not enough layouts.RecyclerViewManager.modifyChildrenLayouttruncates the layout table on its first line, but the render stack is only pruned on some of its exit paths. When the engaged range is unchanged,recomputeEngagedIndices()returnsundefinedandRenderStackManager.sync()never runs against the new data length, leaving keys that point past the last layout.This routes the render path through the existing
tryGetLayoutaccessor and skips entries that have no layout, matching the bounds-safe patternStickyHeadersalready uses. Stale entries are dropped until the next sync restores the pairing.Reviewers' hat-rack 🎩
modifyChildrenLayoutwould address the stale state directly, but reaches into the engaged-indices path — happy to take that route instead if you'd prefer it.src/__tests__/ViewHolderCollection.test.tsxis new. It renders a stack holding indices 0-24 against a layout table truncated to 10 and asserts only the 10 live entries render; it fails without the fix, rendering all 25.Verified locally on the branch: 201 tests pass across 16 suites,
tsc --noEmitclean,eslintclean.Screenshots or videos (if needed)
Not applicable — no visual change; the fix prevents a render-time crash.