Repository navigation
Conversation
…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>
…hase
ScrollState described its lifecycle through `behavior` (switched from
'smooth' to 'auto' by reconcile for the final approach), `hasReachedTarget`
and `startedSmooth`. Their combinations encode four situations the code
tells apart, so name them in one field instead:
smooth a smooth animation is travelling to the target
smoothLanded a smooth scroll is at its target, waiting to retire
smoothFinishing a smooth scroll switched to instant writes for the final
approach; its animation can still report positions
instant instant writes
`isSmoothScroll` (compensation, the prepend anchor sync and far
measurements hold off) is true exactly where `behavior === 'smooth'` was,
and `isAnimating` (a non-read-back scroll event may not be the user) exactly
where `!hasReachedTarget && startedSmooth` was. reconcileScroll's
transitions move the phase where it used to flip those flags. The
ios-deferral diagnostic page logs `phase` instead of `hasReachedTarget`.
No behaviour change: 3000 random event sequences (auto, smooth and instant
commands, animation steps, resizes, drags, frames, cancels, prepends, the
safety valve, iOS) produce identical traces before and after, including
`shouldMeasureDuringScroll` for sample rows at every step, and the
comparison catches each transition broken on purpose.
Refs TanStack#1307
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 113f9b4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
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 |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
|
View your CI Pipeline Execution ↗ for commit 113f9b4
☁️ Nx Cloud last updated this comment at |
|
Reviewed the last commit at 113f9b4 against its parent 2b7ac8b. I could not find a behaviour difference. Differential run, written independently of yours. Same seeded event script on both revisions, full trace compared at every step: every
Events: all four scroll commands with Coverage. All four phases and every transition were reached (5,000-seed run: Sensitivity. Each of these, applied alone to this branch, is caught:
Static. No reader of E2E. Nothing that separates it from #1310. One WebKit failure of Limits: the differential run uses a simulated scroll element, and it did not exercise Nit: the changeset is a |
This branch has not been deployed
2 participants
🎯 Changes
No behaviour change.
ScrollStatedescribed an in-flight scroll command's lifecycle through three fields:behavior(switched from'smooth'to'auto'by reconcile for the final approach, which also re-enables size compensation),hasReachedTarget, andstartedSmooth(added in #1310 because WebKit keeps delivering smooth-animation events after that switch). Each fix in this area had added one. Their combinations encode four situations the code tells apart, so they become one field:type ScrollState = { index, align, toEnd, startedAt, lastTargetOffset, stableFrames, - behavior: ScrollBehavior - hasReachedTarget: boolean - startedSmooth: boolean + phase: 'smooth' | 'smoothLanded' | 'smoothFinishing' | 'instant' }Two helpers replace the flag combinations:
isSmoothScroll(smooth|smoothLanded)shouldMeasureDuringScrollbehavior === 'smooth'isAnimating(smooth|smoothFinishing)!hasReachedTarget && startedSmoothreconcileScrollmoves the phase where it used to flip the flags: at the target,smooth→smoothLandedandsmoothFinishing→instant; on a retarget, a far smooth retarget →smooth, otherwisesmooth→smoothFinishingandsmoothLanded→instant.Evidence
cancelScroll, prepends withanchorTo: 'end', the 5s safety valve, iOS UA, lanes,scrollPaddingStart) produce identical traces before and after: writes,scrollTop, tracked offset, scroll state, andshouldMeasureDuringScrollfor sample rows at every step. Breaking any single transition or helper on purpose is caught. A separate independent probe (600 seeds, includinguseAnimationFrameWithResizeObserverandlanes: 2) also found zero mismatches and exercised every transition.virtual-core: 233 passed + 5 expected failures; types and lint clean. One test that matchedscrollStateonbehavior: 'smooth'now matchesphase: 'smooth'.cached-measurements,smooth-scroll,smooth-prepend,scrollin Chromium, Firefox and WebKit; react e2e 39/39; Marko chat "does NOT follow appends while reading history" 15/15 interleaved against the parent;pnpm run test:prgreen.ios-deferraldiagnostic page (test(virtual-core): pin the iOS deferral and index-scroll orderings measured for #1307 #1308) loggedscrollState.hasReachedTarget; it now logsphase.Merge Danger
Door: two-way. Private state only; no public API or option changes.
Blast Radius: none intended. Every reader of the removed fields is in
virtual-core/srcand maps onto a phase helper; no adapter, test or doc reads them.Part of the plan in #1307 (step 2a), ahead of reworking the iOS deferral state.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
🤖 Generated with Claude Code