Skip to content

Commit 5cb6524

Browse files
kulkarni-rohanmeta-codesync[bot]
authored andcommitted
Fix maintainVisibleContentPosition with rapid data updates (#53542) (#57955)
Summary: Fixes #53542 **Problem:** `maintainVisibleContentPosition` fails when FlatList data is updated rapidly with prepends in quick succession (e.g., chat receiving messages). After 2 prepends before native scroll drains, render window stays frozen, `onViewableItemsChanged` suppressed, `onEndReached` never fires. **Root cause:** `pendingScrollUpdateCount` assumption is structurally unsound – it increments by 1 per prepend in `getDerivedStateFromProps` (0→1→2) but native scroll events are dispatched as unique events that coalesce. Verified in C++: - `ScrollViewEventEmitter.cpp:13` – `onScroll` dispatched with `dispatchUniqueEvent("scroll", ...)` - `EventQueue.cpp:29-50` – unique event whose target+type matches existing replaces in place (line 49) instead of appending So N scroll events emitted before queue flush deliver exactly 1 to JS. Single coalesced event drains 2→1 leaving blocked. Also leak when JS predicate fires (old key found at new index) but native declines to adjust (tag recycled, view deleted, delta ≤0.5, clamped). **Fix (1 file, 2 lines core):** Clamp pending to at most 1 and drain to 0 on any scroll: - `pendingScrollUpdateCount: 1` instead of `prev+1` – prevents accumulation during rapid prepends - `setState({pendingScrollUpdateCount: 0})` instead of `-1` – single coalesced scroll unblocks This turns "stuck at N" into unblocked after next scroll, fixing frozen window / viewability / onEndReached for rapid updates. For leak path where native declines (no scroll ever), flag would still be stranded at 1 – addressed by boolean rename + escape hatch in follow-up, but this PR already strictly improves and matches existing tests that drain 0→1→0. ## Changelog: [GENERAL] [FIXED] - Fix maintainVisibleContentPosition with rapid data updates (#53542) Pull Request resolved: #57955 Test Plan: **Jest (VirtualizedList):** ```bash yarn jest --watchman=false packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js --no-coverage --ci # Before: new coalesced test fails with pending 1 # After: 83 passed (82 existing + 1 new rapid prepends with coalesced scroll), 1 skipped, 59 snapshots ``` **New test ( regression for #53542 ):** - Simulates 2 rapid prepends WITHOUT intermediate scroll (5 + 3 items) - Only 1 coalesced scroll event (delta 8*ITEM_HEIGHT) - Asserts `pendingScrollUpdateCount === 0` (fails on main with 1) and `firstVisibleItemKey` not null **Existing MVCP tests still pass:** - `handles maintainVisibleContentPosition` - `handles multiple rapid prepends` (separate scroll per prepend) - `delta stays bounded` - `minIndexForVisible >0` and `inverted` **Lint:** ```bash yarn lint # Done (max-warnings 0) ``` Closes #53542 Reviewed By: javache Differential Revision: D115907610 Pulled By: fabriziocucci fbshipit-source-id: 0cd59b35d7c43803a000b93779cd84c9c4202053
1 parent f63b2a1 commit 5cb6524

2 files changed

Lines changed: 79 additions & 4 deletions

File tree

‎packages/virtualized-lists/Lists/VirtualizedList.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ class VirtualizedList extends StateSafePureComponent<
777777
firstVisibleItemKey: newFirstVisibleItemKey,
778778
pendingScrollUpdateCount:
779779
maintainVisibleContentPositionAdjustment != null
780-
? prevState.pendingScrollUpdateCount + 1
780+
? 1
781781
: prevState.pendingScrollUpdateCount,
782782
};
783783
}
@@ -1760,9 +1760,7 @@ class VirtualizedList extends StateSafePureComponent<
17601760
zoomScale,
17611761
};
17621762
if (this.state.pendingScrollUpdateCount > 0) {
1763-
this.setState<'pendingScrollUpdateCount'>(state => ({
1764-
pendingScrollUpdateCount: state.pendingScrollUpdateCount - 1,
1765-
}));
1763+
this.setState<'pendingScrollUpdateCount'>({pendingScrollUpdateCount: 0});
17661764
}
17671765
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
17681766
if (!this.props) {

‎packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2877,6 +2877,83 @@ it('maintainVisibleContentPosition with inverted VirtualizedList handles prepend
28772877
expect(anchorAfterPrepend).toBeLessThanOrEqual(anchorBeforePrepend + 10);
28782878
});
28792879

