Skip to content

Commit ff83e94

Browse files
piecykpiecyk
authored
fix(virtual-core): early return in _measureElement for disconnected nodes (#1135)
1 parent e0e4dcd commit ff83e94

File tree

10 files changed

+191
-6
lines changed

10 files changed

+191
-6
lines changed

‎.changeset/ninety-games-accept.md‎

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(virtual-core): early return in \_measureElement for disconnected nodes

packages/react-virtual/e2e/app/index.html renamed to packages/react-virtual/e2e/app/measure-element/index.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
</head>
66
<body>
77
<div id="root"></div>
8-
<script type="module" src="/main.tsx"></script>
8+
<script type="module" src="./main.tsx"></script>
99
</body>
1010
</html>

‎packages/react-virtual/e2e/app/measure-element/main.tsx‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import React from 'react'
2+
import ReactDOM from 'react-dom/client'
3+
import { useVirtualizer } from '@tanstack/react-virtual'
4+
5+
interface Item {
6+
id: string
7+
label: string
8+
}
9+
10+
const INITIAL_ITEMS: Array<Item> = [
11+
{ id: 'item-a', label: 'A' },
12+
{ id: 'item-b', label: 'B' },
13+
{ id: 'item-c', label: 'C' },
14+
]
15+
16+
const App = () => {
17+
const parentRef = React.useRef<HTMLDivElement>(null)
18+
const [items, setItems] = React.useState(INITIAL_ITEMS)
19+
const [expandedId, setExpandedId] = React.useState<string | null>(null)
20+
21+
const rowVirtualizer = useVirtualizer({
22+
count: items.length,
23+
getScrollElement: () => parentRef.current,
24+
estimateSize: () => 36,
25+
getItemKey: (index) => items[index].id,
26+
})
27+
28+
const toggleExpand = (id: string) => {
29+
setExpandedId((prev) => (prev === id ? null : id))
30+
}
31+
32+
const deleteItem = (id: string) => {
33+
setItems((prev) => prev.filter((item) => item.id !== id))
34+
if (expandedId === id) {
35+
setExpandedId(null)
36+
}
37+
}
38+
39+
return (
40+
<div>
41+
<div
42+
ref={parentRef}
43+
id="scroll-container"
44+
style={{ height: 400, overflow: 'auto' }}
45+
>
46+
<div
47+
style={{
48+
height: rowVirtualizer.getTotalSize(),
49+
position: 'relative',
50+
}}
51+
>
52+
{rowVirtualizer.getVirtualItems().map((v) => {
53+
const item = items[v.index]
54+
const isExpanded = expandedId === item.id
55+
56+
return (
57+
<div
58+
key={item.id}
59+
data-testid={item.id}
60+
ref={rowVirtualizer.measureElement}
61+
data-index={v.index}
62+
style={{
63+
position: 'absolute',
64+
top: 0,
65+
left: 0,
66+
transform: `translateY(${v.start}px)`,
67+
width: '100%',
68+
}}
69+
>
70+
<div
71+
style={{
72+
display: 'flex',
73+
gap: 8,
74+
alignItems: 'center',
75+
padding: 4,
76+
}}
77+
>
78+
<span>Row {item.label}</span>
79+
<button
80+
data-testid={`expand-${item.id}`}
81+
onClick={() => toggleExpand(item.id)}
82+
>
83+
{isExpanded ? 'Collapse' : 'Expand'}
84+
</button>
85+
<button
86+
data-testid={`delete-${item.id}`}
87+
onClick={() => deleteItem(item.id)}
88+
>
89+
Delete
90+
</button>
91+
</div>
92+
{isExpanded && (
93+
<div
94+
data-testid={`content-${item.id}`}
95+
style={{
96+
height: 124,
97+
background: '#eee',
98+
padding: 8,
99+
}}
100+
>
101+
Expanded content for {item.label}
102+
</div>
103+
)}
104+
</div>
105+
)
106+
})}
107+
</div>
108+
</div>
109+
<div data-testid="total-size">{rowVirtualizer.getTotalSize()}</div>
110+
</div>
111+
)
112+
}
113+
114+
ReactDOM.createRoot(document.getElementById('root')!).render(<App />)

‎packages/react-virtual/e2e/app/scroll/index.html‎

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>

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

File renamed without changes.

‎packages/react-virtual/e2e/app/test/measure-element.spec.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { expect, test } from '@playwright/test'
2+
3+
test('positions items correctly after expand → collapse → delete → expand', async ({
4+
page,
5+
}) => {
6+
await page.goto('/measure-element/')
7+
8+
// All 3 items visible at ~36px each
9+
await expect(page.locator('[data-testid="item-a"]')).toBeVisible()
10+
await expect(page.locator('[data-testid="item-b"]')).toBeVisible()
11+
await expect(page.locator('[data-testid="item-c"]')).toBeVisible()
12+
13+
// Step 1: Expand A → should grow to ~160px
14+
await page.click('[data-testid="expand-item-a"]')
15+
await expect(page.locator('[data-testid="content-item-a"]')).toBeVisible()
16+
17+
// Step 2: Collapse A → back to ~36px
18+
await page.click('[data-testid="expand-item-a"]')
19+
await expect(page.locator('[data-testid="content-item-a"]')).not.toBeVisible()
20+
21+
// Step 3: Delete A
22+
await page.click('[data-testid="delete-item-a"]')
23+
await expect(page.locator('[data-testid="item-a"]')).not.toBeVisible()
24+
25+
// Step 4: Expand B → should grow to ~160px
26+
await page.click('[data-testid="expand-item-b"]')
27+
await expect(page.locator('[data-testid="content-item-b"]')).toBeVisible()
28+
29+
// Wait for ResizeObserver to measure the expanded B
30+
await page.waitForTimeout(200)
31+
32+
// C should be positioned after the expanded B, not overlapping it
33+
const bBox = await page.locator('[data-testid="item-b"]').boundingBox()
34+
const cBox = await page.locator('[data-testid="item-c"]').boundingBox()
35+
36+
expect(bBox).not.toBeNull()
37+
expect(cBox).not.toBeNull()
38+
39+
// C's top should be at or after B's bottom (with no overlap)
40+
const bBottom = bBox!.y + bBox!.height
41+
expect(cBox!.y).toBeGreaterThanOrEqual(bBottom - 1) // 1px tolerance
42+
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ const check = () => {
1919
}
2020

2121
test('scrolls to index 1000', async ({ page }) => {
22-
await page.goto('/')
22+
await page.goto('/scroll/')
2323
await page.click('#scroll-to-1000')
2424

2525
// Wait for scroll effect (including retries)

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,17 @@ import react from '@vitejs/plugin-react'
55
export default defineConfig({
66
root: __dirname,
77
plugins: [react()],
8+
build: {
9+
rollupOptions: {
10+
input: {
11+
scroll: path.resolve(__dirname, 'scroll/index.html'),
12+
'measure-element': path.resolve(
13+
__dirname,
14+
'measure-element/index.html',
15+
),
16+
},
17+
},
18+
},
819
resolve: {
920
alias: {
1021
'@tanstack/react-virtual': path.resolve(__dirname, '../../src/index'),

‎packages/react-virtual/playwright.config.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export default defineConfig({
1010
},
1111
webServer: {
1212
command: `VITE_SERVER_PORT=${PORT} vite build --config e2e/app/vite.config.ts && VITE_SERVER_PORT=${PORT} vite preview --config e2e/app/vite.config.ts --port ${PORT}`,
13-
url: baseURL,
13+
url: `${baseURL}/scroll/`,
1414
reuseExistingServer: !process.env.CI,
1515
stdout: 'pipe',
1616
},

‎packages/virtual-core/src/index.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,11 @@ export class Virtualizer<
863863
node: TItemElement,
864864
entry: ResizeObserverEntry | undefined,
865865
) => {
866+
if (!node.isConnected) {
867+
this.observer.unobserve(node)
868+
return
869+
}
870+
866871
const index = this.indexFromElement(node)
867872
const item = this.measurementsCache[index]
868873
if (!item) {
@@ -879,9 +884,7 @@ export class Virtualizer<
879884
this.elementsCache.set(key, node)
880885
}
881886

882-
if (node.isConnected) {
883-
this.resizeItem(index, this.options.measureElement(node, entry, this))
884-
}
887+
this.resizeItem(index, this.options.measureElement(node, entry, this))
885888
}
886889

887890
resizeItem = (index: number, size: number) => {

0 commit comments

Comments
 (0)