Skip to content

fix(virtual-core): let a user scroll end a non-smooth scrollToIndex before it lands - #1310

Draft
piecyk wants to merge 2 commits into
TanStack:mainfrom
piecyk:fix/index-scroll-user-interrupt
Draft

piecyk wants to merge 2 commits into
TanStack:mainfrom
piecyk:fix/index-scroll-user-interrupt

Conversation

@piecyk

@piecyk piecyk commented Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #1309. This branch contains #1309's commit; review only the last commit, fix(virtual-core): let a user scroll end a non-smooth scrollToIndex before it lands. I'll rebase and mark it ready once #1309 merges.

🎯 Changes

The scroll handler treated a scroll event as the user leaving an index scroll only after reconcile had seen the target reached. When the target keeps moving while rows are measured (or, on iOS, when a write is ignored under a finger) the scroll never "reaches" it, so the state stays alive after the user scrolls away, and the next measurement that moves the target writes them back to it. Reproduced on desktop and on iOS in #1307; pinned by two test.fails in #1308.

 scroll event that isn't the read-back of our own write
   if index scroll
-     && hasReachedTarget
+     && (hasReachedTarget || !startedSmooth)
      && offset not ≈ lastTargetOffset and not ≈ fresh target
      && not a clamped compensation read-back
     end the index scroll

 ScrollState
