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/chilled-falcons-battle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/virtual-core': patch
---

fix(virtual-core): scroll to index doesn't scroll to bottom correctly
2 changes: 2 additions & 0 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ jobs:
uses: nrwl/nx-set-shas@v4.3.0
with:
main-branch-name: main
- name: Install Playwright browsers
run: pnpm exec playwright install chromium
- name: Run Checks
run: pnpm run test:pr
preview:
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,8 @@ stats.html

vite.config.js.timestamp-*
vite.config.ts.timestamp-*

# Playwright test artifacts
test-results/
playwright-report/
*.log
2 changes: 1 addition & 1 deletion knip.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"$schema": "https://unpkg.com/knip@5/schema.json",
"ignoreWorkspaces": ["examples/**"]
"ignoreWorkspaces": ["examples/**", "packages/react-virtual/e2e/**"]
}
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@
"clean": "pnpm --filter \"./packages/**\" run clean",
"preinstall": "node -e \"if(process.env.CI == 'true') {console.log('Skipping preinstall...'); process.exit(1)}\" || npx -y only-allow pnpm",
"test": "pnpm run test:ci",
"test:pr": "nx affected --targets=test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build",
"test:ci": "nx run-many --targets=test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build",
"test:pr": "nx affected --targets=test:sherif,test:knip,test:eslint,test:lib,test:e2e,test:types,test:build,build",
"test:ci": "nx run-many --targets=test:sherif,test:knip,test:eslint,test:lib,test:e2e,test:types,test:build,build",
"test:eslint": "nx affected --target=test:eslint",
"test:format": "pnpm run prettier --check",
"test:sherif": "sherif",
"test:lib": "nx affected --target=test:lib --exclude=examples/**",
"test:lib:dev": "pnpm run test:lib && nx watch --all -- pnpm run test:lib",
"test:build": "nx affected --target=test:build --exclude=examples/**",
"test:types": "nx affected --target=test:types --exclude=examples/**",
"test:e2e": "nx affected --target=test:e2e --exclude=examples/**",
"test:knip": "knip",
"build": "nx affected --target=build --exclude=examples/**",
"build:all": "nx run-many --target=build --exclude=examples/**",
Expand All @@ -39,6 +40,7 @@
},
"devDependencies": {
"@changesets/cli": "^2.29.4",
"@playwright/test": "^1.53.1",
"@svitejs/changesets-changelog-github-compact": "^1.2.0",
"@tanstack/config": "^0.18.2",
"@testing-library/jest-dom": "^6.6.3",
Expand Down
10 changes: 10 additions & 0 deletions packages/react-virtual/e2e/app/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>
76 changes: 76 additions & 0 deletions packages/react-virtual/e2e/app/main.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import React from 'react'
import ReactDOM from 'react-dom/client'
import { useVirtualizer } from '../../src/index'

function getRandomInt(min: number, max: number) {
return Math.floor(Math.random() * (max - min + 1)) + min
}

const randomHeight = (() => {
const cache = new Map()
return (id: string) => {
const value = cache.get(id)
if (value !== undefined) {
return value
}
const v = getRandomInt(25, 100)
cache.set(id, v)
return v
}
})()

const App = () => {
const parentRef = React.useRef<HTMLDivElement>(null)
const rowVirtualizer = useVirtualizer({
count: 1002,
getScrollElement: () => parentRef.current,
estimateSize: () => 50,
debug: true,
})

return (
<div>
<button
id="scroll-to-1000"
onClick={() => rowVirtualizer.scrollToIndex(1000)}
>
Scroll to 1000
</button>

<div
ref={parentRef}
id="scroll-container"
style={{ height: 400, overflow: 'auto' }}
>
<div
style={{
height: rowVirtualizer.getTotalSize(),
position: 'relative',
}}
>
{rowVirtualizer.getVirtualItems().map((v) => (
<div
key={v.key}
data-testid={`item-${v.index}`}
ref={rowVirtualizer.measureElement}
data-index={v.index}
style={{
position: 'absolute',
top: 0,
left: 0,
transform: `translateY(${v.start}px)`,
width: '100%',
}}
>
<div style={{ height: randomHeight(String(v.key)) }}>
Row {v.index}
</div>
</div>
))}
</div>
</div>
</div>
)
}

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

const check = () => {
const item = document.querySelector('[data-testid="item-1000"]')
const container = document.querySelector('#scroll-container')

if (!item || !container) throw new Error('Elements not found')

const itemRect = item.getBoundingClientRect()
const containerRect = container.getBoundingClientRect()
const scrollTop = container.scrollTop

const top = itemRect.top + scrollTop - containerRect.top
const botttom = top + itemRect.height

const containerBottom = scrollTop + container.clientHeight

return Math.abs(botttom - containerBottom)
}

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

// Wait for scroll effect (including retries)
await page.waitForTimeout(1000)

await expect(page.locator('[data-testid="item-1000"]')).toBeVisible()

const delta = await page.evaluate(check)
console.log('bootom element detla', delta)
expect(delta).toBeLessThan(1.01)
})
14 changes: 14 additions & 0 deletions packages/react-virtual/e2e/app/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"compilerOptions": {
"strict": true,
"esModuleInterop": true,
"jsx": "react-jsx",
"target": "ESNext",
"moduleResolution": "Bundler",
"module": "ESNext",
"resolveJsonModule": true,
"allowJs": true,
"skipLibCheck": true
},
"exclude": ["node_modules", "dist"]
}
7 changes: 7 additions & 0 deletions packages/react-virtual/e2e/app/vite.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'

