fix(virtual-core): key scroll retirement off where the scroll parked

Review follow-up. Retirement was gated on a reachedTarget boolean that was
re-armed only when the target moved more than a viewport, on the grounds
that measurement settling nudges it a pixel or two. That left a gap: a
retarget larger than the 1.01px arrival tolerance but smaller than the
viewport kept the scroll marked as arrived while it was genuinely
travelling to the new target, so the next prepend retired it early and
stranded the reader — the same class of bug as cancelling an in-flight
smooth scroll, just in a narrower window.

Replace the boolean with arrivedAtOffset: the offset the scroll actually
came to rest at, or null while travelling. Cleared whenever reconcile
re-drives the scroll and recorded again when it next comes to rest, so
"arrived, then the reader moved away" is compared against where we parked
rather than against a target that measurement settling has since moved.
Retirement no longer consults viewport size at all.

Adds a regression test for the sub-viewport retarget. Both existing tests
still hold, and each of the three fails on exactly one of the earlier
revisions:

                        pristine  unconditional  viewport  arrivedAt
  clobbered anchor        FAIL        pass          pass      pass
  in-flight smooth        pass        FAIL          pass      pass
  sub-viewport retarget   pass        FAIL          FAIL      pass

react-virtual e2e clean over 20 consecutive runs; marko 82 x2, angular
13 x3, core 131/131, repo-wide eslint/types/lib/build green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: piecyk

packages/virtual-core/src/index.ts

