Skip to content

Commit c1d4a48

Browse files
mozziusclaude
andcommitted
Fix VirtualizedList unmounting the maintainVisibleContentPosition anchor before its correction
A cell's layout event reaches JS before the scroll event carrying native maintainVisibleContentPosition's correction for the same commit. A cells update in between computes the render window from the old offset and the new cell metrics, so when the content above the anchor changed size by a lot, the window can skip the visible row and unmount it. Hold the window while the anchor's move is uncorrected, as for a pending prepend. Fixes #58921 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent df5aa64 commit c1d4a48

2 files changed

Lines changed: 129 additions & 2 deletions

File tree

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

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@
88
* @format
99
*/
1010

11-
import type {CellMetricProps, ListOrientation} from './ListMetricsAggregator';
11+
import type {
12+
CellMetricProps,
13+
CellMetrics,
14+
ListOrientation,
15+
} from './ListMetricsAggregator';
1216
import type {ViewToken} from './ViewabilityHelper';
1317
import type {
1418
Item,
@@ -631,7 +635,7 @@ class VirtualizedList extends StateSafePureComponent<
631635
} else {
632636
// If we have a pending scroll update, we should not adjust the render window as it
633637
// might override the correct window.
634-
if (pendingScrollUpdateCount > 0) {
638+
if (pendingScrollUpdateCount > 0 || this._pendingAnchorCorrection) {
635639
return cellsAroundViewport.last >= getItemCount(data)
636640
? VirtualizedList._constrainToItemCount(cellsAroundViewport, props)
637641
: cellsAroundViewport;
@@ -1242,6 +1246,10 @@ class VirtualizedList extends StateSafePureComponent<
12421246
_nestedChildLists: ChildListCollection<VirtualizedList> =
12431247
new ChildListCollection();
12441248
_offsetFromParentVirtualizedList: number = 0;
1249+
// Set when the cell maintainVisibleContentPosition is anchored on moves, until
1250+
// the scroll event with native's correction for it. Not state, so that cells
1251+
// updates queued by earlier layout events in the same batch see it too.
1252+
_pendingAnchorCorrection: boolean = false;
12451253
_pendingViewabilityUpdate: boolean = false;
12461254
_prevParentOffset: number = 0;
12471255
_scrollMetrics: {
@@ -1328,6 +1336,8 @@ class VirtualizedList extends StateSafePureComponent<
13281336
cellKey: string,
13291337
cellIndex: number,
13301338
): void => {
1339+
const anchorMetrics =
1340+
this._getMaintainVisibleContentPositionAnchor(cellIndex);
13311341
const layoutHasChanged = this._listMetrics.notifyCellLayout({
13321342
cellIndex,
13331343
cellKey,
@@ -1336,6 +1346,18 @@ class VirtualizedList extends StateSafePureComponent<
13361346
});
13371347

13381348
if (layoutHasChanged) {
1349+
if (
1350+
anchorMetrics != null &&
1351+
this._listMetrics.getCellMetrics(cellIndex, this.props)?.offset !==
1352+
anchorMetrics.offset
1353+
) {
1354+
// Native maintainVisibleContentPosition shifts the scroll offset by as
1355+
// much as this cell moved, but the scroll event reporting that arrives
1356+
// after this layout event. Until then the scroll offset is stale
1357+
// relative to the cell metrics, and a window computed from the two is
1358+
// off by the shift.
1359+
this._pendingAnchorCorrection = true;
1360+
}
13391361
this._scheduleCellsToRenderUpdate();
13401362
}
13411363

@@ -1344,6 +1366,34 @@ class VirtualizedList extends StateSafePureComponent<
13441366
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
13451367
};
13461368

1369+
/**
1370+
* The metrics of the cell at `cellIndex` if native
1371+
* `maintainVisibleContentPosition` is anchored on it: a mounted cell laid out
1372+
* across the start of the viewport.
1373+
*/
1374+
_getMaintainVisibleContentPositionAnchor(cellIndex: number): ?CellMetrics {
1375+
const {data, getItemCount, getItemLayout, maintainVisibleContentPosition} =
1376+
this.props;
1377+
if (
1378+
maintainVisibleContentPosition == null ||
1379+
getItemLayout != null ||
1380+
// Native picks its anchor by physical position, not flow-relative, so in
1381+
// a horizontal RTL list it isn't this cell.
1382+
this._isHorizontalRTL() ||
1383+
cellIndex >= getItemCount(data)
1384+
) {
1385+
return null;
1386+
}
1387+
const metrics = this._listMetrics.getCellMetrics(cellIndex, this.props);
1388+
const {offset} = this._scrollMetrics;
1389+
return metrics != null &&
1390+
metrics.isMounted &&
1391+
metrics.offset <= offset &&
1392+
offset < metrics.offset + metrics.length
1393+
? metrics
1394+
: null;
1395+
}
1396+
13471397
_onCellFocusCapture = (cellKey: string) => {
13481398
this._lastFocusedCellKey = cellKey;
13491399
if (ReactNativeFeatureFlags.deferFlatListFocusChangeRenderUpdate()) {
@@ -1783,6 +1833,7 @@ class VirtualizedList extends StateSafePureComponent<
17831833
visibleLength,
17841834
zoomScale,
17851835
};
1836+
this._pendingAnchorCorrection = false;
17861837
if (this.state.pendingScrollUpdateCount > 0) {
17871838
this.setState<'pendingScrollUpdateCount'>({pendingScrollUpdateCount: 0});
17881839
}

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3048,6 +3048,82 @@ it('handles rapid prepends with coalesced scroll event (regression for #53542)',
30483048
).toBeGreaterThanOrEqual(0);
30493049
});
30503050

3051+
// Trigger: Content above the cell at the start of the viewport changes size, e.g. cells mounted above it are
3052+
// taller than estimated. Native MVCP moves the scroll offset by the same amount, but the layout events reach
3053+
// JS before the scroll event, and a cells update runs in between.
3054+
// Expected: The render window waits for the corrected offset and keeps the anchor cell rendered.
3055+
it('waits for the maintainVisibleContentPosition correction when the anchor cell moves', async () => {
3056+
const items = generateItems(40);
3057+
const ITEM_HEIGHT = 10;
3058+
3059+
let component;
3060+
await act(() => {
3061+
component = create(
3062+
<VirtualizedList
3063+
initialNumToRender={1}
3064+
windowSize={1}
3065+
maintainVisibleContentPosition={{minIndexForVisible: 0}}
3066+
{...baseItemProps(items)}
3067+
/>,
3068+
);
3069+
});
3070+
const instance = component.getInstance();
3071+
// Lays out the rendered cells: cell 0 (kept for scroll-to-top) and the
3072+
// render window, `shift` further down than their index says.
3073+
const layoutRenderedCells = (shift = 0) => {
3074+
const {first, last} = instance.state.cellsAroundViewport;
3075+
const indices = [0];
3076+
for (let i = Math.max(1, first); i <= last; i++) {
3077+
indices.push(i);
3078+
}
3079+
for (const i of indices) {
3080+
simulateCellLayout(component, items, i, {
3081+
width: 10,
3082+
height: ITEM_HEIGHT,
3083+
x: 0,
3084+
y: i * ITEM_HEIGHT + shift,
3085+
});
3086+
}
3087+
};
3088+
3089+
await act(() => {
3090+
simulateLayout(component, {
3091+
viewport: {width: 10, height: 50},
3092+
content: {width: 10, height: items.length * ITEM_HEIGHT},
3093+
});
3094+
layoutRenderedCells();
3095+
simulateScroll(component, {x: 0, y: 205});
3096+
performAllBatches();
3097+
});
3098+
await act(() => {
3099+
layoutRenderedCells();
3100+
performAllBatches();
3101+
});
3102+
3103+
// Cell 20 is laid out across the start of the viewport.
3104+
expect(instance.state.cellsAroundViewport).toEqual({first: 20, last: 25});
3105+
3106+
// Content above the cells grows by 500 (e.g. a header), moving cell 20 down
3107+
// by 500. A cells update runs before the scroll event with native's
3108+
// correction.
3109+
await act(() => {
3110+
layoutRenderedCells(500);
3111+
performAllBatches();
3112+
});
3113+
3114+
// The offset (205) is now 495 above cell 20. The window waits for the
3115+
// correction rather than moving there and unmounting cell 20.
3116+
expect(instance.state.cellsAroundViewport).toEqual({first: 20, last: 25});
3117+
3118+
await act(() => {
3119+
simulateScroll(component, {x: 0, y: 705});
3120+
performAllBatches();
3121+
});
3122+
3123+
expect(instance.state.cellsAroundViewport.first).toBeLessThanOrEqual(20);
3124+
expect(instance.state.cellsAroundViewport.last).toBeGreaterThanOrEqual(20);
3125+
});
3126+
30513127
function generateItems(count, startKey = 0) {
30523128
return Array(count)
30533129
.fill()

0 commit comments

Comments
 (0)