StickyHeaders.compute() throws "index out of bounds, not enough layouts" — its guard checks getDataLength() but the binary search indexes the layout array
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 7.2k
- Forks
- 392
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 1
Description
Description
StickyHeaders.compute() can throw index out of bounds, not enough layouts from the scroll handler, taking down the tree. Its early-return guard validates the sticky indices against the data array, but the binary search immediately after indexes into the layout array through the unguarded getLayout(index).
Same error and same getLayout family as #2291 and #2440, different call site again — those are the validateItemSize measurement callback and the ViewHolderCollection render path; this one is reached from reportScrollEvent on every scroll tick.
Current behavior
The guard checks getDataLength():
const lengthInvalid =
sortedIndices.length === 0 ||
recyclerViewManager.getDataLength() <=
sortedIndices[sortedIndices.length - 1];
…and then the comparator reads layouts:
const currentIndexInArray = findCurrentStickyIndex(
sortedIndices,
adjustedScrollOffset + stickyHeaderOffset,
(index) => recyclerViewManager.getLayout(index).y
);
Those two arrays are sized in separate passes:
| source | when it updates | |
|---|---|---|
getDataLength() |
propsRef.data?.length — RecyclerViewManager.ts#L346 |
every render via updateProps — useRecyclerViewManager.ts#L16 |
layouts.length |
LayoutManager.layouts |
only in processDataUpdate(), wrapped in if (this.hasLayout()) — RecyclerViewManager.ts#L302-L303 |
So when data grows while layoutManager is still undefined, getDataLength() already reports the new length, lengthInvalid passes, and a scroll event landing in that window calls LayoutManager.getLayout(index) with index >= this.layouts.length, which throws by design:
The guard is effectively checking a different array from the one it protects.
Expected behavior
A sticky index without a layout yet should be skipped or treated as unresolved, not throw. tryGetLayout() already exists for exactly this and returns undefined out of range:
Worth noting this looks like an oversight rather than intent: the same compute() reads layouts twice more and both use the safe accessor — tryGetLayout(newNextStickyIndex)?.y ?? 0 and tryGetLayout(newStickyIndex)?.height ?? 0, plus another at L173. Only the binary-search comparator uses the throwing variant, and it's the one on the hot scroll path.
Suggested fix
Two small changes that reinforce each other:
- Extend the early return to also require a layout for the highest sticky index, so
compute()skips the frame rather than searching a layout array that isn't sized yet:
if (!recyclerViewManager.tryGetLayout(sortedIndices[sortedIndices.length - 1])) {
return;
}
- Move the comparator onto the nullable accessor, matching its siblings:
- (index) => recyclerViewManager.getLayout(index).y
+ (index) => recyclerViewManager.tryGetLayout(index)?.y ?? 0
With (1) in place, (2) is defence-in-depth; on its own a missing layout reads as y = 0 ("already scrolled past"), which can pick the wrong sticky header for one frame and self-corrects on the next compute() once layouts settle. Either is strictly preferable to throwing.
Happy to open a PR if you'd like it in that form.
Reproduction
I don't have a Snack for this one, and I want to be upfront about why rather than pad the issue: it's a timing window between a data commit and the layout pass, observed in production rather than constructed. It needs sticky headers armed on a list whose data grows while hasLayout() is still false, with a scroll event landing inside that window — reliable at scale, fiddly to force deterministically in a small app.
What I can offer instead is the exact mechanism above, which is source-level and checkable by reading: the guard reads getDataLength(), the very next call indexes layouts, and the two are written in different passes with only one of them gated on hasLayout(). The sibling reads in the same function already use tryGetLayout, so the fix is self-consistent with the file.
A unit test would be the honest repro here — drive compute() with a recyclerViewManager whose getDataLength() exceeds its layout count and assert it doesn't throw. Glad to write that if it would help move this along.
Platform
- iOS
- Android
- Web (if applicable)
Not platform-specific — it's in the shared JS. Observed on both; the crash sample below is iOS.
Environment
@shopify/flash-list: 2.3.2 (latest published; bug is also present onmainat the lines linked above, commit435b5141df)react-native: 0.86.3expo: 57.0.20- Architecture: Fabric / new architecture, Hermes
- Scale: ~70 occurrences across ~50 users in 90 days, from the sticky-header scroll path specifically
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/recyclerview/components/StickyHeaders.tsx at StickyHeaders.compute(), then compare its layout reads with tryGetLayout() in the same function. Check RecyclerViewManager.ts for the data and layout accessors. Add coverage using a manager whose data length exceeds its layout count, and verify compute() does not throw during that unresolved-layout window.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react-native, typescript
- Domain
- mobile
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100