Skip to content

fix: reset pendingStartReached when data changes - #2429

Open
sergeymild wants to merge 1 commit into
Shopify:mainfrom
sergeymild:fix/pending-start-reached-reset-on-data-change
Open

fix: reset pendingStartReached when data changes#2429
sergeymild wants to merge 1 commit into
Shopify:mainfrom
sergeymild:fix/pending-start-reached-reset-on-data-change

Conversation

@sergeymild

Copy link
Copy Markdown

Summary

pendingEndReached is reset to false on every data change, but pendingStartReached is not:

// Reset end reached state when data changes
useMemo(() => {
    pendingEndReached.current = false;
    // needs to run only when data changes
}, [data]);

onStartReached/onEndReached are edge-triggered off these refs:

const isNearStart = lastScrollOffset <= startThresholdDistance;
if (isNearStart && !pendingStartReached.current) {
    pendingStartReached.current = true;
    onStartReached();
}
pendingStartReached.current = isNearStart;

In a normal "load older items on scroll to top" pagination flow (inverted list, maintainVisibleContentPosition, onStartReached prepending a page of older items to data), we saw onStartReached stop firing entirely after a handful of successful backward loads — no further call, no error, nothing — until the list was scrolled away from the start past the threshold and back.

Suspected mechanism

After onStartReached fires and the app prepends older items, maintainVisibleContentPosition adjusts the scroll offset to keep the visible content stable. Depending on timing, the checkBounds pass right after that adjustment can still read lastScrollOffset as within startThresholdDistance (especially with a multi-viewport onStartReachedThreshold), so isNearStart never dips to false between one onStartReached call and the next data update. Since pendingStartReached isn't part of the [data] reset (unlike pendingEndReached), the latch then stays true and blocks every subsequent call.

This is timing/threshold-dependent, so it doesn't reproduce on every load — in our repro it took ~8 successful backward loads before it latched.

Fix

Reset pendingStartReached in the same place pendingEndReached is reset, mirroring the existing behavior for the end-of-list case.

Testing

Verified manually against a production chat screen with 1000+ items: before this change, repeated scroll-to-top pagination reliably stopped firing onStartReached after ~8 consecutive backward loads. With the fix, 25+ consecutive backward loads succeeded with no stall.

I don't have a minimal standalone repro repo, but can put one together if that's useful for review.

  • @shopify/flash-list: 2.3.2
  • react-native: 0.84.1
  • Platform: Android (physical device)

pendingEndReached is reset on every data change, but pendingStartReached
is not. onStartReached is edge-triggered off pendingStartReached, so once
it latches true it only re-arms when isNearStart briefly reads false —
which the maintainVisibleContentPosition offset adjustment after a
backward prepend doesn't reliably guarantee before the next bounds check.

Observed in an inverted list with maintainVisibleContentPosition +
onStartReached prepending pages of older items: after a handful of
successful backward loads, onStartReached stopped firing entirely, with
no error and no further callback invocations, until the list was
scrolled away from the start and back. Resetting pendingStartReached
alongside pendingEndReached on data change closes that race.
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