@@ -386,13 +386,13 @@ type ScrollState = {
   // settling
   stableFrames: number
 
-  // Whether the scroll position has ever actually reached `lastTargetOffset`.
-  // Separates "still travelling" from "arrived": only an arrived scroll can be
-  // stale, so only an arrived scroll may be retired out from under the
-  // reconcile loop. Re-armed when the target moves more than a viewport — a
-  // genuinely new journey — but not for the pixel-scale drift that measurement
-  // settling produces.
-  reachedTarget: boolean
+  // The offset this scroll came to rest at on its target, or null while it is
+  // still travelling. Separates "arrived" from "in flight": only an arrived
+  // scroll can be stale, so only an arrived scroll may be retired out from
+  // under the reconcile loop. Cleared whenever reconcile re-drives the scroll,
+  // and set again when it next comes to rest — so it records where we actually
+  // parked rather than how far the target happened to move.
+  arrivedAtOffset: number | null
 }
 
 type PendingScrollAnchor = [
@@ -650,6 +650,10 @@ export class Virtualizer<
     // frame, producing a visible "jump" on prepend with dynamic sizes.
     let anchorResolved = false
     let anchorDelta = 0
+    // Where the reader was sitting before this correction shifted them, used
+    // below to tell "they left the place a finished scroll parked them" from
+    // "they are still where it parked them".
+    let offsetBeforeAnchor = 0
     if (anchor && this.scrollOffset !== null) {
       const [anchorKey, anchorOffset] = anchor
       const newMeasurements = this.getMeasurements()
@@ -668,6 +672,7 @@ export class Virtualizer<
           const newOffset = Math.max(0, anchorItem.start + anchorOffset)
           if (newOffset !== this.scrollOffset) {
             anchorDelta = newOffset - this.scrollOffset
+            offsetBeforeAnchor = this.scrollOffset
             this.scrollOffset = newOffset
             anchorResolved = true
           }
@@ -689,15 +694,17 @@ export class Virtualizer<
     // there — overwriting this correction and dropping the user several
     // screens from the message they were reading.
     //
-    // `reachedTarget` is what keeps this off scrolls that are still travelling:
-    // a prepend during an in-flight `scrollToIndex`, smooth or not, must leave
-    // `scrollState` alone so reconcile can still land on the requested index
-    // at its new position.
+    // `arrivedAtOffset` is what keeps this off scrolls that are still
+    // travelling: a prepend during an in-flight `scrollToIndex`, smooth or not,
+    // must leave `scrollState` alone so reconcile can still land on the
+    // requested index at its new position. Comparing against where the scroll
+    // parked — not against its current target — is what makes this independent
+    // of how far measurement settling has since nudged that target.
     if (
       anchorResolved &&
       this.scrollState !== null &&
-      this.scrollState.reachedTarget &&
-      !approxEqual(this.getScrollOffset(), this.scrollState.lastTargetOffset)
+      this.scrollState.arrivedAtOffset !== null &&
+      !approxEqual(offsetBeforeAnchor, this.scrollState.arrivedAtOffset)
     ) {
       this.scrollState = null
     }
@@ -885,15 +892,15 @@ export class Virtualizer<
           }
           this._intendedScrollOffset = null
 
-          // Record arrival at the programmatic target. This only ever *marks*
-          // — retiring `scrollState` from here would abort long scrolls, since
-          // the browser moves scrollTop itself as items measure and a smooth
-          // animation arrives entirely as events we did not write.
+          // Record where a programmatic scroll came to rest. This only ever
+          // *records* — retiring `scrollState` from here would abort long
+          // scrolls, since the browser moves scrollTop itself as items measure
+          // and a smooth animation arrives entirely as events we did not write.
           if (
             this.scrollState !== null &&
             approxEqual(offset, this.scrollState.lastTargetOffset)
           ) {
-            this.scrollState.reachedTarget = true
+            this.scrollState.arrivedAtOffset = offset
           }
 
           this.scrollAdjustments = 0
@@ -1094,7 +1101,7 @@ export class Virtualizer<
     const targetChanged = targetOffset !== this.scrollState.lastTargetOffset
 
     if (!targetChanged && approxEqual(targetOffset, this.getScrollOffset())) {
-      this.scrollState.reachedTarget = true
+      this.scrollState.arrivedAtOffset = this.getScrollOffset()
       this.scrollState.stableFrames++
       if (this.scrollState.stableFrames >= STABLE_FRAMES) {
         // Final-pass exact landing. The reconcile-stable check uses a 1.01px
@@ -1130,16 +1137,14 @@ export class Virtualizer<
 
         this.scrollState.lastTargetOffset = targetOffset
 
-        // A destination more than a viewport away is a new journey, so
-        // "arrived" no longer holds — without this re-arm a scroll that
-        // reached an earlier target would stay eligible for retirement while
-        // genuinely in flight toward the new one. Small moves do NOT re-arm:
-        // measurements settling right after mount routinely nudge the target a
-        // pixel or two, and dropping `reachedTarget` for that would forget
-        // that we are sitting on the destination.
-        if (distance > viewport) {
-          this.scrollState.reachedTarget = false
-        }
+        // We are about to re-drive the scroll, so it is travelling again and
+        // any previous resting place is void. It is recorded afresh when the
+        // write below lands and the resulting scroll event observes us at the
+        // new target. Keying off the re-drive rather than off how far the
+        // target moved is what keeps this independent of viewport size: a
+        // measurement nudge of a pixel and a genuine retarget are handled the
+        // same way, by waiting until we actually come to rest again.
+        this.scrollState.arrivedAtOffset = null
         if (!keepSmooth) {
           this.scrollState.behavior = 'auto'
         }
@@ -1854,7 +1859,7 @@ export class Virtualizer<
       startedAt: now,
       lastTargetOffset: offset,
       stableFrames: 0,
-      reachedTarget: false,
+      arrivedAtOffset: null,
     }
 
     this._scrollToOffset(offset, { adjustments: undefined, behavior })
@@ -1889,7 +1894,7 @@ export class Virtualizer<
       startedAt: now,
       lastTargetOffset: offset,
       stableFrames: 0,
-      reachedTarget: false,
+      arrivedAtOffset: null,
     }
 
     this._scrollToOffset(offset, { adjustments: undefined, behavior })
@@ -1911,7 +1916,7 @@ export class Virtualizer<
       startedAt: now,
       lastTargetOffset: offset,
       stableFrames: 0,
-      reachedTarget: false,
+      arrivedAtOffset: null,
     }
 
     this._scrollToOffset(offset, { adjustments: undefined, behavior })