+  startedSmooth  // set when the command starts; `behavior` is switched to
+                 // 'auto' by reconcile for the final approach
  • Non-smooth: the write lands at once, so any other event is the user's.
  • Clamped writes (fix(virtual-core): reconcile index scrolls after pending measurements #1294): a read-back short of what we wrote because the scroll max shrank reports the max, where the fresh target also clamps, so the existing "≈ fresh target" check keeps it reconciling. Clamped compensation writes stay covered by _clampedAdjustment.
  • Smooth: unchanged. Animation events look like a user's, so a smooth scroll still yields only after reaching its target. Decided by startedSmooth rather than behavior, because WebKit can deliver animation events after reconcile's switch to 'auto'.

Evidence

Case before after
#1308 "a drag that never reached the target is not pulled back when the target moves" (iOS, write ignored under a finger) 2550 (pulled back) 60
#1308 "a scroll away from a target that moved before landing is not pulled back, non-iOS" 2600 1050
Smooth scrollToIndex switched to 'auto' near the end, then a late animation event kept kept (new test; fails if the check uses behavior)
#1294 clamp: read-back at a shrunken max before the target is reached kept kept
  • virtual-core: 233 passed + 5 expected failures (remaining iOS scroll-correction deferral: per-row record, iOS 27 momentum, and index-scroll exits #1307 iOS cases). Mutation check: every part of the condition is covered.
  • react-virtual cached-measurements, smooth-scroll, smooth-prepend, scroll in Chromium, Firefox and WebKit: all pass. WebKit A/B against the parent, 4 runs each interleaved: 0 vs 0 failures (checking behavior instead of startedSmooth gave 8 failures).
  • react-virtual e2e 39/39; Marko chat "does NOT follow appends while reading history" 10/10; pnpm run test:pr green.

Merge Danger

Door: two-way. One condition in the scroll handler plus a flag set where scroll commands start.

Blast Radius: non-smooth scrollToIndex / scrollToEnd while they haven't been seen to land. A user scroll (wheel inertia included) now ends them immediately instead of being pulled back by the next measurement, which matches native scrollIntoView. A non-user event in that window that is neither our read-back nor at the target would now end the scroll too; a differential sweep over 98 scenarios (align, lanes, gap, paddings, scroll paddings, scrollMargin, horizontal, window scroller, custom adjust predicate, anchorTo: 'end', rAF-deferred ResizeObserver, iOS) found none.

Known and unchanged: a prepend (anchorTo: 'end') committing before an index write's read-back still loses the scroll (on main it was stranded, then pulled back later). Tracked with the other shared stalls in #1307.

✅ Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm run test:pr.

🚀 Release Impact

  • This change affects published code, and I have generated a changeset.
  • This change is docs/CI/dev-only (no release).

🤖 Generated with Claude Code

piecyk and others added 2 commits October 11, 2026 16:49
…ead-back to reconcile

After a scrollToIndex/scrollToEnd/reconcile write, `scrollOffset` keeps the
old position until the scroll event reads the write back, and the rows on
screen are still the ones rendered for it. A resize in that window was
compensated against the old position: scrolling up with scrollToIndex could
jump back to the old position plus the delta, and reconcile (target
unchanged) never rewrote it until the 5s safety valve.

Track the offset of the last non-smooth write until its read-back. While a
scroll state is active and `scrollOffset` doesn't reflect that write yet,
skip resize compensation: reconcileScroll retargets from fresh measurements
on the next frame. Compensation writes and the prepend anchor sync update
`scrollOffset` eagerly, so they are unaffected. scrollBy builds on the
written offset, so a scrollBy right after a scroll command adds to its
target.

Classifying those resizes against the written offset instead was tried and
regressed the chat examples: during scrollToEnd the rows at the old
position were compensated, the end target kept moving, and a user scroll
could no longer interrupt the never-landed pin.

Adds tests/ios-differential.test.ts, which runs scripted event sequences on
an iOS and a non-iOS virtualizer and checks they settle at the same offset
without needing the safety valve (TanStack#1307).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efore it lands

The scroll handler only treated a scroll event as the user leaving an index
scroll once reconcile had seen the target reached. A target that keeps
moving while rows are measured (or, on iOS, a write ignored under a finger)
never "reaches" it, so the state stayed alive after the user scrolled away
and the next measurement that moved the target wrote them back to it.

A non-smooth write lands at once, so a scroll event that isn't the
read-back of our own write is the user's even before the target is reached.
A write the browser reads back short of what we wrote because the scroll
max shrank reports the max, where the fresh target also clamps, so it keeps
reconciling as before (TanStack#1294); clamped compensation writes stay covered by
`_clampedAdjustment`. Smooth scrolls are unchanged: their animation events
look like a user's, so they still yield only after reaching the target.
That is decided by a new `startedSmooth` flag rather than `behavior`, which
reconcile switches to 'auto' for the final approach while WebKit can still
deliver animation events.

Removes `.fails` from the two TanStack#1308 pull-back tests, and makes the TanStack#1294
"keeps an index scroll active before reaching its target" test model the
max shrinking before the read-back.

Refs TanStack#1307

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Oct 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2b7ac8b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@tanstack/virtual-core Patch
@tanstack/angular-virtual Patch
@tanstack/lit-virtual Patch
@tanstack/marko-virtual Patch
@tanstack/react-virtual Patch
@tanstack/solid-virtual Patch
@tanstack/svelte-virtual Patch
@tanstack/vue-virtual Patch
@tanstack/virtual-benchmarks Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 2b7ac8b

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 2m 49s View ↗
nx run-many --target=build --exclude=examples/** ✅ Succeeded 13s View ↗

☁️ Nx Cloud last updated this comment at 2026-10-11 16:10:45 UTC

@pkg-pr-new

pkg-pr-new Bot commented Oct 11, 2026

Copy link
Copy Markdown
More templates

@tanstack/angular-virtual

npm i https://pkg.pr.new/@tanstack/angular-virtual@1310

@tanstack/lit-virtual

npm i https://pkg.pr.new/@tanstack/lit-virtual@1310

@tanstack/marko-virtual

npm i https://pkg.pr.new/@tanstack/marko-virtual@1310

@tanstack/react-virtual

npm i https://pkg.pr.new/@tanstack/react-virtual@1310

@tanstack/solid-virtual

npm i https://pkg.pr.new/@tanstack/solid-virtual@1310

@tanstack/svelte-virtual

npm i https://pkg.pr.new/@tanstack/svelte-virtual@1310

@tanstack/virtual-core

npm i https://pkg.pr.new/@tanstack/virtual-core@1310

@tanstack/vue-virtual

npm i https://pkg.pr.new/@tanstack/vue-virtual@1310

commit: 2b7ac8b

@phcorp

phcorp commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Reviewed the last commit at 2b7ac8b. The fix works in real browsers, I found no regression, and it also removes a flake in the Chromium chat spec.

The fix, in real browsers. Plain-DOM list on virtual-core, 1000 rows estimated at 50px and measuring 50–109px. scrollToIndex(500, { align: 'start' }), then scrollTop -= 300. Same numbers in Chromium, Firefox and WebKit:

user scroll one frame later user scroll in the same task
main, #1309 pulled back, row 500 at the top pulled back, row 500 at the top
this PR stays, moved only by the 110px of size corrections stays, moved only by 115px of corrections

It fixes the chat.spec.ts flake. react-virtual e2e, Chromium, 40 repetitions per revision in interleaved blocks of 10, --retries=0:

Test main #1309 this PR #1311
keeps visible messages stable when history is prepended 18/40 19/40 40/40 40/40
does not follow appended messages while reading history 29/40 37/40 40/40 40/40

The first one is the chat.spec.ts:73 timeout I reported in #1308.

No landing regression. The 34,200 uninterrupted index scrolls from my #1309 comment land identically on #1309 and on this PR. The only difference is that 54 runs that sat off-target until the 5s safety valve on #1309 now retire at the next scroll-rest event, at the same position.

Mutation check. Checking behavior instead of startedSmooth, reverting to hasReachedTarget only, and always yielding each fail the tests you would expect. Dropping the lastTargetOffset comparison (not new in this PR) fails nothing.

Two questions, neither a blocker as far as I could measure:

  1. A scrollToFn that does not land in the same task, like the docs' smooth-scroll example (rAF easing, scrollToIndex with the default behavior). Its first intermediate scroll event is neither the read-back nor at the target, so the index scroll now ends on the first animation frame instead of living until it lands. With fixed rows the landing is the same (row 500 at 0 on all revisions, Chromium). With dynamic rows that pattern is already broken on main (row 500 about 20,700px away after 9s, about 22,000px here), so I cannot call it a regression. But startedSmooth only knows about behavior: 'smooth', and a custom scrollToFn is the other way a write can travel. Is that a case you want to keep alive, or is it out of scope?

  2. A clamped read-back while the fresh target is below the new max. The comment says the read-back at the max "is also where the fresh target clamps". That does not hold if rows above the target shrink in the same window. Simulated: 100 rows of 50px, scrollToIndex(90, { align: 'start' }) writes 4500, rows 0–19 measure at 20px before the read-back, the max drops to 4200 and the target to 3900. main lands on 3900; this PR stops at 4200 with row 90 300px above the viewport. I could not produce that ordering in a real browser or anywhere in the sweep, so it may be unreachable in practice. Flagging it because the comment reads as if it cannot happen.

WebKit smooth-scroll and smooth-prepend are flaky on my machine on every revision including main, at similar rates (numbers in #1307). I did not reproduce the 0 vs 0 from your A/B, but nothing there points at this PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants