From c178fa8e1578112216aef522da9584f12b49be82 Mon Sep 17 00:00:00 2001 From: Darkatse <39111369+Darkatse@users.noreply.github.com> Date: Mon, 9 Nov 2026 01:14:18 -0500 Subject: [PATCH 1/2] fix(virtual-core): recognize the virtualizer's own write when it is reported again at rest Each scroll write is reported a second time when the element comes to rest, by `scrollend` or by the debounced fallback, and the virtualizer read that report as an external scroll. With `useScrollendEvent` it arrives before reconcile and ended commands whose target had just moved. The report is now `restEcho`: at rest, within 1.5px of where the last scroll event reported our own write (`_ownWriteAt`). The two command endings that relied on the misreading are made explicit: - A `restEcho` ends a command only where `external` would and the target has not moved since the command wrote it; a moved target is left to reconcile, which writes only a target that moved. - A prepend's anchor ends an index command that has landed (`_followAnchor`, `_landedBeforeAnchor`), in both modes. A queued command has not written, so it has not landed. The `command-retire` e2e scenarios run with and without `useScrollendEvent`. --- .changeset/scrollend-own-write.md | 5 + .../e2e/app/command-retire/main.tsx | 13 ++ .../e2e/app/test/command-retire.spec.ts | 102 +++++++++----- packages/virtual-core/src/index.ts | 58 ++++++-- .../virtual-core/src/scroll-classifier.ts | 35 ++++- .../virtual-core/tests/ios-fling-wait.test.ts | 17 +++ .../tests/scroll-classifier.test.ts | 28 ++++ .../tests/scroll-commands.test.ts | 124 ++++++++++++++++++ 8 files changed, 335 insertions(+), 47 deletions(-) create mode 100644 .changeset/scrollend-own-write.md diff --git a/.changeset/scrollend-own-write.md b/.changeset/scrollend-own-write.md new file mode 100644 index 000000000..456b6c5dd --- /dev/null +++ b/.changeset/scrollend-own-write.md @@ -0,0 +1,5 @@ +--- +'@tanstack/virtual-core': patch +--- + +With `useScrollendEvent: true`, `scrollToIndex` and `scrollToEnd` no longer stop short of a target that moved while they settled: the `scrollend` that reports the virtualizer's own write again (a size correction, or a write the browser clamped) is no longer read as the user scrolling away. With `anchorTo: 'end'`, rows prepended once a `scrollToIndex` has landed end the command in either mode, so the view stays on the rows it showed rather than moving to the row the index names in the new list. diff --git a/packages/react-virtual/e2e/app/command-retire/main.tsx b/packages/react-virtual/e2e/app/command-retire/main.tsx index e037bf05f..38a38192d 100644 --- a/packages/react-virtual/e2e/app/command-retire/main.tsx +++ b/packages/react-virtual/e2e/app/command-retire/main.tsx @@ -1,5 +1,6 @@ import React from 'react' import ReactDOM from 'react-dom/client' +import { flushSync } from 'react-dom' import { useVirtualizer } from '@tanstack/react-virtual' // Scroll commands that must finish without the user's help (#1307 step 5, @@ -12,6 +13,14 @@ import { useVirtualizer } from '@tanstack/react-virtual' // from the spec through `window.__v`). The compensation moves the element // past the target; the command must retire rather than reconcile until the // 5s safety valve. +// - scrollToIndex, then one ResizeObserver batch measures a row above the +// fold (compensated) and one between the fold and the target (not): the +// target moves further than the correction (rows resized from the spec +// through `window.__resize`). The command must land on the fresh target. +// With `?scrollend=1` (`useScrollendEvent`) the browser also reports each +// write by `scrollend`, before reconcile; every command must finish the same. + +const params = new URLSearchParams(window.location.search) const COUNT = 100 const EST = 50 @@ -27,8 +36,12 @@ const App = () => { getScrollElement: () => parentRef.current, estimateSize: () => EST, overscan: 2, + useScrollendEvent: params.get('scrollend') === '1', }) ;(window as any).__v = virtualizer + // Commits in the caller's task, so the rows are measured in this frame. + ;(window as any).__resize = (next: Record) => + flushSync(() => setSizes((s) => s.map((size, i) => next[i] ?? size))) return (
diff --git a/packages/react-virtual/e2e/app/test/command-retire.spec.ts b/packages/react-virtual/e2e/app/test/command-retire.spec.ts index 262175100..a4e6b1c4f 100644 --- a/packages/react-virtual/e2e/app/test/command-retire.spec.ts +++ b/packages/react-virtual/e2e/app/test/command-retire.spec.ts @@ -14,41 +14,73 @@ const state = (page: Page) => } }) -test.beforeEach(async ({ page }) => { - await page.goto('/command-retire/') - await page.waitForFunction(() => (window as any).__v != null) -}) +for (const scrollend of [false, true]) { + test.describe(`useScrollendEvent=${scrollend}`, () => { + test.beforeEach(async ({ page }) => { + await page.goto(`/command-retire/${scrollend ? '?scrollend=1' : ''}`) + await page.waitForFunction(() => (window as any).__v != null) + }) -test('scrollToIndex lands on the fresh target when rows above shrink in the same handler', async ({ - page, -}) => { - await page.click('[data-testid="index-shrink"]') - // Rows 0–19 lost 30px each: row 90 moved from 4500 to 3900, and the max - // from 4600 to 4000, under the 4500 written first. - // Retired on its own; the safety valve would take 5s. - await expect - .poll(async () => (await state(page)).command, { timeout: 1000 }) - .toBe(false) - const { scrollTop, row90 } = await state(page) - expect(row90).toBe(3900) - expect(scrollTop).toBe(3900) -}) + test('scrollToIndex lands on the fresh target when rows above shrink in the same handler', async ({ + page, + }) => { + await page.click('[data-testid="index-shrink"]') + // Rows 0–19 lost 30px each: row 90 moved from 4500 to 3900, and the max + // from 4600 to 4000, under the 4500 written first. + // Retired on its own; the safety valve would take 5s. + await expect + .poll(async () => (await state(page)).command, { timeout: 1000 }) + .toBe(false) + const { scrollTop, row90 } = await state(page) + expect(row90).toBe(3900) + expect(scrollTop).toBe(3900) + }) -test('scrollToOffset retires after a row above grows, without waiting for the safety valve', async ({ - page, -}) => { - await page.evaluate(() => { - const v = (window as any).__v - const el = document.querySelector('[data-testid="scroll-container"]')! - // Row 10 is measured larger as the write's read-back arrives, before - // reconcile has seen the target reached. - el.addEventListener('scroll', () => v.resizeItem(10, 100), { once: true }) - v.scrollToOffset(2000) + test('scrollToOffset retires after a row above grows, without waiting for the safety valve', async ({ + page, + }) => { + await page.evaluate(() => { + const v = (window as any).__v + const el = document.querySelector('[data-testid="scroll-container"]')! + // Row 10 is measured larger as the write's read-back arrives, before + // reconcile has seen the target reached. + el.addEventListener('scroll', () => v.resizeItem(10, 100), { + once: true, + }) + v.scrollToOffset(2000) + }) + // The compensation keeps the content in place. + await expect.poll(async () => (await state(page)).scrollTop).toBe(2050) + // Retired within a few frames; the safety valve would take 5s. + await expect + .poll(async () => (await state(page)).command, { timeout: 1000 }) + .toBe(false) + }) + + test('scrollToIndex lands on the fresh target when it moves further than a correction', async ({ + page, + }) => { + await page.evaluate(() => { + const v = (window as any).__v + const el = document.querySelector('[data-testid="scroll-container"]')! + // Row 60 centred: 2825, rows 54–66 rendered. At the read-back, after + // this frame's reconcile, row 55 (above the fold, compensated) grows + // by 50 and row 58 (in view, above the target) by 100. + el.addEventListener( + 'scroll', + () => + requestAnimationFrame(() => + (window as any).__resize({ 55: 100, 58: 150 }), + ), + { once: true }, + ) + v.scrollToIndex(60, { align: 'center' }) + }) + await expect + .poll(async () => (await state(page)).command, { timeout: 1000 }) + .toBe(false) + // Row 60 starts at 3150: centred at 2975, not at the correction's 2875. + expect((await state(page)).scrollTop).toBe(2975) + }) }) - // The compensation keeps the content in place. - await expect.poll(async () => (await state(page)).scrollTop).toBe(2050) - // Retired within a few frames; the safety valve would take 5s. - await expect - .poll(async () => (await state(page)).command, { timeout: 1000 }) - .toBe(false) -}) +} diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index bd6e7cb23..2e799dd2e 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -633,6 +633,10 @@ export class Virtualizer< smooth: boolean lowMaxes?: Array } | null = null + // Where the last scroll event reported our own write, until a report + // anywhere else. The browser reports the write again when the element + // comes to rest (`restEcho`). + private _ownWriteAt: number | null = null // A compensation write from `applyScrollAdjustment` whose target exceeded // the element's scroll max at the moment of the write. The browser clamps // such a write because the consumer's sizer has not grown yet: an @@ -956,7 +960,7 @@ export class Virtualizer< // the rows and the total shifted back by it, so the reader's row // stays put, and the flush writes it once a write is possible. this._holdOffset += anchorDelta - this._followOffsetTarget(anchorDelta) + this._followAnchor(anchorDelta) } } } @@ -1081,6 +1085,7 @@ export class Virtualizer< this._clampedAdjustment = null // A write to the old element will never be read back. this.pendingWrite = null + this._ownWriteAt = null this.scrollElement = null this.targetWindow = null } @@ -1170,6 +1175,17 @@ export class Virtualizer< this.scrollState = null } this.pendingWrite = null + // Remember where our own write was reported. Its echo at rest + // leaves that as it is; any other report clears it. + if ( + kind === 'readBack' || + kind === 'clampedReadBack' || + kind === 'clampedEcho' + ) { + this._ownWriteAt = offset + } else if (kind !== 'restEcho') { + this._ownWriteAt = null + } // A pending clamped compensation write (#1258) survives only its // own read-back, which the browser reports at the scroll max we @@ -1292,14 +1308,11 @@ export class Virtualizer< // an update scheduled from a layout effect commits before paint. if (!this._canWriteScroll()) { this._holdOffset += anchor.delta - this._followOffsetTarget(anchor.delta) + this._followAnchor(anchor.delta) this.notify(false) } else if ( isSmoothScroll(this.scrollState) && - !approxEqual( - this.getScrollOffset() - anchor.delta, - this.scrollState.lastTargetOffset, - ) + !this._landedBeforeAnchor(this.scrollState, anchor.delta) ) { // A smooth programmatic scroll is still travelling. Writing scrollTop // here would cancel the browser's animation, and Chromium drops a @@ -1307,10 +1320,10 @@ export class Virtualizer< // the journey would be stranded. The target is index-based and // recomputes against the new layout in reconcileScroll, so let the // animation run; the next scroll event re-syncs the tracked offset. - // A smooth scroll that has already landed (offset at its target, - // reconcile not yet retired it) still gets the anchor sync. + // A smooth scroll that has already landed (reconcile not yet + // retired it) still gets the anchor sync. } else { - this._followOffsetTarget(anchor.delta) + this._followAnchor(anchor.delta) this._scrollToOffset(this.getScrollOffset()) } } else { @@ -1345,6 +1358,7 @@ export class Virtualizer< scrollOffset: this.scrollOffset, pendingWrite: this.pendingWrite, clampedMaxAtWrite: this._clampedAdjustment?.maxAtWrite ?? null, + ownWriteAt: this._ownWriteAt, }) // Re-issue a compensation write the browser clamped because the sizer had @@ -2763,6 +2777,32 @@ export class Virtualizer< } } + // Whether a command had landed where a prepend's anchor (`delta`, already + // in `getScrollOffset()`) found the viewport: it has written (a `queued` + // command has not, wherever the fling is), and reconcile has seen it at + // its target or the viewport is where it last wrote. A correction since + // then can have moved the viewport and the target together. + private _landedBeforeAnchor = (state: ScrollState, delta: number) => + state.phase !== 'queued' && + (state.stableFrames > 0 || + approxEqual(this.getScrollOffset() - delta, state.lastTargetOffset)) + + // A prepend's anchor moves the viewport with the rows it shows. An offset + // target moves with it. An index command that had landed is done: the + // reader stays on the rows it was aimed at, and its index now names + // others. An end target is still the end. + private _followAnchor = (delta: number) => { + this._followOffsetTarget(delta) + const state = this.scrollState + if ( + state?.index != null && + !state.toEnd && + this._landedBeforeAnchor(state, delta) + ) { + this.scrollState = null + } + } + measure = () => { // Reset pendingMin so the next getMeasurements rebuilds from index 0. // Without this, a prior resizeItem() that left pendingMin > 0 would diff --git a/packages/virtual-core/src/scroll-classifier.ts b/packages/virtual-core/src/scroll-classifier.ts index 00130f784..0ede85389 100644 --- a/packages/virtual-core/src/scroll-classifier.ts +++ b/packages/virtual-core/src/scroll-classifier.ts @@ -12,6 +12,8 @@ import { approxEqual } from './utils' // through since the write // clampedEcho a compensation write the browser clamped (#1258) // reporting at the max seen at write time +// restEcho our own write reported again at rest, where the last +// scroll event reported it // external anything else: the user, a fling, a smooth animation // frame type ScrollEventKind = @@ -19,6 +21,7 @@ type ScrollEventKind = | 'readBack' | 'clampedReadBack' | 'clampedEcho' + | 'restEcho' | 'external' export interface ScrollEventInput { @@ -44,6 +47,9 @@ export interface ScrollEventInput { // The element's max when a clamped compensation write was made (#1258), // or null when none is pending. clampedMaxAtWrite: number | null + // Where the last scroll event reported our own write (read back, clamped + // or echoed), until a report anywhere else. + ownWriteAt: number | null } export function classifyScrollEvent({ @@ -54,6 +60,7 @@ export function classifyScrollEvent({ scrollOffset, pendingWrite, clampedMaxAtWrite, + ownWriteAt, }: ScrollEventInput): ScrollEventKind { // A scroll event that reports movement but lands on the offset we already // hold — and isn't a self-write read-back — is a spurious no-op re-emit @@ -99,6 +106,17 @@ export function classifyScrollEvent({ ) { return 'clampedEcho' } + // A write is reported again when the element comes to rest: `scrollend` + // follows its scroll event in the same frame's scroll steps, before + // reconcile; the fallback follows it after `isScrollingResetDelay`. + // Nothing has moved the element in between. + if ( + !isScrolling && + ownWriteAt !== null && + Math.abs(offset - ownWriteAt) < 1.5 + ) { + return 'restEcho' + } return 'external' } @@ -142,14 +160,17 @@ export interface RetireInput { // Whether a scroll event that is not the read-back of our own write retires // the in-flight index command: an external scroll moved away from its -// target. Our own measurement updates (including clamped writes) still need -// to reconcile. +// target, or our own write left the element where reconcile will not move +// it. Our own measurement updates (including clamped writes) still need to +// reconcile. export function externalRetiresCommand({ kind, offset, command, target, }: RetireInput): boolean { + let fresh: number | undefined + const freshTarget = () => (fresh ??= target()) return ( kind !== 'spurious' && kind !== 'readBack' && @@ -170,7 +191,15 @@ export function externalRetiresCommand({ // has landed. !isAnimating(command.phase) && !approxEqual(offset, command.lastTargetOffset) && - !approxEqual(offset, target()) && + !approxEqual(offset, freshTarget()) && + // Our own write reported again at rest (`restEcho`): nothing else has + // moved the element, but reconcile writes only a target that moved. Once + // the target has moved since the command wrote it, reconcile lands the + // command there (with `scrollend`, a correction's echo arrives before + // reconcile has run on the measurements behind it); while it has not, + // nothing will bring the element back, and the command ends here. + (kind !== 'restEcho' || + approxEqual(freshTarget(), command.lastTargetOffset)) && // A compensation write's clamped echo (#1258) keeps the command. The // order is deliberate, not tidiness: this clause stays after `target()` // (and `clampedReadBack` before it) because `target()` can rebuild the diff --git a/packages/virtual-core/tests/ios-fling-wait.test.ts b/packages/virtual-core/tests/ios-fling-wait.test.ts index 263fb5204..d5177cccc 100644 --- a/packages/virtual-core/tests/ios-fling-wait.test.ts +++ b/packages/virtual-core/tests/ios-fling-wait.test.ts @@ -213,6 +213,23 @@ test('iOS 27: a waiting index target follows rows measured during the fling', () expect(h.writes).toEqual([4550]) }) +test('iOS 27: a prepend as the fling passes the waiting target does not end the command', () => { + const h = flingOnIOS27() + h.run(() => { + h.v.setOptions({ ...h.v.options, anchorTo: 'end', getItemKey: (i) => i }) + h.v._willUpdate() + h.v.scrollToIndex(20, { align: 'start' }) + h.scroll(true, 1000) + h.v.setOptions({ ...h.v.options, count: 112, getItemKey: (i) => i - 12 }) + h.v._willUpdate() + h.scroll(false) + }) + h.expireTail() + // The held anchor keeps the reader's rows. Nothing had been written for + // the command, so it had not landed: it is written against the new list. + expect(h.writes).toEqual([1600, 1000]) +}) + // A touch that lands on a scroller still in its fling fires no touch or // pointer events on iOS (seen on the 26.5 and 27.0 simulators and an iPhone // on 27.0.1, #1307). Touch events arrive only once the fling has stopped, diff --git a/packages/virtual-core/tests/scroll-classifier.test.ts b/packages/virtual-core/tests/scroll-classifier.test.ts index d859a2ba4..2ef864417 100644 --- a/packages/virtual-core/tests/scroll-classifier.test.ts +++ b/packages/virtual-core/tests/scroll-classifier.test.ts @@ -21,6 +21,7 @@ const at = (input: Partial) => scrollOffset: 500, pendingWrite: null, clampedMaxAtWrite: null, + ownWriteAt: null, ...input, }) @@ -222,6 +223,19 @@ describe('classifyScrollEvent', () => { ).toBe('clampedEcho') }) }) + + describe('restEcho', () => { + test('at rest within 1.5px of where our own write was reported', () => { + // WebKit reports a write of 1000.5 at 1000. + const rest = { isScrolling: false, ownWriteAt: 1000.5 } + expect(at({ ...rest, offset: 1000 })).toBe('restEcho') + expect(at({ ...rest, offset: 1002 })).toBe('external') + }) + + test('not while scrolling', () => { + expect(at({ offset: 1000, ownWriteAt: 1000 })).toBe('external') + }) + }) }) describe('externalRetiresCommand', () => { @@ -253,6 +267,20 @@ describe('externalRetiresCommand', () => { }, ) + test('our write echoed at rest keeps a command whose target has moved since it wrote it', () => { + expect(retires({ kind: 'restEcho' }, 1200)).toEqual({ + result: false, + targetCalls: 1, + }) + }) + + test('our write echoed at rest retires a command whose target has not moved, computing it once', () => { + expect(retires({ kind: 'restEcho' })).toEqual({ + result: true, + targetCalls: 1, + }) + }) + test('a clamped echo keeps the command, but only after the target check', () => { expect(retires({ kind: 'clampedEcho' })).toEqual({ result: false, diff --git a/packages/virtual-core/tests/scroll-commands.test.ts b/packages/virtual-core/tests/scroll-commands.test.ts index f7eaacaae..cf8f0ec74 100644 --- a/packages/virtual-core/tests/scroll-commands.test.ts +++ b/packages/virtual-core/tests/scroll-commands.test.ts @@ -1027,6 +1027,74 @@ test('reconcileScroll: smooth scroll retargets remain smooth while distance > vi expect(lastCall![1].behavior).toBe('smooth') }) +// ─── a write reported again at rest ────────────────────────────────────────── +// With `useScrollendEvent` a write is reported twice: by its scroll event, +// and by `scrollend` at the same position, both before reconcile (the +// fallback reports it again after `isScrollingResetDelay`). The second +// report is the virtualizer's own write, not the user scrolling away. + +test('a correction reported again at rest does not cancel a scrollToIndex whose target moved further', () => { + const { v, el, scroll, frame, settle } = setupHandDrivenScroll() + + // Row 20 ends at the bottom of the viewport: 1050 - 200. + v.scrollToIndex(20, { align: 'end' }) + scroll(850, true) + scroll(850, false) + frame() + // One ResizeObserver batch: row 10, above the fold, is compensated + // (850 -> 900); row 18, in view above the target, grows by 100. The target + // moves to 1000. The correction's `scrollend` used to read as the user + // scrolling away: the element was neither where the command wrote nor at + // its fresh target. + v.resizeItem(10, 100) + v.resizeItem(18, 150) + expect(el.scrollTop).toBe(900) + scroll(900, true) + scroll(900, false) + settle() + + expect(el.scrollTop).toBe(1000) + expect(v.scrollState).toBeNull() +}) + +test('a clamped write reported again at rest does not cancel scrollToIndex', () => { + // As `index-shrink` in the command-retire e2e (#1310): rows 0–19 shrink + // by 30px each before the read-back, and the browser clamps the write of + // 4500 at the smaller max. + const { v, el, scroll, settle } = setupHandDrivenScroll() + + v.scrollToIndex(90, { align: 'start' }) + for (let i = 0; i < 20; i++) v.resizeItem(i, 20) + el.scrollHeight = 4400 + el.scrollTop = 4200 + scroll(4200, true) + scroll(4200, false) + settle() + + expect(el.scrollTop).toBe(3900) + expect(v.scrollState).toBeNull() +}) + +test('a correction that moves the element off an unmoved target ends the scrollToIndex', () => { + // A predicate that compensates rows below the target too: the element + // moves, the target does not, and reconcile writes only a target that + // moved. The command ends at the correction's echo rather than holding + // until the safety valve. + const { v, el, scroll, frame } = setupHandDrivenScroll() + v.shouldAdjustScrollPositionOnItemSizeChange = () => true + + v.scrollToIndex(20, { align: 'start' }) + scroll(1000, true) + scroll(1000, false) + frame() + v.resizeItem(22, 100) + expect(el.scrollTop).toBe(1050) + scroll(1050, true) + scroll(1050, false) + + expect(v.scrollState).toBeNull() +}) + // ─── a prepend must not strand a travelling smooth scroll ──────────────────── // The end-anchor prepend sync in _willUpdate writes scrollTop instantly. That // cancels a smooth scrollToIndex the browser is still animating, and Chromium @@ -1641,6 +1709,62 @@ test('scrollToOffset: a prepend after the read-back moves the target with the an expect(v.scrollState).toBeNull() }) +test('scrollToIndex: a prepend after a correction keeps the reader on its rows', () => { + // Row 10 grows once reconcile has seen the command at its target: the + // correction moves the viewport and the target together, off where the + // command wrote. The anchor still ends the command, as its index now names + // other rows: reconcile must not take the reader to row 20 of the new + // list (old row 8) before any report at rest. + const { v, el, scroll, frame } = setupHandDrivenScroll({ + anchorTo: 'end', + getItemKey: (i) => i, + }) + + v.scrollToIndex(20, { align: 'start' }) + scroll(1000, true) + scroll(1000, false) + frame() + v.resizeItem(10, 100) + expect(el.scrollTop).toBe(1050) + el.scrollHeight = 5650 + v.setOptions({ ...v.options, count: 112, getItemKey: (i) => i - 12 }) + v._willUpdate() + expect(el.scrollTop).toBe(1650) + scroll(1650, true) + frame() + // Old row 3, above the fold, grows. + v.resizeItem(15, 100) + frame() + + expect(el.scrollTop).toBe(1700) +}) + +test('scrollToEnd: a prepend after the read-back keeps the command following the end', () => { + // The anchor ends an index command that had landed, as its index now names + // other rows; an end target is still the end. + const { v, el, scroll, frame, settle } = setupHandDrivenScroll({ + anchorTo: 'end', + getItemKey: (i) => i, + }) + + v.scrollToEnd() + scroll(4800, true) + scroll(4800, false) + frame() + el.scrollHeight = 5600 + v.setOptions({ ...v.options, count: 112, getItemKey: (i) => i - 12 }) + v._willUpdate() + expect(el.scrollTop).toBe(5400) + scroll(5400, true) + scroll(5400, false) + // A toolbar takes 75px from the viewport before reconcile. + el.clientHeight = 125 + settle() + + expect(el.scrollTop).toBe(5475) + expect(v.scrollState).toBeNull() +}) + test('scrollToOffset: a prepend held by a bounce before the commit moves the target with it', () => { // At the max (4800). The options change finds the element writable, a // bounce past the max lands before the commit, and `_willUpdate` holds From 0878ae4f2ceb9384921424f1c7ca7bbee180d668 Mon Sep 17 00:00:00 2001 From: Darkatse <39111369+Darkatse@users.noreply.github.com> Date: Mon, 9 Nov 2026 01:51:52 -0500 Subject: [PATCH 2/2] fix(virtual-core): compare a rest echo's target exactly, as reconcile does A `restEcho` ends the command only while reconcile will not move it, and reconcile writes any target that differs from the last one (`!==`). With `approxEqual`, a target that moved by under a pixel ended the command, though reconcile was about to land it. --- packages/virtual-core/src/scroll-classifier.ts | 3 +-- packages/virtual-core/tests/scroll-classifier.test.ts | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/virtual-core/src/scroll-classifier.ts b/packages/virtual-core/src/scroll-classifier.ts index 0ede85389..c7cf68338 100644 --- a/packages/virtual-core/src/scroll-classifier.ts +++ b/packages/virtual-core/src/scroll-classifier.ts @@ -198,8 +198,7 @@ export function externalRetiresCommand({ // command there (with `scrollend`, a correction's echo arrives before // reconcile has run on the measurements behind it); while it has not, // nothing will bring the element back, and the command ends here. - (kind !== 'restEcho' || - approxEqual(freshTarget(), command.lastTargetOffset)) && + (kind !== 'restEcho' || freshTarget() === command.lastTargetOffset) && // A compensation write's clamped echo (#1258) keeps the command. The // order is deliberate, not tidiness: this clause stays after `target()` // (and `clampedReadBack` before it) because `target()` can rebuild the diff --git a/packages/virtual-core/tests/scroll-classifier.test.ts b/packages/virtual-core/tests/scroll-classifier.test.ts index 2ef864417..b62011709 100644 --- a/packages/virtual-core/tests/scroll-classifier.test.ts +++ b/packages/virtual-core/tests/scroll-classifier.test.ts @@ -268,7 +268,8 @@ describe('externalRetiresCommand', () => { ) test('our write echoed at rest keeps a command whose target has moved since it wrote it', () => { - expect(retires({ kind: 'restEcho' }, 1200)).toEqual({ + // Half a pixel is a move: reconcile writes any change. + expect(retires({ kind: 'restEcho' }, 1000.5)).toEqual({ result: false, targetCalls: 1, })