export default defineConfig({
root: __dirname,
plugins: [react()],
})
3 changes: 2 additions & 1 deletion packages/react-virtual/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"test:lib": "vitest",
"test:lib:dev": "pnpm run test:lib --watch",
"test:build": "publint --strict",
"build": "vite build"
"build": "vite build",
"test:e2e": "playwright test"
},
"type": "module",
"types": "dist/esm/index.d.ts",
Expand Down
17 changes: 17 additions & 0 deletions packages/react-virtual/playwright.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { defineConfig } from '@playwright/test'

const PORT = 5173
const baseURL = `http://localhost:${PORT}`

export default defineConfig({
testDir: './e2e/app/test',
use: {
baseURL,
},
webServer: {
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}`,
url: baseURL,
reuseExistingServer: !process.env.CI,
stdout: 'pipe',
},
})
7 changes: 6 additions & 1 deletion packages/react-virtual/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,10 @@
"compilerOptions": {
"jsx": "react"
},
"include": ["src", "eslint.config.js", "vite.config.ts"]
"include": [
"src",
"eslint.config.js",
"vite.config.ts",
"playwright.config.ts"
]
}
78 changes: 40 additions & 38 deletions packages/virtual-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,6 @@ export class Virtualizer<
scrollElement: TScrollElement | null = null
targetWindow: (Window & typeof globalThis) | null = null
isScrolling = false
private scrollToIndexTimeoutId: number | null = null
measurementsCache: Array<VirtualItem> = []
private itemSizeCache = new Map<Key, number>()
private pendingMeasuredCacheIndexes: Array<number> = []
Expand Down Expand Up @@ -904,7 +903,7 @@ export class Virtualizer<
toOffset -= size
}

const maxOffset = this.getTotalSize() - size
const maxOffset = this.getTotalSize() + this.options.scrollMargin - size

return Math.max(Math.min(maxOffset, toOffset), 0)
}
Expand Down Expand Up @@ -943,19 +942,10 @@ export class Virtualizer<

private isDynamicMode = () => this.elementsCache.size > 0

private cancelScrollToIndex = () => {
if (this.scrollToIndexTimeoutId !== null && this.targetWindow) {
this.targetWindow.clearTimeout(this.scrollToIndexTimeoutId)
this.scrollToIndexTimeoutId = null
}
}

scrollToOffset = (
toOffset: number,
{ align = 'start', behavior }: ScrollToOffsetOptions = {},
) => {
this.cancelScrollToIndex()

if (behavior === 'smooth' && this.isDynamicMode()) {
console.warn(
'The `smooth` scroll behavior is not fully supported with dynamic size.',
Expand All @@ -972,50 +962,62 @@ export class Virtualizer<
index: number,
{ align: initialAlign = 'auto', behavior }: ScrollToIndexOptions = {},
) => {
index = Math.max(0, Math.min(index, this.options.count - 1))

this.cancelScrollToIndex()

if (behavior === 'smooth' && this.isDynamicMode()) {
console.warn(
'The `smooth` scroll behavior is not fully supported with dynamic size.',
)
}

const offsetAndAlign = this.getOffsetForIndex(index, initialAlign)
if (!offsetAndAlign) return
index = Math.max(0, Math.min(index, this.options.count - 1))

const [offset, align] = offsetAndAlign
let attempts = 0
const maxAttempts = 10

this._scrollToOffset(offset, { adjustments: undefined, behavior })
const tryScroll = (currentAlign: ScrollAlignment) => {
if (!this.targetWindow) return

if (behavior !== 'smooth' && this.isDynamicMode() && this.targetWindow) {
this.scrollToIndexTimeoutId = this.targetWindow.setTimeout(() => {
this.scrollToIndexTimeoutId = null
const offsetInfo = this.getOffsetForIndex(index, currentAlign)
if (!offsetInfo) {
console.warn('Failed to get offset for index:', index)
return
}
const [offset, align] = offsetInfo
this._scrollToOffset(offset, { adjustments: undefined, behavior })

this.targetWindow.requestAnimationFrame(() => {
const currentOffset = this.getScrollOffset()
const afterInfo = this.getOffsetForIndex(index, align)
if (!afterInfo) {
console.warn('Failed to get offset for index:', index)
return
}

const elementInDOM = this.elementsCache.has(
this.options.getItemKey(index),
)
if (!approxEqual(afterInfo[0], currentOffset)) {
scheduleRetry(align)
}
})
}

if (elementInDOM) {
const result = this.getOffsetForIndex(index, align)
if (!result) return
const [latestOffset] = result
const scheduleRetry = (align: ScrollAlignment) => {
if (!this.targetWindow) return

const currentScrollOffset = this.getScrollOffset()
if (!approxEqual(latestOffset, currentScrollOffset)) {
this.scrollToIndex(index, { align, behavior })
}
} else {
this.scrollToIndex(index, { align, behavior })
attempts++
if (attempts < maxAttempts) {
if (process.env.NODE_ENV !== 'production' && this.options.debug) {
console.info('Schedule retry', attempts, maxAttempts)
}
})
this.targetWindow.requestAnimationFrame(() => tryScroll(align))
} else {
console.warn(
`Failed to scroll to index ${index} after ${maxAttempts} attempts.`,
)
}
}

tryScroll(initialAlign)
}

scrollBy = (delta: number, { behavior }: ScrollToOffsetOptions = {}) => {
this.cancelScrollToIndex()

if (behavior === 'smooth' && this.isDynamicMode()) {
console.warn(
'The `smooth` scroll behavior is not fully supported with dynamic size.',
Expand Down
2 changes: 1 addition & 1 deletion packages/virtual-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export function notUndefined<T>(value: T | undefined, msg?: string): T {
}
}

export const approxEqual = (a: number, b: number) => Math.abs(a - b) <= 1
export const approxEqual = (a: number, b: number) => Math.abs(a - b) < 1.01

export const debounce = (
targetWindow: Window & typeof globalThis,
Expand Down
Loading