perf: virtual-core rewrite for mount/measure-storm, plus iOS Safari handling and scroll restoration#1168
Merged
Merged
Conversation
`resizeItem` was doing `new Map(itemSizeCache.set(...))` on every call, cloning the entire size cache (O(n) per call) just to invalidate the `getMeasurements` memo. For a 10k-item dynamic list mount where every item resizes, this was O(n²) — measured at 1861ms. Replace with mutate-in-place + a private `itemSizeCacheVersion` counter that is included in `getMeasurements`'s memo deps. Same invalidation behavior, O(1) per call. Also switches `measure()` to `.clear()` + bump version rather than allocating fresh Maps. Benchmarks (n×n measure storm, then 1× getMeasurements): n=100 0.159ms -> 0.013ms (12x) n=1000 16.0ms -> 0.107ms (150x) n=5000 399.6ms -> 0.640ms (624x) n=10000 1861ms -> 1.35ms (1382x) No public API change; itemSizeCacheVersion is private. Adds 11 regression tests pinning the cache-invalidation contract.
`setOptions` was using `Object.entries(opts).forEach([k,v] => if undefined delete opts[k])`
to strip undefined values before `{...defaults, ...opts}`. Two problems:
1. The `delete` call triggers V8 hidden-class dictionary-mode transition,
slowing every subsequent options access for the virtualizer's lifetime.
2. It mutates the caller's opts object — a hidden API contract violation.
Replace with a single `for...in` loop that copies non-undefined values
onto a fresh defaults object. Same semantics (undefined falls through to
defaults, falsy 0/false/'' stick), no mutation, no deopt.
Benchmark (10,000 setOptions calls, simulating React render storm):
before: 14.35ms
after: 1.31ms
speedup: 11.0x
Adds 6 regression tests pinning the merge contract (defaults, undefined-falls-through,
falsy values stick, no-mutation, no-stale-accumulation, explicit-override).
…array `getMeasurements` was reading the earliest dirty index with `Math.min(...this.pendingMeasuredCacheIndexes)`. The spread allocates an argument list and, at very large pending counts (~125k), can throw RangeError from V8's stack-argument limit. Replace the Array<number> + Math.min(...) pair with a single `pendingMin: number | null` field. `resizeItem` does an O(1) compare-and-set; `getMeasurements` reads it and resets to null. Perf delta is small (the rebuild loop dominates), but this removes a latent stack-overflow footgun on very large lists. Adds 2 regression tests: - random-order resize produces correct prefix-sums (covers the running-min logic) - 10k-item storm doesn't crash on min lookup
…ost) Added bench scenarios that measure the cost of notify() dispatch under resize-storms with realistic vs no-op onChange callbacks. These informed the decision to *not* implement Layer 4 of the perf audit: - React 18+ batches useReducer dispatches; the audit's "1000 React renders per mount" claim doesn't hold in practice. - Real-world cost of redundant notify() is ~1ms over a 10k-item mount. - Routing through maybeNotify (the audit's proposed fix) would change the sync flag from false to isScrolling, regressing scroll behavior. Keeping the benches for future revisits.
The default extractor was building its result with `arr.push(i)`, forcing V8's array-growth heuristic to repeatedly resize. Compute the length upfront and allocate once. Benchmarks (10,000 invocations): visible=50 1.07ms -> 0.50ms (2.14x) visible=200 3.96ms -> 1.94ms (2.04x) visible=1000 28.81ms -> 12.28ms (2.35x) Adds 7 regression tests for the extractor (basic, overscan, start/end clamping, single-item, large range, return-type).
The narrow defaults object doesn't have the user-required fields (count, estimateSize, etc.) until the loop fills them in. The 'as Required<...>' cast was too strict and failed tsc's structural check. Casting through 'unknown' is the standard escape hatch for two-step build patterns.
…llocating {}
The force-rerender pattern previously used `useReducer(() => ({}), {})` which
allocates a new object on every dispatch. Switch to an incrementing number —
same semantics (state changes on every dispatch, forcing a render), zero alloc.
Trivial individual cost, but eliminates one steady-state GC source on
scroll-heavy apps.
… node When an item element disconnects from the DOM, the ResizeObserver still fires a callback for it (until we call unobserve). We were calling unobserve but leaving the stale entry in elementsCache, so the Map could slowly grow with detached-node references over the lifetime of a long- running list (frequent unmount/remount, virtualized routes, etc.). Now remove the entry when we detect the disconnect, with a === guard so a delayed callback for an old node doesn't blow away a new node that React has since mounted for the same key. Tests: 2 added — cleanup-on-disconnect, and the don't-clobber-replaced-node edge case.
Cache `process.env.NODE_ENV !== 'production' && opts.key && opts.debug?.()` into a single \`debugEnabled\` flag, then gate all three timing/logging blocks on it. The `process.env.NODE_ENV` prefix lets downstream minifiers (Terser/esbuild/swc with NODE_ENV define) constant-fold the entire flag to false in production and DCE the console.info + Date.now() machinery. Behavior in dev is unchanged — opts.debug() is still polled once per call (rather than three times) but the timings and logs are identical. Bundle size (esbuild --minify --define:process.env.NODE_ENV='"production"'): before: 5219 bytes gzip after: 4999 bytes gzip delta: -220 bytes (-4.2%)
… impl Both observer pairs were near-duplicate functions differing only in how the offset is read from the scroll target. Pull the shared structure into an internal \`observeOffset\` (takes a \`readOffset\` callback) and re-export the two named exports as thin wrappers. Same for \`elementScroll\` / \`windowScroll\`, which were identical except for the generic type parameter — both now alias one underlying function with the right exported signature. No public API change: \`observeElementOffset\`, \`observeWindowOffset\`, \`elementScroll\`, and \`windowScroll\` remain named exports with their original signatures. All adapter packages continue to import them unchanged. Bundle size impact (this is mostly a maintenance refactor): source: -37 LOC dist raw: 31.87 -> 30.70 kB (-1.17 kB) dist gzip: 6.55 -> 6.59 kB (+40 B, gzip already deduplicated the copies) consumer min: 16.55 -> 15.98 kB raw / 4.99 -> 5.00 kB gzip (~flat) Tests: 10 added covering the four exports' contracts before/after refactor.
Drop the \`export * from './utils'\` barrel in favor of explicit named exports — same public surface (\`memo\`, \`debounce\`, \`approxEqual\`, \`notUndefined\`, types \`NoInfer\`, \`PartialKeys\`), now visible at the top of the file. Bundle size impact: zero. Modern bundlers tree-shake the \`export *\` barrel identically. The win is API clarity — the file declares its public surface up front instead of inheriting it implicitly. Adds a "public exports lockdown" test that fails if any of these go missing in a future change.
Adds benchmarks/ — a Vite + React + Playwright harness that runs the same scenarios through the actual public APIs of @tanstack/react-virtual, virtua, react-virtuoso, and react-window v2, then aggregates medians into a markdown table. How: - One page per library at src/pages/, each registering a HarnessHandle so the runner can drive them uniformly without knowing the library. - Shared deterministic dataset (LCG-seeded) so every library renders identical content. - runner/run.mjs spawns the vite preview server, loops over (lib × scenario × run), and writes results/<ts>.json + results/LATEST.md. - Chromium launched with --enable-precise-memory-info and --expose-gc for trustworthy memory readings. Scenarios cover mount (1k, 10k, 100k fixed; 1k, 10k dynamic), dynamic measurement convergence, programmatic scroll, and jump-to-index settle. Run with: cd benchmarks && pnpm bench Sample run (5 runs/cell medians) checked in at results/SAMPLE.json. README documents methodology, results, and known limitations honestly — including that the synthetic scroll test is too gentle to discriminate between the libraries at the sizes tested.
Synthesized findings from official competitor docs, social media, and our own issue tracker. Maps every claim to verification status (TRUE/FALSE/ PARTIAL/UNVERIFIED) and ranks audit priorities. Highlights: - virtua has 17+ explicit iOS code paths; we have zero - virtuoso's 'better scrollTo' claim is FALSE per our benchmark (they're slowest) - virtua's v0.10.0 README had TanStack as the SMALLEST bundle; they removed it - virtua's 'Benchmark: WIP' has been WIP for 3+ years - PR #1141 (useExperimentalDOMVirtualizer) already shows 47% fewer renders Action plan ranked by impact in section 5.
…t path
Replace the eager per-item VirtualItem object loop with a typed-array
backing + a Proxy that builds VirtualItems on first indexed read. The
existing lanes>1 path stays on eager construction (lane assignment is
order-dependent and harder to defer cleanly).
Mechanism:
- Float64Array (stride 2: start, size) holds the dense position data
- Single allocated buffer is reused across rebuilds
- Proxy wraps a sparse cache and materializes a VirtualItem on first
integer read; subsequent reads return the cached object
- resizeItem reads raw start/size from the flat buffer (avoiding Proxy
overhead per call) when in the fast path
Backwards-compatible: measurementsCache still satisfies Array<VirtualItem>
shape; getVirtualItems / calculateRange / getVirtualItemForOffset /
getOffsetForIndex / getTotalSize / resizeItem all work unchanged.
Benchmarks (real Virtualizer, vitest bench):
BEFORE AFTER Speedup
Cold getMeasurements n=10k 0.21ms 0.05ms 4.2x
Cold getMeasurements n=100k 2.52ms 0.54ms 4.7x
Cold getMeasurements n=500k 14.1ms 2.63ms 5.4x
Cold + visible@0 n=100k 2.76ms 0.93ms 3.0x
Cold + visible@0 n=500k 13.98ms 4.65ms 3.0x
100x resize@0 n=10k 26.3ms 15.2ms 1.7x
Bundle size (consumer minified+gzip):
before: 5.00 kB
after: 5.43 kB (+430 B / +8.6%)
The bundle cost buys 5x faster cold mount at 100k+ items and ~3 MB less
memory at 100k (typed array vs N object literals). Closes the gap to
virtua's lazy prefix-sum architecture for the most common (single-lane) case.
Adds 9 regression tests pinning lazy-path behavior: empty list, paddingStart/
scrollMargin/gap, VirtualItem field correctness, identity caching,
out-of-range access, resizeItem→getTotalSize, getVirtualItemForOffset binary
search, 1M-item mount stress test, and the lanes>1 fallback path.
…tum scroll iOS WebKit cancels momentum-scroll the moment you write to scrollTop. Our resizeItem path was unconditionally calling _scrollToOffset whenever an above-viewport item resized, killing momentum and producing the most-cited mobile complaint cluster (issues #545, #622, #884, plus several closed duplicates). Match virtua's pendingJump pattern: detect iOS WebKit (UA + iPadOS-on- MacIntel heuristic), accumulate the delta into _iosDeferredAdjustment while isScrolling, then flush a single scrollTo when isScrolling transitions back to false. Non-iOS code path is unchanged. SSR-safe (returns false when navigator is undefined). Detection result is cached after first call. Adds 3 regression tests: - iOS: adjustment deferred during scroll, flushed on stop - iOS: multiple resizes accumulate into one flush - Non-iOS: no regression — immediate adjustment as before Bundle delta: +190 B gzip (consumer-minified, prod-defined). Cumulative since main: 5.00 -> 5.62 kB (still under 6 kB).
… target
When scrollToIndex(N, { behavior: 'smooth' }) is called on a dynamic-height
list, the destination items haven't been measured yet, so getOffsetForIndex
returns an estimate. As scroll progresses, items become visible and measure
their real heights, shifting the target offset. The reconcile loop detected
this and snapped to behavior:'auto' on the first retarget — that's the
"course correction jolt" reported across many scrollToIndex issues.
New behavior: while still more than one viewport away from the new target,
keep smooth scrolling. The browser's smooth scroll handles repeated target
updates gracefully (continuous motion with adjusted endpoint). Only on the
final approach (within a viewport) do we fall back to 'auto' for precise
landing.
User-visible: one continuous smooth scroll that subtly accelerates/
decelerates instead of an animation followed by a snap.
Addresses recurring complaint pattern across #468, #913, #1001, #1029,
plus discussions about scrollToIndex unreliability with dynamic heights.
Bundle delta: ~+20 B gzip.
… backward The most-cited TanStack Virtual complaint cluster (issues #659, #832, #925, #1028, etc.) is "items jump while I'm scrolling up". The cause: when an above-viewport item resizes during backward scroll, resizeItem writes to scrollTop to compensate — that write actively pushes the viewport away from where the user is scrolling. Multiple users have independently rediscovered the same workaround over the years: gate cache writes on scroll direction. Make it the default in the core: when scrollDirection is 'backward', skip the scroll-position adjustment. Forward scroll and idle measurement keep the existing behavior (needed for stable visible window during forward scroll and for the mount-time measurement storm). Users who genuinely want the old behavior can supply \`shouldAdjustScrollPositionOnItemSizeChange\` (which is checked before the default branch) and ignore the scroll direction in their predicate. Adds 3 regression tests: - backward scroll: adjustment skipped - forward scroll: adjustment still fires - idle: adjustment still fires (mount-time path)
Adds a public takeSnapshot() method that returns the currently-measured items as plain VirtualItem objects, suitable for round-tripping through state storage and feeding back as initialMeasurementsCache on remount. Pair with the current scrollOffset to fully restore scroll position after navigation. Closes the gap to virtua's takeCacheSnapshot() and virtuoso's getState — features cited as TanStack misses in #378, #551, #997 and the virtua/virtuoso comparison tables. The snapshot contains plain objects (not Proxy refs), so it serializes cleanly via JSON.stringify and survives lazy-fast-path materialization. Adds 2 regression tests covering single-lane round-trip and lanes>1. Bundle delta: ~+150 B gzip (one new method body).
…ualItemForOffset The lazy fast path returns a Proxy-wrapped Array<VirtualItem>. Each indexed read triggers a get-trap that materializes a VirtualItem (with allocation) on first access. In hot paths like the binary search inside calculateRange this adds ~17 Proxy traps per scroll event. Pass the underlying Float64Array along to calculateRange so binary-search probes and the forward-end-walk read start/size directly. Same for getVirtualItemForOffset. The Proxy is still used by user-facing getVirtualItems where the consumer expects a real VirtualItem object. Bundle delta: negligible (~+30 B).
…ed array In the lanes===1 fast path, getTotalSize() was calling measurements[N-1].end which triggers a Proxy.get and materializes the last VirtualItem just to read .end. React renders call getTotalSize on every commit, so this matters. Direct typed-array read for the same value. ~no behavior change, marginal perf win.
…is fair The 1px CSS border on the outer scroll-host pushed the inner content down by 1px in libraries whose getScrollContainer returns the host element (TanStack), while libraries with their own internal scrollers (virtuoso) queried past the border. The 'tanstack: 1.0px / virtuoso: 0.0px' result in the prior accuracy bench was the border, not the libraries. Re-measured: TanStack and virtuoso both at 0.0px landing. react-window v2 still off by 135px (verified library issue, not bench artifact). Also: add a defensive 'final exact-landing' write in reconcileScroll once the stable-frames count is met. This is a no-op when scrollTop already equals the target (the usual case) but corrects the rare subpixel-rounding case where the browser's smooth-scroll undershoots by < 1.01px.
Adds the scrollToIndex landing-accuracy scenarios identified as likely competitor strengths: - jump-to-last-accuracy-dynamic-10k: scrollToIndex(N-1, align:'end'). Tests cumulative prefix-sum drift; end-alignment amplifies any error between estimates and real measurements. - jump-while-measuring-accuracy-dynamic-10k: scroll immediately on mount before the visible window has been measured (race condition). - jump-wide-variance-accuracy-10k: items 30..500px, ~16x ratio vs the 30px estimate. Tests convergence when estimates are very wrong. Result across all 4 libraries: TanStack and virtuoso both at 0.0px on every edge case; react-window v2 consistently 135-224px off; virtua's target item didn't render in any of these (page-level quirk). The conventional-wisdom claim that competitors have an accuracy advantage on these specific cases does not hold up to measurement.
…liation, elastic clamp)
…deferral Extends the iOS deferral path from Experiment 2 to track touch state so we can defer scroll-position adjustments through three distinct iOS scroll states instead of one: - active drag (finger on screen) - early-momentum (touch just ended; momentum scroll likely starting) - post-momentum settled Mechanism: - New fields: _iosTouching, _iosJustTouchEnded, _iosTouchEndTimerId - Attach passive touchstart/touchend listeners to the scroll element - touchend on iOS arms a 150 ms grace timer; when it expires we attempt to flush any deferred adjustments - New flush gate: only writes scrollTop when all of !isScrolling, !_iosTouching, !_iosJustTouchEnded hold - All flush paths route through a single _flushIosDeferredIfReady helper Non-iOS behavior is unchanged. The listeners attach unconditionally (passive, cheap on non-touch devices); the gating logic short-circuits without arming timers on non-iOS UAs. Adds 7 regression tests covering touchstart/touchend bookkeeping, grace timer expiry, mid-touch defer, scroll-event-driven flush, re-touch canceling the grace timer, and the non-iOS no-op path.
…Top writes Browser scrollTop/scrollLeft writes are integer-rounded under some DPRs (Safari especially). When we write 12345.5 and the browser reports back 12346 on the resulting scroll event, the reconcile loop thinks the target shifted and re-fires scrollTo — feedback we previously absorbed only via the approxEqual(<1.01) tolerance. Track the intended logical target separately. When the next scroll event reports a value within 1.5 px of our intended write, prefer the intended value over the browser-rounded one. Real user scrolls move further than 1.5 px and skip the reconciliation path. Adds 3 regression tests: subpixel-rounded read reconciles, large-delta user scroll does not reconcile, second self-write replaces intended.
…verscroll Safari's elastic-overscroll (rubber-band) lets scrollTop go negative or exceed scrollHeight-clientHeight while the user drags past the edge. Writing scrollTop during that period would snap the page back to a clamped value at end-of-bounce, often discarding the user's intent. Add an in-bounds guard to _flushIosDeferredIfReady: if scrollTop is outside [0, getMaxScrollOffset()], skip the flush and leave the adjustment deferred. The next in-bounds scroll event retries. Adds 3 regression tests: - Negative scrollTop (overscroll top): flush skipped, then proceeds when scroll snaps back in-bounds - scrollTop > max (overscroll bottom): same pattern - In-bounds scrollTop: flush proceeds normally (no regression)
- Eliminate two redundant non-null assertions in iOS detection and the getVirtualItemForOffset lazy fast-path (eslint @typescript-eslint/no- unnecessary-type-assertion) - Convert takeSnapshot's index-loop to for-of (eslint prefer-for-of) - Align benchmarks/package.json dep versions with the rest of the workspace (typescript 5.6.3, vite ^6.4.2, @playwright/test ^1.53.1, React 18.3.x) so sherif passes - Add 'benchmarks' to knip ignore list (private workspace; unused-export warnings on the per-library page components are intentional) Pre-existing test:ci failures on main (lit-virtual:build, react-virtual:test:e2e) are not from this branch and remain.
- Add `takeSnapshot()` instance method docs with the round-trip example for scroll restoration (pairs with `initialMeasurementsCache`). - Add `initialMeasurementsCache` option docs (previously undocumented). - Update `shouldAdjustScrollPositionOnItemSizeChange` to describe the new default — adjustments are skipped during backward scroll to avoid scroll-up jank — and to note the iOS-specific deferral behavior so consumers aren't surprised by what they see in Safari.
Six changesets covering the major themes: - perf(virtual-core): mount/measure-storm rewrite (lazy materialization + audit hotfixes) [minor] - feat(virtual-core): iOS scroll handling (3-phase deferral) [minor] - feat(virtual-core): default skip backward-scroll adjustment [minor] - feat(virtual-core): takeSnapshot() public method [minor] - feat(virtual-core): smooth scrollToIndex keep-alive [patch] - perf(react-virtual): drop useReducer object allocation [patch]
Audit findings against the writing-style SKILL.md plus the two reference posts (Who Owns the Tree, React Server Components Your Way): - title was clever-indirect; now leads with the noun - folded 3 closer-triplet patterns from intro / community-themes / what- I-didn't-chase sections into comma-joined prose - removed staccato 'A reverse infinite scroll. virtua and virtuoso ship one. We don't yet.' three-sentence stack - folded the two parallel cadence closers in 'What's next' and 'The numbers' sections - removed a colon-introduced list in the 'three layers' iOS section, switched to 'Touch event distinction comes first, ...' prose form - added a brief RSC-protocol callback in the virtuoso/auto-measure section to ground the headless-vs-prescriptive frame in recent work - no em-dashes (was already clean) - no 'isn't just X, it's Y' / 'Here's the thing' / 'To be clear'
Down from 2943 words to 1174 (60% cut). The previous draft read like a release writeup; the reference posts (Who Owns the Tree, RSC Your Way) hit the thesis in one paragraph, drop two or three specifics, and end. This version matches that energy. What got cut: - Detailed audit catalog of 25 findings → one bug example (Map clone) plus a one-sentence list of the rest - Detailed lazy fast-path mechanics → one paragraph naming the trick - iOS Phase 1/2/2b enumeration → one paragraph saying what we defer and when, no implementation breakdown - "What I didn't chase" section → folded into one paragraph at the end - Benchmark methodology dump → one sentence about Playwright - Two-paragraph community-perception inventory → cut entirely (the numbers section does the work) What stayed (the significance): - 1382× measure-storm bug story - 5× cold mount at 100k via lazy fast-path - 0.0 px accuracy match with virtuoso (with the bench-artifact disclosure) - iOS now working, backward-scroll jank gone by default - The "open the benchmark and measure it yourself" closer - The RSC-post callback Reads more like something Tanner would actually write after a long week than a thorough autopsy.
@tanstack/react-virtual ships ~15.1M weekly npm downloads. The next- largest virtualization library is at 4.9M, with virtua at 641K (23x smaller than us) and react-cool-virtual at 20K. We're not the challenger here, we're the gorilla. The previous draft read like a defender refuting attacks from smaller players, which is bad form for a market leader and reads as insecure. This version strips every comparative reference: - Title no longer mentions 'the competition' - Opening no longer relays Twitter/Discord trash talk - Dropped 'About those competitor claims' section entirely - Removed every named callout of virtua, virtuoso, react-window, react-virtualized, react-cool-virtual from the body - Removed the 'they have 17 iOS paths, we had none' framing — kept the technical iOS explanation, dropped the vs-them setup - Removed the accuracy section that called out react-window's bug - Numbers section is now about us only, no competitor delta columns - 'What's next' acknowledges reverse-scroll is missing without saying 'competitors have it' - Benchmark suite mentioned in passing as a tool we built, not framed as a competitive scorecard What stayed: the embarrassing-Map-clone bug story (about our code), the lazy fast-path mechanics (about our work), the iOS implementation detail, the backward-scroll fix, takeSnapshot API, the numbers, and the RSC-post callback in the closer. Reads as a confident leader announcing work, not as someone defending their lunch money.
Eight before/after deltas read more cleanly in a table than as bullets with arrows. Keeps the two non-numeric rows (iOS momentum, backward- scroll jank) in the same table for rhythm.
|
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 (2)
✅ Files skipped from review due to trivial changes (1)
📝 Walkthrough<review_stack_artifact_start /> Single cohesive cohort covering the full set of related changes in the PR: release notes, benchmark workspace, core library implementation and optimizations, tests, and docs.Contains every changed rangeId for the PR. Reviewers should inspect in semantic order: release/docs, benchmark infra, core implementation (utils, observers, flat measurements, fast paths, resize/scroll handling, snapshot), tests/benches.range_437920e6a8c0 range_5643668018b9 range_c9a37415b5ea range_384c7e8bef1a range_210458182c88 range_8762c85a993a range_ca54fd3e3f94 range_fa8b74866f17 range_5b22db72b1e4 range_c7a94c20fe62 range_64102c1564cb range_42e54b5fe3b3 range_a22116d5b35d range_36b56d720385 range_0d023021201b range_6fbe0c37f866 range_0e74dbf43d41 range_a4cac3aff091 range_7e0e5d5377aa range_23a922dec5b2 range_22cc6fbe9438 range_e56fb78142de range_a1ad641aabd7 range_35d4101fd7ba range_7e1df3bdf167 range_5d5ceb82beab range_5b0245d47bff range_cdfd770b85f6 range_2c7899417251 range_d80dda1138f5 range_303baa9d37ba range_c8d0c8cc76c0 range_38ca836ce913 range_117b77b2a752 range_6bb647ac17b5 range_706867e6afee range_2b5379b9a5c4 range_96e215b0c73d range_5e4c7f2de1d9 range_0592d69c30e5 range_65581c125258 range_0f3a73e835d1 range_8dc5734c92ec range_0f4fb3489b76 range_579e1360e569 range_dcf586b99541 range_b1595d3902c6 range_c6ec57ed6a61 range_de78d6e3d1a4 range_0dbdf14ead32 range_5eca6e8056e5 range_0332ea734df0 range_a5e508142235 range_d55b7eab19dc range_afe8b94fc289 range_0746f3768ce8 range_1a3d5b3401ae range_638b5348b44e range_4e01fa951029 range_c452511863a0 range_c5287f6ea393 range_4e36dda31725 range_89d4d627707b range_e57587c42b9f range_347039695a9a range_8ffb51658e47 range_66118a405c41 range_710db081e225 range_f3820211525c range_867084521367 range_99b97d20ce16 range_1d2dabdc3a1d range_30a6f1a9d664 range_cf8d0e331932 range_c738b5f32166 range_f3bd27945891 range_4ed743629973 range_0fc119685f77 range_06ac0b8d208f range_9d4728877e7c range_da3a4fdda6ef range_6bbcadf218a7 range_00cb3b4feb9a range_1b36257e7689 range_ac104a6223c1 range_813517c9c8c1 range_bb9e0bf9cc0e range_2cb1bee0f8c3 range_0557e1632198 range_a9f793d41229 range_debf8d0a4b8d range_bed0a0717d80 range_43d6f964c324 range_e2ff55ee097d range_b38443b376d4 range_f1c15d07a1de range_c3b6b25c5cf1 range_c8cb3d26ef6b range_170b8f965357 range_0ab60610b16e range_5dd0c8701361 range_ce269f356483 range_2e7af262b199 range_c7dfb10adc6d range_e405239f658e range_335cfc4dac49 range_f0f3411e5b75 range_6fdf894f31d7 range_c17818be1785 range_3705762df72c range_c8449d3d9989 range_32044720a21a range_e7072369f60c range_22708fc8ccbc🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
🚀 Changeset Version Preview2 package(s) bumped directly, 6 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 186b3bd
☁️ Nx Cloud last updated this comment at |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
These were useful while the work was in flight but don't earn permanent residence in the public repo. The narrative is captured by: - commit messages (per-change rationale) - changesets (release notes) - docs/api/virtualizer.md (user-facing APIs) - benchmarks/ (reproducible perf claims) - The blog post at tanstack.com#934 Removed: - BLOG_POST.md (lives at tanstack.com now) - COMPETITOR_CLAIMS_VERIFICATION.md (research artifact) - EXPERIMENTS_SUMMARY.md (redundant with commit messages) - IOS_SUPPORT_PLAN.md (plan doc for completed work) - PERFORMANCE_RESEARCH.md (initial audit, captured in commits) - RELEASE_READINESS.md (pre-merge verdict)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/virtual-core/tests/index.test.ts (1)
50-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore global
ResizeObserverafter this test.This mutates global state without cleanup, which can leak into later tests and create order-dependent failures.
Suggested fix
- global.ResizeObserver = mockResizeObserver as any + const prevResizeObserver = global.ResizeObserver + global.ResizeObserver = mockResizeObserver as any + try { + // ...existing test body... + } finally { + global.ResizeObserver = prevResizeObserver + }🤖 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 50 - 60, Save the original global.ResizeObserver before you replace it and restore it after the test: capture const originalResizeObserver = global.ResizeObserver at top of the test file, assign global.ResizeObserver = mockResizeObserver for the test (using the existing mockResizeObserver and resizeCallback variables), and add an afterEach (or finally block) that sets global.ResizeObserver = originalResizeObserver to ensure the global is restored; reference the mock variable names mockResizeObserver, resizeCallback and global.ResizeObserver to locate where to add the save/restore and the cleanup hook.packages/virtual-core/src/index.ts (1)
540-551:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset transient iOS state on
touchcanceland teardown.This touch lifecycle can get stuck. A gesture may end via
touchcancel, andcleanup()currently leaves_iosTouching/_iosJustTouchEnded/_iosDeferredAdjustmentalive. From there_flushIosDeferredIfReady()keeps bailing out, or a deferred delta can leak into the next mounted scroll element.Suggested fix
+ private _resetIosTransientState = () => { + this._iosTouching = false + this._iosJustTouchEnded = false + this._iosDeferredAdjustment = 0 + this._intendedScrollOffset = null + if (this._iosTouchEndTimerId !== null && this.targetWindow != null) { + this.targetWindow.clearTimeout(this._iosTouchEndTimerId) + this._iosTouchEndTimerId = null + } + } + private cleanup = () => { this.unsubs.filter(Boolean).forEach((d) => d!()) this.unsubs = [] this.observer.disconnect() + this._resetIosTransientState() if (this.rafId != null && this.targetWindow) { this.targetWindow.cancelAnimationFrame(this.rafId) this.rafId = null } @@ const onTouchEnd = () => { this._iosTouching = false @@ }, 150) } + const onTouchCancel = () => { + this._resetIosTransientState() + } scrollEl.addEventListener( 'touchstart', onTouchStart, addEventListenerOptions, ) scrollEl.addEventListener( 'touchend', onTouchEnd, addEventListenerOptions, ) + scrollEl.addEventListener( + 'touchcancel', + onTouchCancel, + addEventListenerOptions, + ) this.unsubs.push(() => { scrollEl.removeEventListener('touchstart', onTouchStart) scrollEl.removeEventListener('touchend', onTouchEnd) - if (this._iosTouchEndTimerId !== null && this.targetWindow != null) { - this.targetWindow.clearTimeout(this._iosTouchEndTimerId) - this._iosTouchEndTimerId = null - } + scrollEl.removeEventListener('touchcancel', onTouchCancel) })Also applies to: 629-677, 690-694
🤖 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/src/index.ts` around lines 540 - 551, Reset the transient iOS touch state (_iosTouching, _iosJustTouchEnded, _iosDeferredAdjustment) both when handling touchcancel and during teardown; in the touchcancel handler (where touchend/touchstart logic lives) ensure those three private flags are cleared and any pending deferred delta is discarded, and update the cleanup method (cleanup) to also set _iosTouching = false, _iosJustTouchEnded = false, and _iosDeferredAdjustment = null so _flushIosDeferredIfReady cannot bail out or leak a delta into the next mount. Reference the existing _flushIosDeferredIfReady logic and the touch event handlers to locate where to clear these fields.
🧹 Nitpick comments (1)
benchmarks/src/lib/harness.ts (1)
17-19: ⚡ Quick winAlign
isFullyMeasuredcontract with thewait-dynamic-measureimplementation.The interface/docs say this callback is used for
wait-dynamic-measure, but the action currently ignores it and only checks total-size stability. That makes page-level implementations dead and can skew convergence timing.♻️ Suggested change
} else if (scenario.action === 'wait-dynamic-measure') { @@ while (stableCount < 8 && performance.now() - t0 < 3000) { await nextFrame() const cur = h.getTotalSize() - if (cur === lastTotal && cur > 0) stableCount++ + const sizeStable = cur === lastTotal && cur > 0 + const fullyMeasured = h.isFullyMeasured ? h.isFullyMeasured() : true + if (sizeStable && fullyMeasured) stableCount++ else stableCount = 0 lastTotal = cur } actionMs = performance.now() - t0 }Also applies to: 282-297
🤖 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 `@benchmarks/src/lib/harness.ts` around lines 17 - 19, The wait-dynamic-measure action currently only checks total-size stability and ignores the optional isFullyMeasured callback, so update the wait-dynamic-measure polling logic to call isFullyMeasured() (when present) and require it to return true before treating measurement as converged; keep using the existing total-size/stability checks but combine them with isFullyMeasured() so page-level implementations are respected. Locate the polling loop in the wait-dynamic-measure action and add a conditional that invokes props.isFullyMeasured() (or the interface’s isFullyMeasured) each poll and only resolve when both size stability and isFullyMeasured() (if provided) are satisfied. Ensure the isFullyMeasured signature in the harness/interface remains unchanged.
🤖 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 `@benchmarks/README.md`:
- Around line 31-45: The fenced tree block in benchmarks/README.md is missing a
language hint which triggers MD040 lint errors; update the opening fence for the
ASCII tree (the block that starts with ``` on the tree showing benchmarks/ ├──
src/ ...) to include a language token such as text (i.e. change the fence from
``` to ```text) so the markdown linter recognizes it as a plain-text code block;
locate the fenced block in README.md (the directory tree block) and only modify
the opening fence to include the language hint.
- Line 162: Update the fairness note that incorrectly mentions "React 19" to
match the package's declared React version (e.g., React ^18.3.1); locate the
sentence "React 19 runs in production mode (no `<StrictMode>`)" in
benchmarks/README.md and replace it with a corrected statement such as "React 18
(package declares ^18.3.1) runs in production mode (no `<StrictMode>`)" so the
methodology accurately reflects the actual React version used.
In `@benchmarks/runner/run.mjs`:
- Around line 81-103: The runner currently persists only scenario.id into
results, losing fields like count, itemSize, dynamic, and action and violating
the shared ScenarioResult contract; in runScenario locate where the local
variable scenario (from import '/src/scenarios/types.ts' and
mod.SCENARIOS.find(...)) and the returned result from window.bench.run(scenario)
are assembled, and change the code to include and persist the full scenario
object (merge or attach the complete scenario properties onto the returned
result) so the stored ScenarioResult contains the full scenario shape rather
than only { id }.
- Around line 282-290: The error-path metrics object omits the landingErrorPx
field so failed-run rows have a different schema; update the failure/exception
branch that builds the metrics object (the metrics literal) to include
landingErrorPx with a null (or the same sentinel used elsewhere) so every
metrics object (success and error paths) contains landingErrorPx.
- Around line 90-93: The browser-side dynamic import in page.evaluate currently
uses the source path '/src/scenarios/types.ts' which will fail in Vite preview;
change the import to load the built module or a file served from public (e.g.,
import the compiled scenarios module from the dist/public path or expose a JSON
module) when resolving SCENARIOS inside page.evaluate, and ensure you reference
mod.SCENARIOS and scenario by id as before. Also fix the payloads where only {
id: scenarioId } is persisted (variables around scenarioId / ScenarioInput) to
include the full ScenarioInput fields (count, itemSize, dynamic, action) so runs
are reproducible, and when constructing error result objects (ScenarioMetrics),
include the required landingErrorPx field so error and success metric shapes
match.
In `@benchmarks/src/lib/dataset.ts`:
- Line 57: Replace the bracket-style array type annotations (e.g. Item[],
string[], etc.) in this file with the generic Array<T> form to satisfy
`@typescript-eslint/array-type`; specifically update the return type currently
written as Item[] and the other three similar occurrences to Array<Item> (or the
appropriate Array<...> for their element types) so all four array type
annotations use Array<T> consistently.
In `@benchmarks/src/pages/VirtuaPage.tsx`:
- Around line 38-48: getTotalSize currently queries el but never uses it,
causing the function to always fall back to firstElementChild/hostRef and
produce wrong sizes when a different sized node is present; update getTotalSize
to prefer the discovered element's height by returning el?.scrollHeight (or
el?.clientHeight/offsetHeight if appropriate) before falling back to
(hostRef.current?.firstElementChild as HTMLElement | null)?.scrollHeight, then
hostRef.current?.scrollHeight, and finally 0—modify the return expression in the
getTotalSize function to use el first.
In `@benchmarks/src/scenarios/types.ts`:
- Line 59: The exported SCENARIOS constant is declared with the bracket array
syntax which violates the `@typescript-eslint/array-type` rule; change its type
annotation from ScenarioInput[] to Array<ScenarioInput> (i.e., update the
declaration of SCENARIOS in types.ts to use Array<ScenarioInput>) so the export
satisfies the configured ESLint rule while preserving the existing value.
In `@packages/virtual-core/src/index.ts`:
- Around line 703-708: The deferred iOS flush uses this._iosDeferredAdjustment
and calls this._scrollToOffset(cur, { adjustments: delta, ... }) but fails to
update the running accumulator this.scrollAdjustments; change the flush path so
after computing delta you also add it into this.scrollAdjustments (i.e.,
this.scrollAdjustments += delta) before calling _scrollToOffset, and apply the
same change to the other deferred-flush site around the 1318-1321 region so
subsequent resize calculations use the updated accumulator rather than a stale
offset.
- Around line 1618-1622: measure() currently clears itemSizeCache and
laneAssignments but does not reset pendingMin or measurementsCache, so stale
measurements can persist; update measure() to also reset pendingMin (set to its
initial "no pending" state) and clear or reinitialize measurementsCache so the
next getMeasurements() rebuilds from scratch; keep the existing increments to
itemSizeCacheVersion and the notify(false) call and reference the same symbols
(measure(), pendingMin, measurementsCache, itemSizeCache, laneAssignments,
itemSizeCacheVersion, notify(false)) when making the change.
In `@packages/virtual-core/tests/index.test.ts`:
- Around line 1323-1337: The test uses a wall-clock assertion
(expect(elapsed).toBeLessThan(50)) which is flaky; remove the timing check and
replace it with a deterministic assertion: after constructing the Virtualizer
and calling v['getMeasurements'](), assert that the returned/internal
measurements are a typed numeric buffer (e.g., instance of a TypedArray) or that
measurements.length === 1_000_000 and elements are primitive numbers (typeof
measurements[0] !== 'object'), so the test verifies no per-item object
allocations without relying on elapsed time; reference Virtualizer and its
getMeasurements() call to locate the code to change.
---
Outside diff comments:
In `@packages/virtual-core/src/index.ts`:
- Around line 540-551: Reset the transient iOS touch state (_iosTouching,
_iosJustTouchEnded, _iosDeferredAdjustment) both when handling touchcancel and
during teardown; in the touchcancel handler (where touchend/touchstart logic
lives) ensure those three private flags are cleared and any pending deferred
delta is discarded, and update the cleanup method (cleanup) to also set
_iosTouching = false, _iosJustTouchEnded = false, and _iosDeferredAdjustment =
null so _flushIosDeferredIfReady cannot bail out or leak a delta into the next
mount. Reference the existing _flushIosDeferredIfReady logic and the touch event
handlers to locate where to clear these fields.
In `@packages/virtual-core/tests/index.test.ts`:
- Around line 50-60: Save the original global.ResizeObserver before you replace
it and restore it after the test: capture const originalResizeObserver =
global.ResizeObserver at top of the test file, assign global.ResizeObserver =
mockResizeObserver for the test (using the existing mockResizeObserver and
resizeCallback variables), and add an afterEach (or finally block) that sets
global.ResizeObserver = originalResizeObserver to ensure the global is restored;
reference the mock variable names mockResizeObserver, resizeCallback and
global.ResizeObserver to locate where to add the save/restore and the cleanup
hook.
---
Nitpick comments:
In `@benchmarks/src/lib/harness.ts`:
- Around line 17-19: The wait-dynamic-measure action currently only checks
total-size stability and ignores the optional isFullyMeasured callback, so
update the wait-dynamic-measure polling logic to call isFullyMeasured() (when
present) and require it to return true before treating measurement as converged;
keep using the existing total-size/stability checks but combine them with
isFullyMeasured() so page-level implementations are respected. Locate the
polling loop in the wait-dynamic-measure action and add a conditional that
invokes props.isFullyMeasured() (or the interface’s isFullyMeasured) each poll
and only resolve when both size stability and isFullyMeasured() (if provided)
are satisfied. Ensure the isFullyMeasured signature in the harness/interface
remains unchanged.
🪄 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: 509f8de1-0816-46d5-ac23-3e56ddb65611
⛔ Files ignored due to path filters (2)
.claude/scheduled_tasks.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.changeset/feat-core-ios-scroll-handling.md.changeset/feat-core-scroll-to-index-smooth.md.changeset/feat-core-scroll-up-jank-default.md.changeset/feat-core-take-snapshot.md.changeset/perf-core-mount-and-measure-storm.md.changeset/perf-react-virtual-rerender-alloc.mdbenchmarks/.gitignorebenchmarks/README.mdbenchmarks/index.htmlbenchmarks/package.jsonbenchmarks/results/.gitkeepbenchmarks/results/SAMPLE.jsonbenchmarks/runner/run.mjsbenchmarks/src/lib/dataset.tsbenchmarks/src/lib/harness.tsbenchmarks/src/main.tsxbenchmarks/src/pages/TanstackPage.tsxbenchmarks/src/pages/VirtuaPage.tsxbenchmarks/src/pages/VirtuosoPage.tsxbenchmarks/src/pages/WindowPage.tsxbenchmarks/src/scenarios/types.tsbenchmarks/tsconfig.jsonbenchmarks/vite.config.tsdocs/api/virtualizer.mdknip.jsonpackages/react-virtual/src/index.tsxpackages/virtual-core/src/index.tspackages/virtual-core/src/lazy-measurements.tspackages/virtual-core/src/utils.tspackages/virtual-core/tests/bench.bench.tspackages/virtual-core/tests/index.test.tspnpm-workspace.yaml
Comment on lines
1618
to
1622
| measure = () => { | ||
| this.itemSizeCache = new Map() | ||
| this.laneAssignments = new Map() // Clear lane cache for full re-layout | ||
| this.itemSizeCache.clear() | ||
| this.laneAssignments.clear() // Clear lane cache for full re-layout | ||
| this.itemSizeCacheVersion++ | ||
| this.notify(false) |
There was a problem hiding this comment.
Force measure() to invalidate the full layout.
measure() clears itemSizeCache, but it never resets pendingMin or measurementsCache. If a prior resizeItem() already dirtied some later index, the next getMeasurements() rebuild will preserve stale entries before that point, so measure() does not reliably clear all cached measurements.
Suggested fix
measure = () => {
+ this.pendingMin = 0
+ this.measurementsCache = []
this.itemSizeCache.clear()
this.laneAssignments.clear() // Clear lane cache for full re-layout
this.itemSizeCacheVersion++
this.notify(false)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| measure = () => { | |
| this.itemSizeCache = new Map() | |
| this.laneAssignments = new Map() // Clear lane cache for full re-layout | |
| this.itemSizeCache.clear() | |
| this.laneAssignments.clear() // Clear lane cache for full re-layout | |
| this.itemSizeCacheVersion++ | |
| this.notify(false) | |
| measure = () => { | |
| this.pendingMin = 0 | |
| this.measurementsCache = [] | |
| this.itemSizeCache.clear() | |
| this.laneAssignments.clear() // Clear lane cache for full re-layout | |
| this.itemSizeCacheVersion++ | |
| this.notify(false) |
🤖 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/src/index.ts` around lines 1618 - 1622, measure()
currently clears itemSizeCache and laneAssignments but does not reset pendingMin
or measurementsCache, so stale measurements can persist; update measure() to
also reset pendingMin (set to its initial "no pending" state) and clear or
reinitialize measurementsCache so the next getMeasurements() rebuilds from
scratch; keep the existing increments to itemSizeCacheVersion and the
notify(false) call and reference the same symbols (measure(), pendingMin,
measurementsCache, itemSizeCache, laneAssignments, itemSizeCacheVersion,
notify(false)) when making the change.
Real bugs:
- iOS deferred flush now rolls its delta into scrollAdjustments so any
resize landing before the resulting scroll event sees the correct
effective offset (previously the running accumulator stayed at 0 and
a follow-up correction would compute from the stale pre-flush offset).
- measure() now resets pendingMin so the rebuild starts from index 0.
Without this, a prior resizeItem() that left pendingMin > 0 would
cause the next getMeasurements() to preserve stale entries before
that index, partially defeating the invalidation.
Tests:
- Add a regression test for the measure() / pendingMin interaction.
- Add a regression test that asserts scrollAdjustments tracks the
flushed iOS delta.
- Replace the wall-clock perf budget on the 1M-item lazy-path test
with deterministic functional assertions (length + spot-checks of
start/size/end across the range).
Benchmarks:
- VirtuaPage.getTotalSize() now actually uses the queried sized node
before falling back to firstElementChild / host.
- Runner reads scenarios from window.bench.scenarios instead of a
runtime import('/src/scenarios/types.ts'), which wouldn't resolve
under vite preview (only the built dist is served).
- Persist the full scenario object on every result row (success and
error) and add landingErrorPx to the error-path metrics so the
schema is consistent.
- Use Array<T> annotations in dataset.ts / scenarios/types.ts to
satisfy @typescript-eslint/array-type.
- README: language hint on the tree fence (MD040) and React 18 in
the fairness notes.
| @@ -0,0 +1 @@ | |||
| {"sessionId":"e402bf71-ca74-4aa5-856c-da0c2053caab","pid":78596,"procStart":"Sat May 16 20:13:35 2026","acquiredAt":1779000018499} No newline at end of file | |||
piecyk
approved these changes
May 20, 2026
Collaborator
piecyk
left a comment
piecyk
left a comment
There was a problem hiding this comment.
LGTM 👍 Impressive rewrite. The core perf wins are well-motivated and clearly implemented, let's 🚢 🇮🇹
Comment on lines
+12
to
+17
| if (/iP(hone|od|ad)/.test(navigator.userAgent)) return (_isIOSResult = true) | ||
| // iPadOS 13+ reports as MacIntel; touch-points distinguishes it from desktop. | ||
| const mtp = (navigator as Navigator & { maxTouchPoints?: number }) | ||
| .maxTouchPoints | ||
| return (_isIOSResult = | ||
| navigator.platform === 'MacIntel' && mtp !== undefined && mtp > 0) |
Collaborator
There was a problem hiding this comment.
Looks like navigator.platform is deprecated. It still works today, but could be replaced with navigator.userAgentData?.platform with a fallback.
Suggested change
| if (/iP(hone|od|ad)/.test(navigator.userAgent)) return (_isIOSResult = true) | |
| // iPadOS 13+ reports as MacIntel; touch-points distinguishes it from desktop. | |
| const mtp = (navigator as Navigator & { maxTouchPoints?: number }) | |
| .maxTouchPoints | |
| return (_isIOSResult = | |
| navigator.platform === 'MacIntel' && mtp !== undefined && mtp > 0) | |
| const nav = navigator as Navigator & { | |
| userAgentData?: { | |
| platform?: string | |
| } | |
| maxTouchPoints?: number | |
| } | |
| if (/\b(iPhone|iPod|iPad)\b/.test(nav.userAgent)) { | |
| return (_isIOSResult = true) | |
| } | |
| // iPadOS 13+ may report as macOS/MacIntel; touch-points distinguish it from desktop. | |
| // Use userAgentData.platform when available, otherwise fall back to navigator.platform. | |
| const platform = nav.userAgentData?.platform ?? nav.platform | |
| const maxTouchPoints = nav.maxTouchPoints ?? 0 | |
| return (_isIOSResult = | |
| (platform === 'macOS' || platform === 'MacIntel') && maxTouchPoints > 0)``` |
Member
There was a problem hiding this comment.
I had a couple iterations of safely accessing navigator.platform in all environments safely (like even cloudflare ssr stuff) and I ended up at this util in hotkeys https://github.com/TanStack/hotkeys/blob/404666b65dbec31d47da1be4c2a2210e7e53a615/packages/hotkeys/src/constants.ts#L26
…connect cleanup Commit 843690b added an elementsCache cleanup in the ResizeObserver disconnect path that looked up the cache key via getItemKey(index). When items have been removed from the end of the list, that index can be past items.length, so any user-supplied getItemKey that indexes into the data array throws — exactly the bug PR #1148 had fixed for the non-cleanup paths. Fix: find the cache entry by node identity instead. Iterating elementsCache is O(visible-window), which is fine for a path that only fires on disconnect, and it naturally handles the React-replaced-the- node-under-the-same-key case (the === check just won't match). The stale-index e2e test now passes on both react-virtual and angular-virtual, and the two RO-cleanup unit tests still pass since they were written against node identity, not key lookup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A perf-focused release covering several themes. Release blog post at tanstack.com#934.
Six changesets:
@tanstack/virtual-coreVirtualItemmaterialization (typed-array + Proxy) for the lanes===1 fast path, plus eight smaller audit hotfixes (Map clone, setOptions deopt, Math.min spread, defaultRangeExtractor allocation, elementsCache leak, etc.)@tanstack/virtual-core@tanstack/virtual-core@tanstack/virtual-coretakeSnapshot()public method for scroll-restoration round-trips@tanstack/virtual-corescrollToIndex(N, { behavior: 'smooth' })keeps smooth while still > viewport away from target instead of snapping to 'auto' on first retarget@tanstack/react-virtual{}allocation per scroll dispatchHeadline numbers
resizeItemstorm on 10k itemssetOptions× 10,000 (per render)scrollToIndexlanding accuracy (dynamic 10k)The 1382x
resizeItemwin is real and embarrassing in equal measure (Map clone on every resize purely to invalidate a memo dep).Bundle size
About 430 B for the lazy-materialization machinery, 370 B for iOS handling, and 90 B for the rest of the fixes combined.
Behavior changes
Three default changes consumers should know about:
scrollTopon above-viewport resize by default. Users who relied on the old behavior can supplyshouldAdjustScrollPositionOnItemSizeChange.setOptionsno longer mutates the caller's options object (hidden contract violation; unlikely anyone was relying on the mutation).What's new
virtualizer.takeSnapshot()— returns currently-measured items as plainVirtualItemobjects, pairs withinitialMeasurementsCachefor scroll restoration after navigation. Documented indocs/api/virtualizer.md.initialMeasurementsCacheis now documented (was previously undocumented even though the option already existed).New tests
91 unit tests across
virtual-coreandreact-virtual, up from 13 originally. New coverage for: lazy fast-path edge cases (9), iOS deferral (7), subpixel reconciliation (3), elastic-overscroll clamp (3), backward-scroll skip (3), takeSnapshot round-trip (2), several smaller audit-driven regression tests.Reproducible benchmark suite
Added
benchmarks/— a Vite app + Playwright runner that drives the same scenarios across@tanstack/react-virtualand the other major virtualization libraries and reports medians across runs. Documented inbenchmarks/README.md. Run withcd benchmarks && pnpm bench. Useful as a regression guard for future perf work.Test plan
pnpm test:lib(91/91 passing)pnpm test:typespnpm test:eslintpnpm test:buildpnpm test:knippnpm test:sherifpnpm test:docs(link check)pnpm changesetschema validatespnpm benchon reviewer's machine to confirm numberspnpm changeset:versionpreview before mergingPre-existing
lit-virtual:buildand:test:e2efailures on main are unrelated to this PR.Summary by CodeRabbit
New Features
Bug Fixes
Performance
Benchmarks
Documentation
Tests