2880+
it('handles rapid prepends with coalesced scroll event (regression for #53542)', async () => {
2881+
const items = generateItems(20);
2882+
const ITEM_HEIGHT = 10;
2883+
2884+
let component;
2885+
await act(() => {
2886+
component = create(
2887+
<VirtualizedList
2888+
initialNumToRender={1}
2889+
windowSize={1}
2890+
maintainVisibleContentPosition={{minIndexForVisible: 0}}
2891+
{...baseItemProps(items)}
2892+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
2893+
/>,
2894+
);
2895+
});
2896+
2897+
await act(() => {
2898+
simulateLayout(component, {
2899+
viewport: {width: 10, height: 50},
2900+
content: {width: 10, height: items.length * ITEM_HEIGHT},
2901+
});
2902+
simulateScroll(component, {x: 0, y: 50});
2903+
performAllBatches();
2904+
});
2905+
2906+
const afterFirstPrepend = [...generateItems(5, items.length), ...items];
2907+
const afterSecondPrepend = [
2908+
...generateItems(3, afterFirstPrepend.length),
2909+
...afterFirstPrepend,
2910+
];
2911+
2912+
// Two rapid prepends WITHOUT intermediate scroll (coalesced native event)
2913+
await act(() => {
2914+
component.update(
2915+
<VirtualizedList
2916+
initialNumToRender={1}
2917+
windowSize={1}
2918+
maintainVisibleContentPosition={{minIndexForVisible: 0}}
2919+
{...baseItemProps(afterFirstPrepend)}
2920+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
2921+
/>,
2922+
);
2923+
});
2924+
2925+
await act(() => {
2926+
component.update(
2927+
<VirtualizedList
2928+
initialNumToRender={1}
2929+
windowSize={1}
2930+
maintainVisibleContentPosition={{minIndexForVisible: 0}}
2931+
{...baseItemProps(afterSecondPrepend)}
2932+
{...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
2933+
/>,
2934+
);
2935+
});
2936+
2937+
// Only ONE coalesced scroll event for both prepends (delta 8*ITEM_HEIGHT)
2938+
// This simulates EventQueue coalescing: dispatchUniqueEvent replaces previous scroll
2939+
// Previously: pending 0→1→2, scroll 2→1 (still blocked). Now: 0→1→1→0 (unblocked)
2940+
await act(() => {
2941+
simulateContentLayout(component, {
2942+
width: 10,
2943+
height: afterSecondPrepend.length * ITEM_HEIGHT,
2944+
});
2945+
simulateScroll(component, {x: 0, y: 50 + 8 * ITEM_HEIGHT});
2946+
performAllBatches();
2947+
});
2948+
2949+
// Pending should be 0, not 1 – this fails on main before fix
2950+
expect(component.getInstance().state.pendingScrollUpdateCount).toBe(0);
2951+
expect(component.getInstance().state.firstVisibleItemKey).not.toBeNull();
2952+
expect(
2953+
component.getInstance().state.cellsAroundViewport.first,
2954+
).toBeGreaterThanOrEqual(0);
2955+
});
2956+
28802957
function generateItems(count, startKey = 0) {
28812958
return Array(count)
28822959
.fill()

0 commit comments

Comments
 (0)