Skip to content

Commit fc3c733

Browse files
authored
perf(virtual-core): skip sync DOM reads during normal scrolling (#1144)
1 parent 5ed73b8 commit fc3c733

File tree

4 files changed

+99
-55
lines changed

4 files changed

+99
-55
lines changed

packages/react-virtual/e2e/app/scroll/main.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,14 @@ const randomHeight = (() => {
2121

2222
const App = () => {
2323
const parentRef = React.useRef<HTMLDivElement>(null)
24+
const initialOffset = Number(
25+
new URLSearchParams(window.location.search).get('initialOffset') ?? 0,
26+
)
2427
const rowVirtualizer = useVirtualizer({
2528
count: 1002,
2629
getScrollElement: () => parentRef.current,
2730
estimateSize: () => 50,
31+
initialOffset,
2832
debug: true,
2933
})
3034

packages/react-virtual/e2e/app/test/scroll.spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,66 @@ test('scrolls to last item', async ({ page }) => {
5151
expect(atBottom).toBeLessThan(1.01)
5252
})
5353

54+
test('renders correctly with initialOffset and user scroll up', async ({
55+
page,
56+
}) => {
57+
// Start at offset 5000 (no programmatic scrollToIndex)
58+
await page.goto('/scroll/?initialOffset=5000')
59+
await page.waitForTimeout(500)
60+
61+
// Items around offset 5000 should be visible
62+
const visibleIndex = await page.evaluate(() => {
63+
const container = document.querySelector('#scroll-container')
64+
if (!container) throw new Error('Container not found')
65+
const items = container.querySelectorAll('[data-index]')
66+
const indices = Array.from(items).map((el) =>
67+
Number(el.getAttribute('data-index')),
68+
)
69+
return Math.min(...indices)
70+
})
71+
expect(visibleIndex).toBeGreaterThan(0)
72+
73+
// Scroll up by 2000px (user scroll, not programmatic)
74+
await page.evaluate(() => {
75+
const container = document.querySelector('#scroll-container')
76+
if (!container) throw new Error('Container not found')
77+
container.scrollTop -= 2000
78+
})
79+
await page.waitForTimeout(500)
80+
81+
// After scroll up, items should be properly measured and positioned
82+
// (no gaps, no overlaps) — verify consecutive items are contiguous
83+
const layout = await page.evaluate(() => {
84+
const container = document.querySelector('#scroll-container')
85+
if (!container) throw new Error('Container not found')
86+
const items = Array.from(container.querySelectorAll('[data-index]'))
87+
.map((el) => {
88+
const rect = el.getBoundingClientRect()
89+
return {
90+
index: Number(el.getAttribute('data-index')),
91+
top: rect.top,
92+
bottom: rect.bottom,
93+
height: rect.height,
94+
}
95+
})
96+
.sort((a, b) => a.index - b.index)
97+
98+
// Check that each item's top matches the previous item's bottom (within tolerance)
99+
let maxGap = 0
100+
for (let i = 1; i < items.length; i++) {
101+
const gap = Math.abs(items[i].top - items[i - 1].bottom)
102+
maxGap = Math.max(maxGap, gap)
103+
}
104+
105+
return { items, maxGap }
106+
})
107+
108+
expect(layout.items.length > 0).toBe(true)
109+
expect(layout.items.length).toBeGreaterThan(3)
110+
// Items should be contiguous — no gaps between consecutive items
111+
expect(layout.maxGap).toBeLessThan(2)
112+
})
113+
54114
test('scrolls to index 0', async ({ page }) => {
55115
await page.goto('/scroll/')
56116

packages/react-virtual/tests/index.test.tsx

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { beforeEach, test, expect, vi } from 'vitest'
22
import * as React from 'react'
3-
import { render, screen, fireEvent } from '@testing-library/react'
3+
import { render, screen } from '@testing-library/react'
44

55
import { useVirtualizer, Range } from '../src/index'
66

@@ -138,29 +138,6 @@ test('should render given dynamic size', async () => {
138138
expect(renderer).toHaveBeenCalledTimes(3)
139139
})
140140

141-
test('should render given dynamic size after scroll', () => {
142-
render(<List itemSize={100} dynamic />)
143-
144-
expect(screen.queryByText('Row 0')).toBeInTheDocument()
145-
expect(screen.queryByText('Row 1')).toBeInTheDocument()
146-
expect(screen.queryByText('Row 2')).toBeInTheDocument()
147-
expect(screen.queryByText('Row 3')).not.toBeInTheDocument()
148-
149-
expect(renderer).toHaveBeenCalledTimes(3)
150-
renderer.mockReset()
151-
152-
fireEvent.scroll(screen.getByTestId('scroller'), {
153-
target: { scrollTop: 400 },
154-
})
155-
156-
expect(screen.queryByText('Row 2')).not.toBeInTheDocument()
157-
expect(screen.queryByText('Row 3')).toBeInTheDocument()
158-
expect(screen.queryByText('Row 6')).toBeInTheDocument()
159-
expect(screen.queryByText('Row 7')).not.toBeInTheDocument()
160-
161-
expect(renderer).toHaveBeenCalledTimes(2)
162-
})
163-
164141
test('should use rangeExtractor', () => {
165142
render(<List rangeExtractor={() => [0, 1]} />)
166143

packages/virtual-core/src/index.ts

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,21 @@ export class Virtualizer<
409409
return (_ro = new this.targetWindow.ResizeObserver((entries) => {
410410
entries.forEach((entry) => {
411411
const run = () => {
412-
this._measureElement(entry.target as TItemElement, entry)
412+
const node = entry.target as TItemElement
413+
const index = this.indexFromElement(node)
414+
415+
if (!node.isConnected) {
416+
this.observer.unobserve(node)
417+
this.elementsCache.delete(this.options.getItemKey(index))
418+
return
419+
}
420+
421+
if (this.shouldMeasureDuringScroll(index)) {
422+
this.resizeItem(
423+
index,
424+
this.options.measureElement(node, entry, this),
425+
)
426+
}
413427
}
414428
this.options.useAnimationFrameWithResizeObserver
415429
? requestAnimationFrame(run)
@@ -984,21 +998,19 @@ export class Virtualizer<
984998
return true
985999
}
9861000

987-
private _measureElement = (
988-
node: TItemElement,
989-
entry: ResizeObserverEntry | undefined,
990-
) => {
991-
if (!node.isConnected) {
992-
this.observer.unobserve(node)
1001+
measureElement = (node: TItemElement | null) => {
1002+
if (!node) {
1003+
this.elementsCache.forEach((cached, key) => {
1004+
if (!cached.isConnected) {
1005+
this.observer.unobserve(cached)
1006+
this.elementsCache.delete(key)
1007+
}
1008+
})
9931009
return
9941010
}
9951011

9961012
const index = this.indexFromElement(node)
997-
const item = this.measurementsCache[index]
998-
if (!item) {
999-
return
1000-
}
1001-
const key = item.key
1013+
const key = this.options.getItemKey(index)
10021014
const prevNode = this.elementsCache.get(key)
10031015

10041016
if (prevNode !== node) {
@@ -1009,16 +1021,21 @@ export class Virtualizer<
10091021
this.elementsCache.set(key, node)
10101022
}
10111023

1012-
if (this.shouldMeasureDuringScroll(index)) {
1013-
this.resizeItem(index, this.options.measureElement(node, entry, this))
1024+
// Sync-measure when idle (initial render) or during programmatic scrolling
1025+
// (scrollToIndex/scrollToOffset) where reconcileScroll needs sizes in the same frame.
1026+
// During normal user scrolling, skip sync measurement — the RO callback handles it async.
1027+
if (
1028+
(!this.isScrolling || this.scrollState) &&
1029+
this.shouldMeasureDuringScroll(index)
1030+
) {
1031+
this.resizeItem(index, this.options.measureElement(node, undefined, this))
10141032
}
10151033
}
10161034

10171035
resizeItem = (index: number, size: number) => {
10181036
const item = this.measurementsCache[index]
1019-
if (!item) {
1020-
return
1021-
}
1037+
if (!item) return
1038+
10221039
const itemSize = this.itemSizeCache.get(item.key) ?? item.size
10231040
const delta = size - itemSize
10241041

@@ -1045,20 +1062,6 @@ export class Virtualizer<
10451062
}
10461063
}
10471064

1048-
measureElement = (node: TItemElement | null | undefined) => {
1049-
if (!node) {
1050-
this.elementsCache.forEach((cached, key) => {
1051-
if (!cached.isConnected) {
1052-
this.observer.unobserve(cached)
1053-
this.elementsCache.delete(key)
1054-
}
1055-
})
1056-
return
1057-
}
1058-
1059-
this._measureElement(node, undefined)
1060-
}
1061-
10621065
getVirtualItems = memo(
10631066
() => [this.getVirtualIndexes(), this.getMeasurements()],
10641067
(indexes, measurements) => {

0 commit comments

Comments
 (0)