fix(virtual-core): cancel the isScrolling debounce on scroll-observer cleanup

.changeset/olive-pugs-repeat.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/virtual-core': patch
+---
+
+Cancel the pending `isScrolling` reset when a scroll observer is torn down, and reset `isScrolling` and `scrollDirection` in `cleanup()` so they don't stay stuck after the scroll element changes or is removed.

packages/marko-virtual/e2e/app/e2e/option-gates.spec.ts

@@ -101,17 +101,6 @@ test('enabled=false disables the virtualizer (empty window); enabled=true re-win
   const deep = await renderedIndexes(page)
   expect(deep[0]!).toBeGreaterThan(50)
 
-  // Let the end-of-scroll debounce fire while STILL ENABLED before toggling.
-  // KNOWN UPSTREAM CORE BUG (found by this gate): core's debounce (utils.ts) has no
-  // cancel, and observeOffset's unsubscribe only removes the event listeners — a
-  // pending end-of-scroll timer survives cleanup() and later fires
-  // cb(staleOffset, false) into the live instance. Disable + re-enable within
-  // isScrollingResetDelay (150ms) and the stale offset overwrites the correct
-  // re-enable recompute, leaving a stale window until the next real scroll event.
-  // This wait sidesteps the zombie timer so the gate asserts the enabled contract
-  // itself; remove it if/when the core fix (cancellable debounce) lands.
-  await page.waitForTimeout(250)
-
   // Disable: the deep window disappears (measurements cleared, empty/collapsed window).
   await page.locator('[data-testid="toggle"]').click()
   await page.waitForFunction(

packages/virtual-core/src/index.ts

@@ -217,6 +217,10 @@ const observeOffset = <T extends Element | Window>(
     if (registerScrollendEvent) {
       element.removeEventListener('scrollend', endHandler)
     }
+    // Removing the listener doesn't retract a reset already queued by the
+    // last scroll, and that call would land on a virtualizer that has been
+    // torn down — in React, a dispatch into an unmounted tree.
+    fallback?.cancel()
   }
 }
 
@@ -763,6 +767,13 @@ export class Virtualizer<
       this.rafId = null
     }
     this.scrollState = null
+    // The debounce cancelled above is the only thing that writes `isScrolling`
+    // back to false, so a cleanup inside the reset window would strand it, and
+    // the direction derived from it, as true. That matters because `cleanup`
+    // also runs when the scroll element changes or `enabled` goes false, where
+    // the instance lives on.
+    this.isScrolling = false
+    this.scrollDirection = null
     // The iOS gesture/deferral state is scoped to the current scroll
     // element: the touch listeners that maintain it were just removed, and
     // an in-flight touch keeps targeting the old element (implicit touch

packages/virtual-core/src/utils.ts

@@ -102,8 +102,18 @@ export const debounce = (
   ms: number,
 ) => {
   let timeoutId: number
-  return function (this: any, ...args: Array<any>) {
-    targetWindow.clearTimeout(timeoutId)
-    timeoutId = targetWindow.setTimeout(() => fn.apply(this, args), ms)
-  }
+  return Object.assign(
+    function (this: any, ...args: Array<any>) {
+      targetWindow.clearTimeout(timeoutId)
+      timeoutId = targetWindow.setTimeout(() => fn.apply(this, args), ms)
+    },
+    {
+      // The handle is closure-local, so a caller that has already
+      // unsubscribed has no way to stop a queued call. Teardown paths use
+      // this to drop the pending invocation instead of letting it land.
+      cancel: () => {
+        targetWindow.clearTimeout(timeoutId)
+      },
+    },
+  )
 }

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

@@ -3361,6 +3361,131 @@ test('observeElementOffset: attaches scroll listener and fires callback with scr
   expect(listeners.has('scroll')).toBe(false)
 })
 
+// ─── cleanup resets the scroll flags ─────────────────────────────────────────
+// The cancelled debounce is the only writer of `isScrolling = false`, and
+// `cleanup()` also runs while the instance stays alive (element swap,
+// `enabled: false`), so it has to reset the flags itself.
+
+const makeScrollFlagsVirtualizer = () => {
+  const MockResizeObserver = vi.fn(function () {
+    return { observe: vi.fn(), unobserve: vi.fn(), disconnect: vi.fn() }
+  })
+  const mockWindow = {
+    requestAnimationFrame: vi.fn(),
+    cancelAnimationFrame: vi.fn(),
+    ResizeObserver: MockResizeObserver,
+  }
+  const makeElement = () =>
+    ({
+      scrollTop: 0,
+      scrollLeft: 0,
+      scrollWidth: 1000,
+      scrollHeight: 5000,
+      offsetWidth: 400,
+      offsetHeight: 600,
+      ownerDocument: { defaultView: mockWindow },
+    }) as unknown as HTMLDivElement
+
+  const first = makeElement()
+  const second = makeElement()
+  let element: HTMLDivElement | null = first
+  let emit: ((offset: number, isScrolling: boolean) => void) | null = null
+
+  const virtualizer = new Virtualizer({
+    count: 100,
+    estimateSize: () => 50,
+    getScrollElement: () => element,
+    scrollToFn: vi.fn(),
+    observeElementRect: (_instance, cb) => {
+      cb({ width: 400, height: 600 })
+      return () => {}
+    },
+    observeElementOffset: (_instance, cb) => {
+      emit = cb
+      return () => {}
+    },
+  })
+
+  virtualizer._willUpdate()
+
+  // Mid-scroll: this is the state the debounce used to clear on its own.
+  emit!(500, true)
+
+  return {
+    virtualizer,
+    swapElement: () => {
+      element = second
+      virtualizer._willUpdate()
+    },
+    disable: () => {
+      element = null
+      virtualizer._willUpdate()
+    },
+  }
+}
+
+test('cleanup resets the scroll flags when the scroll element is swapped', () => {
+  const { virtualizer, swapElement } = makeScrollFlagsVirtualizer()
+
+  expect(virtualizer.isScrolling).toBe(true)
+
+  swapElement()
+
+  expect(virtualizer.isScrolling).toBe(false)
+  expect(virtualizer.scrollDirection).toBe(null)
+})
+
+test('cleanup resets the scroll flags when the scroll element goes away', () => {
+  const { virtualizer, disable } = makeScrollFlagsVirtualizer()
+
+  expect(virtualizer.isScrolling).toBe(true)
+
+  disable()
+
+  expect(virtualizer.isScrolling).toBe(false)
+  expect(virtualizer.scrollDirection).toBe(null)
+})
+
+test('cleanup resets the scroll flags on unmount', () => {
+  const { virtualizer } = makeScrollFlagsVirtualizer()
+
+  expect(virtualizer.isScrolling).toBe(true)
+
+  virtualizer._didMount()()
+
+  expect(virtualizer.isScrolling).toBe(false)
+  expect(virtualizer.scrollDirection).toBe(null)
+})
+
+test('observeElementOffset: cleanup drops the queued isScrolling reset', () => {
+  vi.useFakeTimers()
+  try {
+    const cb = vi.fn()
+    const listeners = new Map<string, EventListener>()
+    const el: any = {
+      scrollTop: 50,
+      scrollLeft: 0,
+      addEventListener: (name: string, fn: any) => listeners.set(name, fn),
+      removeEventListener: (name: string) => listeners.delete(name),
+    }
+    const cleanup = observeElementOffset(makeObserveInstance(el) as any, cb)
+
+    // Each scroll arms a debounce that resets isScrolling to false.
+    listeners.get('scroll')!({} as Event)
+    expect(cb).toHaveBeenCalledWith(50, true)
+    cb.mockClear()
+
+    // Tearing down inside that window must not leave the reset queued —
+    // it would arrive after the consumer stopped listening.
+    cleanup?.()
+    vi.advanceTimersByTime(1000)
+
+    expect(cb).not.toHaveBeenCalled()
+  } finally {
+    vi.useRealTimers()
+  }
+})
+
 test('observeElementOffset: reads scrollLeft + applies isRtl when horizontal', () => {
   const cb = vi.fn()
   const listeners = new Map<string, EventListener>()