packages/virtual-core/tests/index.test.ts

@@ -3802,3 +3802,86 @@ test('a prepend during an in-flight smooth scroll leaves it alone', () => {
   expect(v['scrollState']).not.toBeNull()
   expect(v['scrollState']!.index).toBe(60)
 })
+
+test('a sub-viewport retarget mid-flight does not make the scroll retirable', () => {
+  // Retirement keys off where the scroll came to REST, not off its current
+  // target. An earlier revision re-armed only when the target moved more than a
+  // viewport, on the grounds that measurement settling nudges it a pixel or two.
+  // That left a gap: a measurement-driven retarget larger than the 1.01px
+  // arrival tolerance but smaller than the viewport kept the scroll marked as
+  // arrived while it was genuinely travelling to the new target, so the next
+  // prepend retired it early and stranded the reader.
+  const rafCallbacks: Array<FrameRequestCallback> = []
+  const mockWindow = {
+    requestAnimationFrame: vi.fn((cb: FrameRequestCallback) => {
+      rafCallbacks.push(cb)
+      return rafCallbacks.length
+    }),
+    cancelAnimationFrame: vi.fn(),
+    performance: { now: () => Date.now() },
+    ResizeObserver: vi.fn(function () {
+      return { observe: vi.fn(), unobserve: vi.fn(), disconnect: vi.fn() }
+    }),
+  }
+  const el = {
+    scrollTop: 0,
+    scrollLeft: 0,
+    scrollWidth: 400,
+    scrollHeight: 1000, // 20 x 50
+    clientWidth: 400,
+    clientHeight: 200,
+    offsetWidth: 400,
+    offsetHeight: 200,
+    ownerDocument: { defaultView: mockWindow },
+    scrollTo: vi.fn(),
+  } as unknown as HTMLDivElement
+  const scrollToFn = vi.fn()
+  let scrollCallback: ((offset: number, isScrolling: boolean) => void) | null =
+    null
+
+  const rows = Array.from({ length: 20 }, (_, i) => `r-${i}`)
+  const v = new Virtualizer<HTMLDivElement, any>({
+    count: rows.length,
+    estimateSize: () => 50,
+    anchorTo: 'end',
+    getItemKey: (i) => rows[i]!,
+    getScrollElement: () => el,
+    scrollToFn,
+    observeElementRect: (_inst, cb) => {
+      cb({ width: 400, height: 200 })
+      return () => {}
+    },
+    observeElementOffset: (_inst, cb) => {
+      scrollCallback = cb
+      cb(0, false)
+      return () => {}
+    },
+  })
+  v._willUpdate()
+  v.getVirtualItems()
+
+  // Land on index 10, so the scroll is genuinely "arrived".
+  v.scrollToIndex(10, { align: 'start' })
+  const target = scrollToFn.mock.calls.at(-1)![0]
+  scrollCallback!(target, false)
+  v.getVirtualItems()
+
+  // A row above the viewport measures 100px taller. Index 10 moves by 100 —
+  // more than the 1.01px arrival tolerance, less than the 200px viewport — so
+  // reconcile re-drives the scroll toward the new target.
+  v.resizeItem(0, 150)
+  v.getVirtualItems()
+  rafCallbacks.splice(0).forEach((cb) => cb(0))
+
+  // History arrives before that re-driven scroll has landed.
+  rows.unshift('r--5', 'r--4', 'r--3', 'r--2', 'r--1')
+  ;(el as any).scrollHeight = 1350
+  v.setOptions({
+    ...v.options,
+    count: rows.length,
+    getItemKey: (i: number) => rows[i]!,
+  })
+
+  // It is still in flight, so it must survive to be landed by reconcile.
+  expect(v['scrollState']).not.toBeNull()
+})