diff --git a/.changeset/quick-rings-happen.md b/.changeset/quick-rings-happen.md new file mode 100644 index 00000000..7a2ec466 --- /dev/null +++ b/.changeset/quick-rings-happen.md @@ -0,0 +1,5 @@ +--- +'@tanstack/virtual-core': patch +--- + +fix(virtual-core): remove incorrect elementsCache cleanup using getItemKey diff --git a/packages/react-virtual/e2e/app/stale-index/index.html b/packages/react-virtual/e2e/app/stale-index/index.html new file mode 100644 index 00000000..56f418f6 --- /dev/null +++ b/packages/react-virtual/e2e/app/stale-index/index.html @@ -0,0 +1,10 @@ + + + + + + +
+ + + diff --git a/packages/react-virtual/e2e/app/stale-index/main.tsx b/packages/react-virtual/e2e/app/stale-index/main.tsx new file mode 100644 index 00000000..13fe85e1 --- /dev/null +++ b/packages/react-virtual/e2e/app/stale-index/main.tsx @@ -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 { + return Array.from({ length: count }, (_, i) => ({ + id: `item-${i}`, + label: `Row ${i}`, + })) +} + +const App = () => { + const parentRef = React.useRef(null) + const [items, setItems] = React.useState(() => makeItems(20)) + const [error, setError] = React.useState(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 ( +
+ +
Count: {items.length}
+ {error &&
{error}
} +
+
+ {rowVirtualizer.getVirtualItems().map((v) => { + const item = items[v.index] + return ( +
+ {item.label} +
+ ) + })} +
+
+
+ ) +} + +ReactDOM.createRoot(document.getElementById('root')!).render() diff --git a/packages/react-virtual/e2e/app/test/stale-index.spec.ts b/packages/react-virtual/e2e/app/test/stale-index.spec.ts new file mode 100644 index 00000000..fcf7d93e --- /dev/null +++ b/packages/react-virtual/e2e/app/test/stale-index.spec.ts @@ -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() +}) diff --git a/packages/react-virtual/e2e/app/vite.config.ts b/packages/react-virtual/e2e/app/vite.config.ts index 005ecd9c..64183e49 100644 --- a/packages/react-virtual/e2e/app/vite.config.ts +++ b/packages/react-virtual/e2e/app/vite.config.ts @@ -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'), }, }, }, diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index 8cd53a81..64e9bd6d 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -414,7 +414,6 @@ export class Virtualizer< if (!node.isConnected) { this.observer.unobserve(node) - this.elementsCache.delete(this.options.getItemKey(index)) return }