Skip to content

Commit ef69ea3

Browse files
authored
fix(virtual-core): adjust scroll on first measurement during backward scroll (#1199)
The default shouldAdjustScroll predicate skipped scroll compensation for any above-viewport resize while scrolling backward. That over-applied to items being measured for the first time: their estimate→actual delta lives above the viewport and must be compensated, or the anchored content jumps when scrolling up into never-seen rows. Distinguish first measurement (!itemSizeCache.has(key)) — always compensate — from re-measurement, which still skips during backward scroll to avoid the cascading "items fight the scroll" jank.
1 parent 932c358 commit ef69ea3

8 files changed

Lines changed: 231 additions & 14 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/virtual-core': patch
3+
---
4+
5+
Fix "items jump while scrolling up": the default scroll-adjustment predicate now compensates scrollTop on the first measurement of an above-viewport item even while scrolling backward (the estimate→actual delta must be absorbed), and only skips compensation for re-measurements during backward scroll to avoid the cascading jank

examples/react/chat/src/index.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ button:hover {
7070
.Messages {
7171
min-height: 0;
7272
overflow: auto;
73+
overflow-anchor: none;
7374
width: 100%;
7475
}
7576

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
</head>
6+
<body>
7+
<div id="root"></div>
8+
<script type="module" src="./main.tsx"></script>
9+
</body>
10+
</html>
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import React from 'react'
2+
import ReactDOM from 'react-dom/client'
3+
import { useVirtualizer } from '@tanstack/react-virtual'
4+
5+
// Deterministic sizing: every row is ITEM_SIZE tall while the virtualizer
6+
// estimates ESTIMATE. The gap means every above-viewport item that gets
7+
// measured for the first time produces a predictable estimate→actual delta,
8+
// which is exactly the scenario where scrolling up must compensate scrollTop
9+
// to keep the anchored content visually stable.
10+
const ITEM_SIZE = 90
11+
const ESTIMATE = 50
12+
13+
const App = () => {
14+
const parentRef = React.useRef<HTMLDivElement>(null)
15+
const initialOffset = Number(
16+
new URLSearchParams(window.location.search).get('initialOffset') ?? 0,
17+
)
18+
const rowVirtualizer = useVirtualizer({
19+
count: 1000,
20+
getScrollElement: () => parentRef.current,
21+
estimateSize: () => ESTIMATE,
22+
initialOffset,
23+
})
24+
25+
return (
26+
<div
27+
ref={parentRef}
28+
id="scroll-container"
29+
style={{ height: 400, overflow: 'auto' }}
30+
>
31+
<div
32+
style={{
33+
height: rowVirtualizer.getTotalSize(),
34+
position: 'relative',
35+
}}
36+
>
37+
{rowVirtualizer.getVirtualItems().map((v) => (
38+
<div
39+
key={v.key}
40+
data-testid={`item-${v.index}`}
41+
ref={rowVirtualizer.measureElement}
42+
data-index={v.index}
43+
style={{
44+
position: 'absolute',
45+
top: 0,
46+
left: 0,
47+
transform: `translateY(${v.start}px)`,
48+
width: '100%',
49+
}}
50+
>
51+
<div style={{ height: ITEM_SIZE }}>Row {v.index}</div>
52+
</div>
53+
))}
54+
</div>
55+
</div>
56+
)
57+
}
58+
59+
ReactDOM.createRoot(document.getElementById('root')!).render(<App />)
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { expect, test } from '@playwright/test'
2+
3+
// Records the data-index and viewport-relative top of a rendered item that
4+
// sits clearly inside the viewport (so it is already measured) with rows above
5+
// it inside the viewport and room to stay visible after a moderate upward
6+
// scroll moves it down. The window is wider than a row (rows are 90px) so it
7+
// always contains at least one candidate; we pick the one nearest the target.
8+
const pickAnchor = () => {
9+
const container = document.querySelector('#scroll-container')
10+
if (!container) throw new Error('Container not found')
11+
const containerTop = container.getBoundingClientRect().top
12+
13+
const TARGET = 100
14+
const candidates = Array.from(container.querySelectorAll('[data-index]'))
15+
.map((el) => ({
16+
index: Number(el.getAttribute('data-index')),
17+
top: el.getBoundingClientRect().top - containerTop,
18+
}))
19+
// Away from both edges: rows above it, and room to move down ~200px.
20+
.filter((c) => c.top > 30 && c.top < 170)
21+
.sort((a, b) => Math.abs(a.top - TARGET) - Math.abs(b.top - TARGET))
22+
23+
if (candidates.length === 0) throw new Error('No anchor candidate found')
24+
return candidates[0]
25+
}
26+
27+
const topOfIndex = (index: number) => {
28+
const container = document.querySelector('#scroll-container')
29+
if (!container) throw new Error('Container not found')
30+
const el = container.querySelector(`[data-index="${index}"]`)
31+
if (!el) return null
32+
return el.getBoundingClientRect().top - container.getBoundingClientRect().top
33+
}
34+
35+
// Largest gap between consecutive rendered rows. Before measurement, rows are
36+
// positioned from the 50px estimate while their real height is 90px, so they
37+
// overlap (gap ~40px); once measured, positions are contiguous (gap ~0). This
38+
// is a direct, pollable signal that measurement has settled. Returns a large
39+
// finite sentinel (not Infinity — page.evaluate serializes that to null) when
40+
// the DOM isn't ready yet so the poll keeps waiting.
41+
const NOT_READY = 1e9
42+
const maxRowGap = () => {
43+
const container = document.querySelector('#scroll-container')
44+
if (!container) return NOT_READY
45+
const containerTop = container.getBoundingClientRect().top
46+
const rows = Array.from(container.querySelectorAll('[data-index]'))
47+
.map((el) => {
48+
const rect = el.getBoundingClientRect()
49+
return {
50+
top: rect.top - containerTop,
51+
bottom: rect.bottom - containerTop,
52+
}
53+
})
54+
.sort((a, b) => a.top - b.top)
55+
if (rows.length < 2) return NOT_READY
56+
let gap = 0
57+
for (let i = 1; i < rows.length; i++) {
58+
gap = Math.max(gap, Math.abs(rows[i].top - rows[i - 1].bottom))
59+
}
60+
return gap
61+
}
62+
63+
// Regression test for the "items jump while scrolling up" bug.
64+
//
65+
// When scrolling backward into never-measured items, their estimate→actual
66+
// size delta lives above the viewport, so scrollTop MUST be compensated or the
67+
// anchored content visibly jumps. The fix adjusts on first measurement even
68+
// during backward scroll (it only skips compensation for RE-measurements while
69+
// scrolling up). This test anchors on a visible item and asserts it moves down
70+
// by exactly the scroll delta — no extra jump from uncompensated measurement.
71+
test('anchored item stays stable when scrolling up into unmeasured items', async ({
72+
page,
73+
}) => {
74+
await page.goto('/scroll-anchor/?initialOffset=10000')
75+
76+
// Wait for the initial measurement to settle (rows become contiguous).
77+
await expect.poll(() => page.evaluate(maxRowGap)).toBeLessThan(1)
78+
79+
const anchor = await page.evaluate(pickAnchor)
80+
81+
const SCROLL_UP = 200
82+
await page.evaluate((delta) => {
83+
const container = document.querySelector('#scroll-container')
84+
if (!container) throw new Error('Container not found')
85+
container.scrollTop -= delta
86+
}, SCROLL_UP)
87+
88+
// Poll until the anchor settles at its compensated position: it should have
89+
// moved down by exactly the scroll delta. With the fix this converges; any
90+
// extra shift is the uncompensated estimate→actual delta of the items
91+
// measured above it (~40px per item) — the regression — and times out here.
92+
await expect
93+
.poll(async () => {
94+
const top = await page.evaluate(topOfIndex, anchor.index)
95+
return top === null ? Infinity : Math.abs(top - (anchor.top + SCROLL_UP))
96+
})
97+
.toBeLessThan(8)
98+
})

packages/react-virtual/e2e/app/vite.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export default defineConfig({
99
rollupOptions: {
1010
input: {
1111
scroll: path.resolve(__dirname, 'scroll/index.html'),
12+
'scroll-anchor': path.resolve(__dirname, 'scroll-anchor/index.html'),
1213
chat: path.resolve(__dirname, 'chat/index.html'),
1314
'measure-element': path.resolve(
1415
__dirname,

packages/virtual-core/src/index.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,14 +1519,15 @@ export class Virtualizer<
15191519
delta,
15201520
this,
15211521
)
1522-
: // Default: adjust scrollTop only when the resize is an above-
1523-
// viewport item AND we're not actively scrolling backward.
1524-
// Adjusting during backward scroll fights the user's scroll
1525-
// direction and produces the "items jump while scrolling up"
1526-
// jank reported across many issues. Users who want the old
1527-
// behavior can pass shouldAdjustScrollPositionOnItemSizeChange.
1522+
: // Default: adjust when the resize is an above-viewport item.
1523+
// First measurement (!has(key)): always adjust — the item
1524+
// has never been sized, so the estimate→actual delta must
1525+
// be compensated regardless of scroll direction.
1526+
// Re-measurement (has(key)): skip during backward scroll
1527+
// to avoid the "items jump while scrolling up" cascade.
15281528
itemStart < this.getScrollOffset() + this.scrollAdjustments &&
1529-
this.scrollDirection !== 'backward')
1529+
(!this.itemSizeCache.has(key) ||
1530+
this.scrollDirection !== 'backward'))
15301531

15311532
if (this.pendingMin === null || index < this.pendingMin) {
15321533
this.pendingMin = index

packages/virtual-core/tests/index.test.ts

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2066,10 +2066,10 @@ test('non-iOS: adjustment is applied immediately during scroll (no regression)',
20662066
expect(v['_iosDeferredAdjustment']).toBe(0)
20672067
})
20682068

2069-
test('scroll-up jank: backward-scroll skips scroll-position adjustment by default', () => {
2070-
// Default behavior change: when an above-viewport item resizes while the
2071-
// user is scrolling BACKWARD, we no longer write to scrollTop. This avoids
2072-
// the well-known "items jump while scrolling up" jank.
2069+
test('scroll-up jank: backward-scroll skips adjustment on re-measurement by default', () => {
2070+
// Default behavior: when an already-measured above-viewport item resizes
2071+
// AGAIN while the user is scrolling BACKWARD, we no longer write to
2072+
// scrollTop. This avoids the well-known "items jump while scrolling up" jank.
20732073
const scrollToFn = vi.fn()
20742074
let scrollCb: ((o: number, s: boolean) => void) | null = null
20752075
const v = new Virtualizer({
@@ -2094,18 +2094,60 @@ test('scroll-up jank: backward-scroll skips scroll-position adjustment by defaul
20942094
})
20952095
v._willUpdate()
20962096
v['getMeasurements']()
2097+
// Seed item 0's size so the backward resize below is a RE-measurement.
2098+
v.resizeItem(0, 80)
20972099
// Now simulate backward scroll: from 200 to 100 (offset decreases).
20982100
scrollCb!(100, true)
20992101
expect(v.scrollDirection).toBe('backward')
21002102
scrollToFn.mockClear()
21012103

2102-
// Resize an above-viewport item while scrolling backward.
2103-
v.resizeItem(0, 100) // item 0 grows by 50px
2104+
// Re-measure an above-viewport item while scrolling backward.
2105+
v.resizeItem(0, 100) // item 0 grows by 20px
21042106

2105-
// Default behavior: no scroll-position adjustment fires.
2107+
// Default behavior: no scroll-position adjustment fires for re-measurements.
21062108
expect(scrollToFn).not.toHaveBeenCalled()
21072109
})
21082110

2111+
test('scroll-up jank: backward-scroll still applies adjustment on first measurement', () => {
2112+
// First measurement is special: the item was rendered at its estimate and is
2113+
// now reporting its actual size. That estimate→actual delta lives above the
2114+
// viewport and MUST be compensated, or the anchored content jumps when
2115+
// scrolling up into never-measured rows.
2116+
const scrollToFn = vi.fn()
2117+
let scrollCb: ((o: number, s: boolean) => void) | null = null
2118+
const v = new Virtualizer({
2119+
count: 10,
2120+
estimateSize: () => 50,
2121+
getScrollElement: () =>
2122+
({
2123+
scrollTop: 200,
2124+
scrollLeft: 0,
2125+
scrollHeight: 500,
2126+
clientHeight: 200,
2127+
offsetHeight: 200,
2128+
}) as any,
2129+
scrollToFn,
2130+
observeElementRect: () => {},
2131+
observeElementOffset: (_inst, cb) => {
2132+
scrollCb = cb
2133+
cb(200, false)
2134+
return () => {}
2135+
},
2136+
})
2137+
v._willUpdate()
2138+
v['getMeasurements']()
2139+
// Backward scroll: 200 → 100.
2140+
scrollCb!(100, true)
2141+
expect(v.scrollDirection).toBe('backward')
2142+
scrollToFn.mockClear()
2143+
2144+
// First measurement of an above-viewport item while scrolling backward.
2145+
v.resizeItem(0, 100) // never measured before → estimate(50)→actual(100)
2146+
2147+
// Adjustment still fires for the first measurement.
2148+
expect(scrollToFn).toHaveBeenCalled()
2149+
})
2150+
21092151
test('scroll-up jank: forward-scroll still applies adjustment (no regression)', () => {
21102152
const scrollToFn = vi.fn()
21112153
let scrollCb: ((o: number, s: boolean) => void) | null = null

0 commit comments

Comments
 (0)