Repository navigation
Conversation
|
I have signed the CLA! |
… photo The mispark is upstream, not ours: Shopify/flash-list#2307, open and P1. `applyInitialScrollAdjustment` recomputes layouts only as far as the opening index, which leaves the layout array out of order at that boundary — and the binary search in `getVisibleLayouts` assumes it is sorted, so it resolves an unrelated item. It needs items larger than the 200pt default estimate to bite, which a full-width slide always is, and it arrived in 2.3.1 with the reorder that reads the target offset after recomputing rather than before. Measured here on the example's gallery, twenty-two slides at 420pt opening on index 14: the scroll is *correct* — `offset 5880`, and `getLayout(14).x` is also 5880 — but the content is 6380 against a true 9240 and the item resolved under that offset is 21. Every signal from outside reads healthy, which is why this looked like a timing problem for so long. This carries the one line from the upstream fix, Shopify/flash-list#2444: recomputeLayouts(0, initialScrollIndex) → recomputeLayouts(0, this.getDataLength() - 1) Nothing in `src/` changes. The slider stays exactly as published, and the patch goes when a release carries the fix. The version is also locked now. `example/package.json` asked for 2.3.2 while the lockfile still resolved 2.0.2, so a clean install produced the version where the defect does not exist — the reproduction would have quietly stopped reproducing. The root devDependency moves to 2.3.2 as well, so the library is typechecked against the version its consumers actually get.
|
We hit #2307 in production after upgrading 2.0.2 → 2.3.3, and this one-line change fixes it for us. Device repro below, in case it helps review. Setup: FlashList 2.3.3, React Native 0.86.3 (New Architecture), Expo SDK 57, iPhone 17 simulator on iOS 27.0. A bare FlashList of 150 numbered items, opened with
In the pager case, the scroll offset and The same thing shows in a Node run of We're shipping this as a patch-package patch on 2.3.3 for now. It would be great to see this (or #2318, which is the same change) merged. |
Fixes #2307
The problem
applyInitialScrollAdjustmentrecomputes layouts for[0, initialScrollIndex]and stops there. Items are positioned from their predecessor, so this moves the target item without moving anything after it, and the layout array is left out of order at that boundary.getVisibleLayoutsbinary searches that array.binarySearchVisibleIndexdocuments that it assumes the array is sorted, so once it is not, the search does not return a slightly wrong answer, it returns an unrelated one.With 600 items of 300px and
initialScrollIndex={250}:The search for offset 75000 then returns item 333, which matches what the issue reports on device.
The gap only opens when items are taller than the 200px the average window starts at. Below that the corrective pass moves items to a lower position, which leaves a forward gap and keeps the array sorted, which is why smaller items never show the bug.
The change
Recompute to the end of the list instead of stopping at
initialScrollIndex.This is the same range the layout managers already use when window size changes, and Masonry already recomputes to the end regardless of the range it is given.
_recomputeLayoutshas tail repair for partial recomputes, but this call site goes to the publicrecomputeLayoutsand bypasses it, and its condition compares the last item rather than the boundary item so it would not fire here anyway.I kept the change at the call site because the ordering only breaks when the prefix is corrected against a changed average, and positions chain, so anything after the target genuinely has to be recomputed. Happy to move it into
_recomputeLayoutsas a boundary check instead if you would rather have it there, though that path is shared with Masonry where offsets are not ordered by index by design.Test
src/__tests__/initialScrollIndex2307.test.tsxrenders the case from the issue with items measuring 300px and asserts item 250 is rendered and item 333 is not. It fails on main and passes with this change.Full suite is green, 188 tests across 15 suites, and type check passes.