Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quick-rings-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/virtual-core': patch
---

fix(virtual-core): remove incorrect elementsCache cleanup using getItemKey
10 changes: 10 additions & 0 deletions packages/react-virtual/e2e/app/stale-index/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
</head>
<body>
<div id="root"></div>
<script type="module" src="./main.tsx"></script>
</body>
</html>
95 changes: 95 additions & 0 deletions packages/react-virtual/e2e/app/stale-index/main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import React from 'react'
import ReactDOM from 'react-dom/client'
import { useVirtualizer } from '@tanstack/react-virtual'

/**
* Regression test app for stale index bug:
* When items are removed from the end of the list, the ResizeObserver may fire
* for a disconnected node whose data-index >= the new count. If getItemKey
* indexes into the items array, this causes an out-of-bounds error.
*/

interface Item {
id: string
label: string
}

function makeItems(count: number): Array<Item> {
return Array.from({ length: count }, (_, i) => ({
id: `item-${i}`,
label: `Row ${i}`,
}))
}

const App = () => {
const parentRef = React.useRef<HTMLDivElement>(null)
const [items, setItems] = React.useState(() => makeItems(20))
const [error, setError] = React.useState<string | null>(null)

const rowVirtualizer = useVirtualizer({
count: items.length,
getScrollElement: () => parentRef.current,
estimateSize: () => 50,
getItemKey: (index) => {
if (index < 0 || index >= items.length) {
const msg = `getItemKey called with stale index ${index} (count=${items.length})`
setError(msg)
throw new Error(msg)
}
return items[index].id
},
})

const removeLastFive = () => {
setItems((prev) => prev.slice(0, Math.max(0, prev.length - 5)))
}

return (
<div>
<button data-testid="remove-items" onClick={removeLastFive}>
Remove last 5
</button>
<div data-testid="item-count">Count: {items.length}</div>
{error && <div data-testid="error">{error}</div>}
<div
ref={parentRef}
data-testid="scroll-container"
style={{ height: 300, overflow: 'auto' }}
>
<div
style={{
height: rowVirtualizer.getTotalSize(),
position: 'relative',
}}
>
{rowVirtualizer.getVirtualItems().map((v) => {
const item = items[v.index]
return (
<div
key={item.id}
data-testid={item.id}
ref={rowVirtualizer.measureElement}
data-index={v.index}
style={{
position: 'absolute',
top: 0,
left: 0,
transform: `translateY(${v.start}px)`,
width: '100%',
height: 50,
borderBottom: '1px solid #ccc',
padding: 8,
boxSizing: 'border-box',
}}
>
{item.label}
</div>
)
})}
</div>
</div>
</div>
)
}

ReactDOM.createRoot(document.getElementById('root')!).render(<App />)
38 changes: 38 additions & 0 deletions packages/react-virtual/e2e/app/test/stale-index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { expect, test } from '@playwright/test'

test('does not call getItemKey with stale index after removing items', async ({
page,
}) => {
await page.goto('/stale-index/')

// Verify initial state
await expect(page.locator('[data-testid="item-count"]')).toHaveText(
'Count: 20',
)

// Scroll to the bottom so the last items are rendered and observed by ResizeObserver
const container = page.locator('[data-testid="scroll-container"]')
await container.evaluate((el) => (el.scrollTop = el.scrollHeight))
await page.waitForTimeout(100)

// Remove 5 items from the end — the RO may still fire for the now-disconnected nodes
await page.click('[data-testid="remove-items"]')
await expect(page.locator('[data-testid="item-count"]')).toHaveText(
'Count: 15',
)

// Wait for any pending ResizeObserver callbacks
await page.waitForTimeout(200)

// No error should have been thrown
await expect(page.locator('[data-testid="error"]')).not.toBeVisible()

// Remove 5 more to stress it
await page.click('[data-testid="remove-items"]')
await expect(page.locator('[data-testid="item-count"]')).toHaveText(
'Count: 10',
)
await page.waitForTimeout(200)

await expect(page.locator('[data-testid="error"]')).not.toBeVisible()
})
1 change: 1 addition & 0 deletions packages/react-virtual/e2e/app/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default defineConfig({
'measure-element/index.html',
),
'smooth-scroll': path.resolve(__dirname, 'smooth-scroll/index.html'),
'stale-index': path.resolve(__dirname, 'stale-index/index.html'),
},
},
},
Expand Down
1 change: 0 additions & 1 deletion packages/virtual-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ export class Virtualizer<

if (!node.isConnected) {
this.observer.unobserve(node)
this.elementsCache.delete(this.options.getItemKey(index))
return
}

Expand Down
Loading