Skip to content

Commit da709a1

Browse files
mozziusclaude
andcommitted
Anchor VirtualizedList interior spacers to the cell before them
An interior spacer whose first cell is unmeasured started at averageCellLength * index instead of where the rendered cell before it ends. After a large prepend under maintainVisibleContentPosition, the cells after it then moved each time a cell mounted or unmounted at the render window's leading edge, and the list oscillated indefinitely. Fixes #58870. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent bf62cce commit da709a1

2 files changed

Lines changed: 146 additions & 1 deletion

File tree

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,25 @@ class VirtualizedList extends StateSafePureComponent<
878878
_getSpacerKey = (isVertical: boolean): string =>
879879
isVertical ? 'height' : 'width';
880880

881+
// An unmeasured cell's approximate offset can be the average cell length
882+
// times its index, which ignores where the rendered cells before it end. A
883+
// spacer starting there moves the cells after it whenever the average
884+
// changes or a cell mounts or unmounts at its end. Start it where the cell
885+
// before it ends instead. (Not where the cell after it starts: that would
886+
// include any gap or margin between cells.)
887+
_getSpacerStart(first: number, approxOffset: number): number {
888+
if (
889+
first > 0 &&
890+
this._listMetrics.getCellMetrics(first, this.props) == null
891+
) {
892+
const before = this._listMetrics.getCellMetrics(first - 1, this.props);
893+
if (before != null) {
894+
return before.offset + before.length;
895+
}
896+
}
897+
return approxOffset;
898+
}
899+
881900
static _keyExtractor(
882901
item: Item,
883902
index: number,
@@ -1038,7 +1057,9 @@ class VirtualizedList extends StateSafePureComponent<
10381057
this.props,
10391058
);
10401059
const spacerSize =
1041-
lastMetrics.offset + lastMetrics.length - firstMetrics.offset;
1060+
lastMetrics.offset +
1061+
lastMetrics.length -
1062+
this._getSpacerStart(section.first, firstMetrics.offset);
10421063
cells.push(
10431064
<View
10441065
key={`$spacer-${section.first}`}

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2240,6 +2240,102 @@ it('renders full tail spacer if all cells measured', async () => {
22402240
expect(component).toMatchSnapshot();
22412241
});
22422242

2243+
// The cells kept by initialNumToRender are followed by a spacer up to the
2244+
// render window. When the spacer's first cell has never been measured at its
2245+
// index, the spacer should still start where the cell before it ends, not at
2246+
// `averageCellLength * index`, or the cells after it move whenever the average
2247+
// does, and maintainVisibleContentPosition has to chase them.
2248+
it.each([0, 10])(
2249+
'starts an interior spacer where the cell before it ends (gap: %d)',
2250+
async gap => {
2251+
const items = generateItems(40);
2252+
// The kept cells are taller than the rest, so the average cell length
2253+
// doesn't predict where they end.
2254+
const lengths = new Map(
2255+
items.map(item => [item.key, item.key < 2 ? 200 : 100]),
2256+
);
2257+
const list = extraData => (
2258+
<VirtualizedList
2259+
initialNumToRender={2}
2260+
windowSize={1}
2261+
extraData={extraData}
2262+
{...baseItemProps(items)}
2263+
/>
2264+
);
2265+
2266+
let component;
2267+
await act(() => {
2268+
component = create(list(0));
2269+
});
2270+
2271+
await act(() => {
2272+
simulateCellLayout(component, items, 0, {
2273+
width: 10,
2274+
height: 200,
2275+
x: 0,
2276+
y: 0,
2277+
});
2278+
simulateCellLayout(component, items, 1, {
2279+
width: 10,
2280+
height: 200,
2281+
x: 0,
2282+
y: 200 + gap,
2283+
});
2284+
// The list jumped past cells 2-9, so they have never been measured.
2285+
for (let i = 10; i < 20; i++) {
2286+
simulateCellLayout(component, items, i, {
2287+
width: 10,
2288+
height: 100,
2289+
x: 0,
2290+
y: 1200 + (100 + gap) * (i - 10),
2291+
});
2292+
}
2293+
simulateLayout(component, {
2294+
viewport: {width: 10, height: 50},
2295+
content: {width: 10, height: 5000},
2296+
});
2297+
// Scroll into the middle of cell 12.
2298+
simulateScroll(component, {x: 0, y: 1425 + 2 * gap});
2299+
performAllBatches();
2300+
});
2301+
2302+
expect(component.getInstance().state.cellsAroundViewport).toEqual({
2303+
first: 12,
2304+
last: 12,
2305+
});
2306+
// From where cell 1 ends to where cell 11 ends.
2307+
expect(getSpacerLengths(component)[0]).toBe(1000);
2308+
2309+
// Measuring another cell changes the average cell length, but not the
2310+
// spacer.
2311+
lengths.set(20, 300);
2312+
await act(() => {
2313+
simulateCellLayout(component, items, 20, {
2314+
width: 10,
2315+
height: 300,
2316+
x: 0,
2317+
y: 1200 + (100 + gap) * 10,
2318+
});
2319+
});
2320+
await act(() => {
2321+
component.update(list(1));
2322+
});
2323+
expect(getSpacerLengths(component)[0]).toBe(1000);
2324+
2325+
// Nor does laying the list out again, including any gap between cells.
2326+
for (let pass = 2; pass < 5; pass++) {
2327+
await act(() => {
2328+
simulateLayoutPass(component, items, lengths, gap);
2329+
performAllBatches();
2330+
});
2331+
await act(() => {
2332+
component.update(list(pass));
2333+
});
2334+
expect(getSpacerLengths(component)[0]).toBe(1000);
2335+
}
2336+
},
2337+
);
2338+
22432339
it('renders windowSize derived region at top', async () => {
22442340
const items = generateItems(10);
22452341
const ITEM_HEIGHT = 10;
@@ -3141,6 +3237,34 @@ function simulateCellLayout(component, items, itemIndex, dimensions) {
31413237
);
31423238
}
31433239

3240+
// Lays out the rendered cells and spacers one after another, as native layout
3241+
// would, with `gap` between each.
3242+
function simulateLayoutPass(component, items, lengths, gap) {
3243+
let offset = 0;
3244+
for (const child of component.toJSON().children[0].children) {
3245+
if (child.children == null) {
3246+
offset += child.props.style.height + gap;
3247+
continue;
3248+
}
3249+
const key = child.children[0].props.value;
3250+
const length = lengths.get(key);
3251+
simulateCellLayout(
3252+
component,
3253+
items,
3254+
items.findIndex(item => item.key === key),
3255+
{width: 10, height: length, x: 0, y: offset},
3256+
);
3257+
offset += length + gap;
3258+
}
3259+
}
3260+
3261+
function getSpacerLengths(component) {
3262+
return component
3263+
.toJSON()
3264+
.children[0].children.filter(child => child.children == null)
3265+
.map(child => child.props.style.height);
3266+
}
3267+
31443268
function simulateScroll(component, position) {
31453269
component.getInstance()._onScroll({
31463270
nativeEvent: {

0 commit comments

Comments
 (0)