feat(core): implement built-in scroll-scaling for large lists#1172
feat(core): implement built-in scroll-scaling for large lists#1172DivyamUp14 wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements scroll-range scaling in TanStack Virtual's Virtualizer to handle virtual lists exceeding browser CSS height limits (~33.5 million pixels). It introduces a ChangesScroll-Range Scaling Feature
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/virtual-core/tests/index.test.tsParsing error: "parserOptions.project" has been provided for Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/virtual-core/tests/index.test.ts (1)
2660-2677: ⚡ Quick winThe
scale === 1zero-overhead test doesn’t verify object identity yet.This test currently checks size/scale only, so it won’t fail if
getVirtualItems()starts cloning underscale === 1. Assert reference equality between returned items andgetMeasurements()entries for visible indexes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/virtual-core/tests/index.test.ts` around lines 2660 - 2677, The test currently only checks scale and total size but doesn't assert object identity; update it to ensure getVirtualItems returns actual items (provide a non-null scroll element in the Virtualizer config, e.g., a dummy element) when scale === 1, call virtualizer.getMeasurements() and virtualizer.getVirtualItems(), then for each virtual item assert strict reference equality with the corresponding measurements entry (match by item.index) so the test fails if getVirtualItems clones objects under scale === 1; keep existing getTotalSize and scale assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/virtual-core/src/index.ts`:
- Around line 386-390: The getScale function can divide by invalid
options.maxScrollSize (0, negative, NaN, Infinity) causing invalid scale values;
update getScale to validate maxScrollSize before dividing (use Number.isFinite
and maxScrollSize > 0) and treat any invalid value as a safe fallback (e.g., 1
or a small positive clamp), keeping the existing behavior of returning
virtualTotal / max when valid; change references in getScale (and callers like
getTotalSize/offset conversion/anchoring logic) to rely on this validated result
so downstream math never sees Infinity/NaN/negative scale.
---
Nitpick comments:
In `@packages/virtual-core/tests/index.test.ts`:
- Around line 2660-2677: The test currently only checks scale and total size but
doesn't assert object identity; update it to ensure getVirtualItems returns
actual items (provide a non-null scroll element in the Virtualizer config, e.g.,
a dummy element) when scale === 1, call virtualizer.getMeasurements() and
virtualizer.getVirtualItems(), then for each virtual item assert strict
reference equality with the corresponding measurements entry (match by
item.index) so the test fails if getVirtualItems clones objects under scale ===
1; keep existing getTotalSize and scale assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a8f8b85-a408-461b-b026-09188d95fa4e
📒 Files selected for processing (4)
.changeset/scroll-scaling.mddocs/api/virtualizer.mdpackages/virtual-core/src/index.tspackages/virtual-core/tests/index.test.ts
🎯 Changes
Fixes #616
Introduced native scroll-scaling in
virtual-coreto compress coordinates and bypass the browser's maximum scroll height limitation (~33.5 million pixels) for extremely large lists.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests