From 9117fc3ba8af8f9b5dd8aab8993c3e3f11d2af4f Mon Sep 17 00:00:00 2001 From: gianluca-guarini Date: Mon, 7 Sep 2026 10:24:31 +0200 Subject: [PATCH 1/4] fix: #1258 --- .changeset/fix-scrolling-issue-1258.md | 5 ++ packages/react-virtual/e2e/app/chat/main.tsx | 5 ++ .../react-virtual/e2e/app/test/chat.spec.ts | 12 +++ packages/virtual-core/src/index.ts | 87 ++++++++++++------- 4 files changed, 79 insertions(+), 30 deletions(-) create mode 100644 .changeset/fix-scrolling-issue-1258.md diff --git a/.changeset/fix-scrolling-issue-1258.md b/.changeset/fix-scrolling-issue-1258.md new file mode 100644 index 000000000..0cd277f8c --- /dev/null +++ b/.changeset/fix-scrolling-issue-1258.md @@ -0,0 +1,5 @@ +--- +'@tanstack/virtual-core': patch +--- + +Fix scroll adjustment issue when reaching scroll limit. diff --git a/packages/react-virtual/e2e/app/chat/main.tsx b/packages/react-virtual/e2e/app/chat/main.tsx index 497095812..e30fe0d75 100644 --- a/packages/react-virtual/e2e/app/chat/main.tsx +++ b/packages/react-virtual/e2e/app/chat/main.tsx @@ -25,6 +25,10 @@ function App() { const firstMessageIndexRef = React.useRef(0) const nextMessageIndexRef = React.useRef(initialMessages.length) + const paddingEnd = Number( + new URLSearchParams(window.location.search).get('paddingEnd') ?? 0, + ) + const virtualizer = useVirtualizer({ count: messages.length, getScrollElement: () => parentRef.current, @@ -34,6 +38,7 @@ function App() { followOnAppend: true, scrollEndThreshold: 4, overscan: 4, + paddingEnd, }) React.useLayoutEffect(() => { diff --git a/packages/react-virtual/e2e/app/test/chat.spec.ts b/packages/react-virtual/e2e/app/test/chat.spec.ts index b35b3333b..1d574723b 100644 --- a/packages/react-virtual/e2e/app/test/chat.spec.ts +++ b/packages/react-virtual/e2e/app/test/chat.spec.ts @@ -158,3 +158,15 @@ test('chat mode keeps streaming bottom message pinned as it grows', async ({ await expect(page.locator('[data-testid="message-m-29"]')).toBeVisible() }) + +test('chat mode keeps streaming bottom message pinned as it grows with paddingEnd', async ({ + page, +}) => { + await page.goto('/chat/?paddingEnd=80') + await waitForEnd(page) + + await page.click('#grow-last') + await waitForEnd(page) + + await expect(page.locator('[data-testid="message-m-29"]')).toBeVisible() +}) diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index dc6f1010c..1d7e4ec53 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -447,6 +447,7 @@ export class Virtualizer< // value when the diff is < 1.5 px, distinguishing it from a real user // scroll. The +0.5 over Math.abs lets us also absorb the +1 / -1 cases. private _intendedScrollOffset: number | null = null + private _maxScrollOffsetAtWrite: number | null = null shouldAdjustScrollPositionOnItemSizeChange: | undefined | (( @@ -480,32 +481,31 @@ export class Virtualizer< // it. We can't call getItemKey(index) here because items may // have been removed since this node was rendered — the index // could be stale and out-of-bounds in the user's data array - // (regression test in e2e/.../stale-index.spec.ts, fix #1148). - // The === comparison naturally handles the React-replaced- - // a-node-for-the-same-key case: that entry now points to a - // different node, so this loop won't match. - for (const [cacheKey, cachedNode] of this.elementsCache) { - if (cachedNode === node) { - this.elementsCache.delete(cacheKey) - break - } - } - return - } - - if (!this.isIndexInRange(index)) return + // (regression test in e2e/.../stale-index.spec.ts, fix #1148). + // The === comparison naturally handles the React-replaced- + // a-node-for-the-same-key case: that entry now points to a + // different node, so this loop won't match. + for (const [cacheKey, cachedNode] of this.elementsCache) { + if (cachedNode === node) { + this.elementsCache.delete(cacheKey) + break + } + } + return + } + if (!this.isIndexInRange(index)) return + if (this.shouldMeasureDuringScroll(index)) { + this.resizeItem( + index, + this.options.measureElement(node, entry, this), + ) + } + } + this.options.useAnimationFrameWithResizeObserver + ? requestAnimationFrame(run) + : run() +}) - if (this.shouldMeasureDuringScroll(index)) { - this.resizeItem( - index, - this.options.measureElement(node, entry, this), - ) - } - } - this.options.useAnimationFrameWithResizeObserver - ? requestAnimationFrame(run) - : run() - }) })) } @@ -853,11 +853,14 @@ export class Virtualizer< // self-write — by the time the user has moved 1.5 px, the // intended value will already have been consumed by a prior // scroll event and cleared. + const intendedOffset = this._intendedScrollOffset + const maxAtWrite = this._maxScrollOffsetAtWrite + if ( - this._intendedScrollOffset !== null && - Math.abs(offset - this._intendedScrollOffset) < 1.5 + intendedOffset !== null && + Math.abs(offset - intendedOffset) < 1.5 ) { - offset = this._intendedScrollOffset + offset = intendedOffset } this._intendedScrollOffset = null @@ -882,12 +885,35 @@ export class Virtualizer< // screen, and the post-touchend grace window has expired. this._flushIosDeferredIfReady() + // Check if we hit the scroll limit we recorded at write time. + // If we landed on that limit and are still short of the + // intended offset, the scroll container hadn't grown yet when + // the write was issued (e.g. paddingEnd, dynamic content). The + // container may have grown since — re-issue the write so we + // reach the intended position now that there's room. + if ( + intendedOffset !== null && + maxAtWrite !== null && + offset === maxAtWrite && + offset < intendedOffset && + this.getMaxScrollOffset() > maxAtWrite + ) { + this._maxScrollOffsetAtWrite = null + this._scrollToOffset(intendedOffset, { + adjustments: undefined, + behavior: undefined, + }) + } else { + this._maxScrollOffsetAtWrite = null + } + if (this.scrollState) { this.scheduleScrollReconcile() } this.maybeNotify() - }), - ) + }), + ) + // Touch event listeners (iOS-aware deferral). We attach unconditionally // — the listeners are passive and cheap; on non-touch devices they @@ -1970,6 +1996,7 @@ export class Virtualizer< // Record the intended logical scroll target so the next scroll event // can reconcile against subpixel rounding by the browser. this._intendedScrollOffset = offset + (adjustments ?? 0) + this._maxScrollOffsetAtWrite = this.getMaxScrollOffset() this.options.scrollToFn(offset, { behavior, adjustments }, this) } From c294fcdcd5402e8e3d2fcc62aef39549bc64eae9 Mon Sep 17 00:00:00 2001 From: gianluca-guarini Date: Mon, 7 Sep 2026 10:40:13 +0200 Subject: [PATCH 2/4] fix: E2E chat tests --- packages/virtual-core/src/index.ts | 43 +++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index 1d7e4ec53..f38df16b6 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -886,23 +886,19 @@ export class Virtualizer< this._flushIosDeferredIfReady() // Check if we hit the scroll limit we recorded at write time. - // If we landed on that limit and are still short of the - // intended offset, the scroll container hadn't grown yet when - // the write was issued (e.g. paddingEnd, dynamic content). The - // container may have grown since — re-issue the write so we - // reach the intended position now that there's room. + // If we landed exactly on that limit but are still short of the + // intended offset, the scroll container hadn't grown yet when the + // write was issued (e.g. paddingEnd + a growing last item). Keep + // _maxScrollOffsetAtWrite set so _willUpdate can re-issue the write + // once React has committed the new sizer size and there is room. if ( intendedOffset !== null && maxAtWrite !== null && offset === maxAtWrite && - offset < intendedOffset && - this.getMaxScrollOffset() > maxAtWrite + offset < intendedOffset ) { - this._maxScrollOffsetAtWrite = null - this._scrollToOffset(intendedOffset, { - adjustments: undefined, - behavior: undefined, - }) + // Leave _maxScrollOffsetAtWrite intact — _willUpdate will clear it + // and re-issue once getMaxScrollOffset() has grown. } else { this._maxScrollOffsetAtWrite = null } @@ -1006,6 +1002,29 @@ export class Virtualizer< this.scrollToEnd({ behavior: followOnAppend }) } } + + // Re-issue a previously clamped scroll write now that React has committed + // and the sizer may have grown. This handles the case where + // applyScrollAdjustment wrote a scrollTop that the browser clamped because + // the sizer hadn't caught up yet (e.g. paddingEnd + a growing last item). + // _maxScrollOffsetAtWrite is kept set by the scroll callback when it detects + // a clamped self-write; we clear it here once we can satisfy the offset. + if ( + this._maxScrollOffsetAtWrite !== null && + this._intendedScrollOffset !== null && + this.scrollElement && + this.options.enabled + ) { + const newMax = this.getMaxScrollOffset() + if (newMax > this._maxScrollOffsetAtWrite) { + const intended = this._intendedScrollOffset + this._maxScrollOffsetAtWrite = null + this._scrollToOffset(intended, { + adjustments: undefined, + behavior: undefined, + }) + } + } } // Apply any accumulated iOS-deferred scroll adjustment, but only when we're From 937b7bce33256c6dbe2fed2a579095ea45d477da Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Mon, 7 Sep 2026 13:33:43 +0200 Subject: [PATCH 3/4] fix(virtual-core): re-issue clamped end-anchor compensation once the sizer grows (#1258) Co-Authored-By: Claude Fable 5.1 --- .changeset/fix-scrolling-issue-1258.md | 2 +- packages/virtual-core/src/index.ts | 144 ++++++++++++---------- packages/virtual-core/tests/index.test.ts | 105 +++++++++++++++- 3 files changed, 182 insertions(+), 69 deletions(-) diff --git a/.changeset/fix-scrolling-issue-1258.md b/.changeset/fix-scrolling-issue-1258.md index 0cd277f8c..9823468e7 100644 --- a/.changeset/fix-scrolling-issue-1258.md +++ b/.changeset/fix-scrolling-issue-1258.md @@ -2,4 +2,4 @@ '@tanstack/virtual-core': patch --- -Fix scroll adjustment issue when reaching scroll limit. +Fix end-anchored streaming growth with `paddingEnd`: re-issue the scroll compensation write when the browser clamped it because the sizer had not grown yet (#1258). diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index f38df16b6..c30049e92 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -447,7 +447,16 @@ export class Virtualizer< // value when the diff is < 1.5 px, distinguishing it from a real user // scroll. The +0.5 over Math.abs lets us also absorb the +1 / -1 cases. private _intendedScrollOffset: number | null = null - private _maxScrollOffsetAtWrite: 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 + // end-anchored item growing at the bottom only extends `scrollHeight` to + // its own end, so with `paddingEnd > 0` the clamp lands exactly + // `paddingEnd` short of the target (#1258). `_willUpdate` re-issues the + // write once the sizer has caught up; the clamped read-back keeps it + // pending, any other scroll event (a real gesture) cancels it. + private _clampedAdjustment: { target: number; maxAtWrite: number } | null = + null shouldAdjustScrollPositionOnItemSizeChange: | undefined | (( @@ -481,31 +490,32 @@ export class Virtualizer< // it. We can't call getItemKey(index) here because items may // have been removed since this node was rendered — the index // could be stale and out-of-bounds in the user's data array - // (regression test in e2e/.../stale-index.spec.ts, fix #1148). - // The === comparison naturally handles the React-replaced- - // a-node-for-the-same-key case: that entry now points to a - // different node, so this loop won't match. - for (const [cacheKey, cachedNode] of this.elementsCache) { - if (cachedNode === node) { - this.elementsCache.delete(cacheKey) - break - } - } - return - } - if (!this.isIndexInRange(index)) return - if (this.shouldMeasureDuringScroll(index)) { - this.resizeItem( - index, - this.options.measureElement(node, entry, this), - ) - } - } - this.options.useAnimationFrameWithResizeObserver - ? requestAnimationFrame(run) - : run() -}) + // (regression test in e2e/.../stale-index.spec.ts, fix #1148). + // The === comparison naturally handles the React-replaced- + // a-node-for-the-same-key case: that entry now points to a + // different node, so this loop won't match. + for (const [cacheKey, cachedNode] of this.elementsCache) { + if (cachedNode === node) { + this.elementsCache.delete(cacheKey) + break + } + } + return + } + if (!this.isIndexInRange(index)) return + + if (this.shouldMeasureDuringScroll(index)) { + this.resizeItem( + index, + this.options.measureElement(node, entry, this), + ) + } + } + this.options.useAnimationFrameWithResizeObserver + ? requestAnimationFrame(run) + : run() + }) })) } @@ -706,6 +716,18 @@ export class Virtualizer< this._iosDeferredAdjustment += delta return false } else { + const target = this.getScrollOffset() + this.scrollAdjustments + delta + // Guarded so a bare test double without `scrollHeight` / `document` + // does not crash in `getMaxScrollOffset`. + const el = this.scrollElement + const maxAtWrite = + el !== null && ('scrollHeight' in el || 'document' in el) + ? this.getMaxScrollOffset() + : null + this._clampedAdjustment = + maxAtWrite !== null && target > maxAtWrite + 0.5 + ? { target, maxAtWrite } + : null this._scrollToOffset(this.getScrollOffset(), { adjustments: (this.scrollAdjustments += delta), behavior, @@ -787,6 +809,7 @@ export class Virtualizer< this._iosDeferredAdjustment = 0 this._iosTouching = false this._iosJustTouchEnded = false + this._clampedAdjustment = null this.scrollElement = null this.targetWindow = null } @@ -853,17 +876,25 @@ export class Virtualizer< // self-write — by the time the user has moved 1.5 px, the // intended value will already have been consumed by a prior // scroll event and cleared. - const intendedOffset = this._intendedScrollOffset - const maxAtWrite = this._maxScrollOffsetAtWrite - if ( - intendedOffset !== null && - Math.abs(offset - intendedOffset) < 1.5 + this._intendedScrollOffset !== null && + Math.abs(offset - this._intendedScrollOffset) < 1.5 ) { - offset = intendedOffset + offset = this._intendedScrollOffset } this._intendedScrollOffset = null + // A pending clamped compensation write (#1258) survives only its + // own read-back, which the browser reports at the scroll max we + // saw at write time. Anything else is a real gesture (or the write + // landing after all), so drop it rather than yank the user later. + if ( + this._clampedAdjustment !== null && + Math.abs(offset - this._clampedAdjustment.maxAtWrite) >= 1.5 + ) { + this._clampedAdjustment = null + } + this.scrollAdjustments = 0 // If the offset hasn't moved, this is the echo of our own // adjustment write — `applyScrollAdjustment` already folded it @@ -885,31 +916,12 @@ export class Virtualizer< // screen, and the post-touchend grace window has expired. this._flushIosDeferredIfReady() - // Check if we hit the scroll limit we recorded at write time. - // If we landed exactly on that limit but are still short of the - // intended offset, the scroll container hadn't grown yet when the - // write was issued (e.g. paddingEnd + a growing last item). Keep - // _maxScrollOffsetAtWrite set so _willUpdate can re-issue the write - // once React has committed the new sizer size and there is room. - if ( - intendedOffset !== null && - maxAtWrite !== null && - offset === maxAtWrite && - offset < intendedOffset - ) { - // Leave _maxScrollOffsetAtWrite intact — _willUpdate will clear it - // and re-issue once getMaxScrollOffset() has grown. - } else { - this._maxScrollOffsetAtWrite = null - } - if (this.scrollState) { this.scheduleScrollReconcile() } this.maybeNotify() - }), - ) - + }), + ) // Touch event listeners (iOS-aware deferral). We attach unconditionally // — the listeners are passive and cheap; on non-touch devices they @@ -1003,23 +1015,24 @@ export class Virtualizer< } } - // Re-issue a previously clamped scroll write now that React has committed - // and the sizer may have grown. This handles the case where - // applyScrollAdjustment wrote a scrollTop that the browser clamped because - // the sizer hadn't caught up yet (e.g. paddingEnd + a growing last item). - // _maxScrollOffsetAtWrite is kept set by the scroll callback when it detects - // a clamped self-write; we clear it here once we can satisfy the offset. + // Re-issue a compensation write the browser clamped because the sizer + // had not grown yet (#1258). By now the consumer has committed the new + // total size, so there may be room. Runs before the clamped read-back + // in a synchronous (flushSync) render and after it otherwise; both + // paths leave `_clampedAdjustment` set, so the timing does not matter. if ( - this._maxScrollOffsetAtWrite !== null && - this._intendedScrollOffset !== null && + this._clampedAdjustment !== null && this.scrollElement && this.options.enabled ) { - const newMax = this.getMaxScrollOffset() - if (newMax > this._maxScrollOffsetAtWrite) { - const intended = this._intendedScrollOffset - this._maxScrollOffsetAtWrite = null - this._scrollToOffset(intended, { + const { target, maxAtWrite } = this._clampedAdjustment + const max = this.getMaxScrollOffset() + if (max > maxAtWrite + 0.5) { + // Still short (the sizer grew only partially): stay pending against + // the new max so the next commit retries. + this._clampedAdjustment = + target > max + 0.5 ? { target, maxAtWrite: max } : null + this._scrollToOffset(target, { adjustments: undefined, behavior: undefined, }) @@ -2015,7 +2028,6 @@ export class Virtualizer< // Record the intended logical scroll target so the next scroll event // can reconcile against subpixel rounding by the browser. this._intendedScrollOffset = offset + (adjustments ?? 0) - this._maxScrollOffsetAtWrite = this.getMaxScrollOffset() this.options.scrollToFn(offset, { behavior, adjustments }, this) } diff --git a/packages/virtual-core/tests/index.test.ts b/packages/virtual-core/tests/index.test.ts index 05a674663..00417bb02 100644 --- a/packages/virtual-core/tests/index.test.ts +++ b/packages/virtual-core/tests/index.test.ts @@ -2879,6 +2879,7 @@ function createChatVirtualizer({ itemSize = 50, followOnAppend = false, threshold = 1, + paddingEnd = 0, }: { messages: Array<{ id: string }> offset: number @@ -2886,13 +2887,16 @@ function createChatVirtualizer({ itemSize?: number followOnAppend?: boolean | 'auto' | 'smooth' | 'instant' threshold?: number + paddingEnd?: number }) { let currentMessages = messages const scrollToFn = vi.fn() + let offsetCb: ((offset: number, isScrolling: boolean) => void) | null = null const scrollElement = { scrollTop: offset, scrollLeft: 0, - scrollHeight: messages.length * itemSize, + // The sizer is `getTotalSize()`, which includes paddingEnd. + scrollHeight: messages.length * itemSize + paddingEnd, scrollWidth: 1000, clientHeight: viewportSize, clientWidth: 400, @@ -2935,12 +2939,14 @@ function createChatVirtualizer({ _instance: any, cb: (offset: number, isScrolling: boolean) => void, ) => { + offsetCb = cb cb(scrollElement.scrollTop, false) return () => {} }, anchorTo: 'end' as const, followOnAppend, scrollEndThreshold: threshold, + paddingEnd, } } @@ -2956,9 +2962,15 @@ function createChatVirtualizer({ setMessages(nextMessages: Array<{ id: string }>) { currentMessages = nextMessages virtualizer.setOptions(makeOptions()) - ;(scrollElement as any).scrollHeight = nextMessages.length * itemSize + ;(scrollElement as any).scrollHeight = + nextMessages.length * itemSize + paddingEnd virtualizer._willUpdate() }, + // Simulate the browser's scroll event after a write (or a gesture). + emitScroll(nextOffset: number, isScrolling = false) { + scrollElement.scrollTop = nextOffset + offsetCb?.(nextOffset, isScrolling) + }, } } @@ -3753,3 +3765,92 @@ test('#1218: first measurement of a spanning item still compensates', () => { expect(v.scrollOffset).toBe(before + 70) }) + +// ─── #1258: compensation write clamped by a not-yet-grown sizer ───────────── +// 5 items × 50px + paddingEnd 80 = 330px sizer, 200px viewport → the bottom is +// scrollTop 130. When the last item grows by 70, resizeItem writes 200, but +// the consumer has not committed the 400px sizer yet, so the browser clamps +// the write to the current max (130) — exactly paddingEnd short once the +// overflowing item alone has extended scrollHeight. The write must be +// re-issued once the sizer has grown. + +function clampedGrowthSetup() { + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const setup = createChatVirtualizer({ + messages, + offset: 130, + paddingEnd: 80, + threshold: 0, + }) + setup.virtualizer.resizeItem(4, 120) + // The end-anchor compensation fired against the not-yet-grown sizer. + expect(setup.scrollToFn).toHaveBeenCalledTimes(1) + expect(setup.scrollToFn.mock.calls[0]![1].adjustments).toBe(70) + return setup +} + +test('#1258: clamped end-anchor write is re-issued after the read-back once the sizer grows', () => { + const { virtualizer, scrollElement, scrollToFn, emitScroll } = + clampedGrowthSetup() + + // Browser read-back of the clamped write: it landed on the old max. + emitScroll(130) + // Consumer commits the grown sizer, then the layout effect runs. + ;(scrollElement as any).scrollHeight = 400 + virtualizer._willUpdate() + + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(200) +}) + +test('#1258: clamped end-anchor write is re-issued in a sync render before the read-back', () => { + const { virtualizer, scrollElement, scrollToFn } = clampedGrowthSetup() + + // flushSync path: the layout effect runs before the scroll event arrives. + ;(scrollElement as any).scrollHeight = 400 + virtualizer._willUpdate() + + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(200) +}) + +test('#1258: a render without sizer growth does not re-issue; a later one does', () => { + const { virtualizer, scrollElement, scrollToFn, emitScroll } = + clampedGrowthSetup() + + emitScroll(130) + virtualizer._willUpdate() + expect(scrollToFn).toHaveBeenCalledTimes(1) + ;(scrollElement as any).scrollHeight = 400 + virtualizer._willUpdate() + expect(scrollToFn).toHaveBeenCalledTimes(2) +}) + +test('#1258: a real gesture after the clamp cancels the pending re-issue', () => { + const { virtualizer, scrollElement, scrollToFn, emitScroll } = + clampedGrowthSetup() + + emitScroll(130) + // The user scrolls up to read history. + emitScroll(60, true) + ;(scrollElement as any).scrollHeight = 400 + virtualizer._willUpdate() + + expect(scrollToFn).toHaveBeenCalledTimes(1) +}) + +test('#1258: an unclamped compensation write is not re-issued when the sizer later grows', () => { + // 8 × 50 = 400px, viewport 200, user reading at 100. Item 0 (above the + // fold) re-measures +10 → compensation writes 110, well within max 200. + const messages = Array.from({ length: 8 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, scrollElement, scrollToFn, emitScroll } = + createChatVirtualizer({ messages, offset: 100 }) + + virtualizer.resizeItem(0, 60) + expect(scrollToFn).toHaveBeenCalledTimes(1) + emitScroll(110) + ;(scrollElement as any).scrollHeight = 450 + virtualizer._willUpdate() + + expect(scrollToFn).toHaveBeenCalledTimes(1) +}) From a01c52fba0f71cf09eeac14d01b6b8cf5f817d50 Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Mon, 7 Sep 2026 13:59:51 +0200 Subject: [PATCH 4/4] fix(virtual-core): retry clamped compensation after notify so direct DOM consumers recover without a re-render (#1266) Co-Authored-By: Claude Fable 5.1 --- .changeset/fix-scrolling-issue-1258.md | 2 +- .../e2e/app/chat-resize/index.html | 10 +++ .../e2e/app/chat-resize/main.tsx | 89 +++++++++++++++++++ .../react-virtual/e2e/app/test/chat.spec.ts | 22 +++++ packages/react-virtual/e2e/app/vite.config.ts | 1 + packages/virtual-core/src/index.ts | 54 ++++++----- packages/virtual-core/tests/index.test.ts | 79 ++++++++++++++++ 7 files changed, 236 insertions(+), 21 deletions(-) create mode 100644 packages/react-virtual/e2e/app/chat-resize/index.html create mode 100644 packages/react-virtual/e2e/app/chat-resize/main.tsx diff --git a/.changeset/fix-scrolling-issue-1258.md b/.changeset/fix-scrolling-issue-1258.md index 9823468e7..503f7660b 100644 --- a/.changeset/fix-scrolling-issue-1258.md +++ b/.changeset/fix-scrolling-issue-1258.md @@ -2,4 +2,4 @@ '@tanstack/virtual-core': patch --- -Fix end-anchored streaming growth with `paddingEnd`: re-issue the scroll compensation write when the browser clamped it because the sizer had not grown yet (#1258). +Recover the bottom pin when the browser clamps an end-anchored scroll compensation write. `resizeItem` compensates a size change by writing `scrollTop` before the consumer has committed the new total size, so when the grown item does not itself extend the scroll range the browser clamps the write to the old maximum and the viewport is left short of the end with no scroll event to correct it. Two cases hit this: `paddingEnd > 0` with a growing last item, where the overflowing item only extends `scrollHeight` to its own end and the clamp lands exactly `paddingEnd` short ([#1258](https://github.com/TanStack/virtual/issues/1258)); and a row above the last one growing while the last row keeps its size, under `directDomUpdates` ([#1266](https://github.com/TanStack/virtual/issues/1266)). A compensation write whose target exceeds the scroll maximum at write time is now recorded as clamped and re-issued once the sizer has grown — right after `notify` for consumers that size the container synchronously in `onChange`, and from `_willUpdate` for consumers that size it during a render. The clamped read-back keeps the retry pending; any other scroll event cancels it, so a user reading history is never yanked. diff --git a/packages/react-virtual/e2e/app/chat-resize/index.html b/packages/react-virtual/e2e/app/chat-resize/index.html new file mode 100644 index 000000000..56f418f61 --- /dev/null +++ b/packages/react-virtual/e2e/app/chat-resize/index.html @@ -0,0 +1,10 @@ + + + + + + +
+ + + diff --git a/packages/react-virtual/e2e/app/chat-resize/main.tsx b/packages/react-virtual/e2e/app/chat-resize/main.tsx new file mode 100644 index 000000000..2eb5dcbaa --- /dev/null +++ b/packages/react-virtual/e2e/app/chat-resize/main.tsx @@ -0,0 +1,89 @@ +// Exact reproduction of #1266, adapted from PR #1265 by @tigerBeA: an +// end-anchored direct-DOM chat where a row ABOVE the last one grows. The last +// row is exactly the viewport height and keeps its size, so its overflow cannot +// extend the scroll range before the sizer is updated, and with `useFlushSync: +// false` and an unchanged range nothing re-renders after the resize. +import React from 'react' +import { createRoot } from 'react-dom/client' +import { useVirtualizer } from '@tanstack/react-virtual' + +const VIEWPORT_HEIGHT = 300 +const initialMessages = Array.from({ length: 8 }, (_, index) => ({ + id: `m-${index}`, + height: index === 7 ? VIEWPORT_HEIGHT : 50, +})) + +function App() { + const [messages, setMessages] = React.useState(initialMessages) + const parentRef = React.useRef(null) + const virtualizer = useVirtualizer({ + count: messages.length, + getScrollElement: () => parentRef.current, + getItemKey: (index) => messages[index]!.id, + estimateSize: (index) => initialMessages[index]!.height, + anchorTo: 'end', + followOnAppend: true, + scrollEndThreshold: 4, + overscan: 4, + directDomUpdates: true, + useFlushSync: false, + }) + + React.useLayoutEffect(() => { + virtualizer.scrollToEnd() + }, [virtualizer]) + + return ( +
+ +
+
+ {virtualizer.getVirtualItems().map((item) => ( +
+
+ Message {messages[item.index]!.id} +
+
+ ))} +
+
+
+ ) +} + +createRoot(document.getElementById('root')!).render() diff --git a/packages/react-virtual/e2e/app/test/chat.spec.ts b/packages/react-virtual/e2e/app/test/chat.spec.ts index 1d574723b..ed2075a39 100644 --- a/packages/react-virtual/e2e/app/test/chat.spec.ts +++ b/packages/react-virtual/e2e/app/test/chat.spec.ts @@ -170,3 +170,25 @@ test('chat mode keeps streaming bottom message pinned as it grows with paddingEn await expect(page.locator('[data-testid="message-m-29"]')).toBeVisible() }) + +// #1266 — adapted from PR #1265 by @tigerBeA. Direct DOM updates, no flushSync, +// and a row ABOVE the last one grows: the compensation write is clamped against +// the old scroll range, and with an unchanged range nothing re-renders afterwards, +// so only the post-notify retry in core can recover the lost distance. +test('direct DOM chat stays pinned when a previous message grows without a re-render', async ({ + page, +}) => { + await page.goto('/chat-resize/') + await waitForEnd(page) + // Let the initial scrollToEnd's isScrolling debounce settle first. Its reset + // triggers a re-render that would run _willUpdate and mask a missing retry. + await page.waitForTimeout(300) + const before = await getScrollState(page) + + await page.click('#grow-previous') + await expect + .poll(async () => (await getScrollState(page)).scrollHeight) + .toBe(before.scrollHeight + 24) + await waitForEnd(page) + expect((await getScrollState(page)).scrollTop).toBe(before.scrollTop + 24) +}) diff --git a/packages/react-virtual/e2e/app/vite.config.ts b/packages/react-virtual/e2e/app/vite.config.ts index e5594d0f2..505279170 100644 --- a/packages/react-virtual/e2e/app/vite.config.ts +++ b/packages/react-virtual/e2e/app/vite.config.ts @@ -11,6 +11,7 @@ export default defineConfig({ scroll: path.resolve(__dirname, 'scroll/index.html'), 'scroll-anchor': path.resolve(__dirname, 'scroll-anchor/index.html'), chat: path.resolve(__dirname, 'chat/index.html'), + 'chat-resize': path.resolve(__dirname, 'chat-resize/index.html'), 'measure-element': path.resolve( __dirname, 'measure-element/index.html', diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index c30049e92..55c276edf 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -1015,28 +1015,37 @@ export class Virtualizer< } } - // Re-issue a compensation write the browser clamped because the sizer - // had not grown yet (#1258). By now the consumer has committed the new - // total size, so there may be room. Runs before the clamped read-back - // in a synchronous (flushSync) render and after it otherwise; both - // paths leave `_clampedAdjustment` set, so the timing does not matter. + // The consumer has committed the new total size by now, so a clamped + // compensation write may have room (#1258). + this._retryClampedAdjustment() + } + + // Re-issue a compensation write the browser clamped because the sizer had + // not grown yet (#1258, #1266). Called after `notify` in `resizeItem`, + // which covers consumers that size the container synchronously inside + // `onChange` (direct DOM updates, flushSync renders — where no re-render + // may follow at all), and from `_willUpdate` for consumers that size it + // during an asynchronous render. Both the clamped read-back and the + // absence of one leave `_clampedAdjustment` set, so timing does not matter. + private _retryClampedAdjustment = () => { if ( - this._clampedAdjustment !== null && - this.scrollElement && - this.options.enabled + this._clampedAdjustment === null || + !this.scrollElement || + !this.options.enabled ) { - const { target, maxAtWrite } = this._clampedAdjustment - const max = this.getMaxScrollOffset() - if (max > maxAtWrite + 0.5) { - // Still short (the sizer grew only partially): stay pending against - // the new max so the next commit retries. - this._clampedAdjustment = - target > max + 0.5 ? { target, maxAtWrite: max } : null - this._scrollToOffset(target, { - adjustments: undefined, - behavior: undefined, - }) - } + return + } + const { target, maxAtWrite } = this._clampedAdjustment + const max = this.getMaxScrollOffset() + if (max > maxAtWrite + 0.5) { + // Still short (the sizer grew only partially): stay pending against + // the new max so the next opportunity retries. + this._clampedAdjustment = + target > max + 0.5 ? { target, maxAtWrite: max } : null + this._scrollToOffset(target, { + adjustments: undefined, + behavior: undefined, + }) } } @@ -1704,6 +1713,11 @@ export class Virtualizer< // land in one paint. When nothing moved (or the write was deferred on // iOS), keep the cheaper async notify. this.notify(adjustedSync) + // A consumer that grows the sizer synchronously inside `onChange` + // (direct DOM updates) may never re-render when the range is + // unchanged, so retry a clamped write here rather than only in + // `_willUpdate` (#1266). + this._retryClampedAdjustment() } } diff --git a/packages/virtual-core/tests/index.test.ts b/packages/virtual-core/tests/index.test.ts index 00417bb02..b9fc6996a 100644 --- a/packages/virtual-core/tests/index.test.ts +++ b/packages/virtual-core/tests/index.test.ts @@ -3854,3 +3854,82 @@ test('#1258: an unclamped compensation write is not re-issued when the sizer lat expect(scrollToFn).toHaveBeenCalledTimes(1) }) + +test('#1266: a consumer that grows the sizer synchronously in onChange gets the clamped write re-issued without a re-render', () => { + // Direct DOM updates: the adapter sets the container height inside + // onChange, and with an unchanged range nothing re-renders afterwards. + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, scrollElement, scrollToFn } = createChatVirtualizer({ + messages, + offset: 130, + paddingEnd: 80, + threshold: 0, + }) + virtualizer.options.onChange = (instance) => { + ;(scrollElement as any).scrollHeight = instance.getTotalSize() + } + + virtualizer.resizeItem(4, 120) + + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[0]![1].adjustments).toBe(70) + expect(scrollToFn.mock.calls[1]![0]).toBe(200) + expect(virtualizer['_clampedAdjustment']).toBeNull() +}) + +test('#1258: both retry sites firing in one sync pass write exactly once', () => { + // flushSync consumer: onChange grows the sizer AND runs the layout effect + // (_willUpdate) synchronously inside notify. The retry after notify must then + // find nothing pending — no double write. + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, scrollElement, scrollToFn } = createChatVirtualizer({ + messages, + offset: 130, + paddingEnd: 80, + threshold: 0, + }) + virtualizer.options.onChange = (instance) => { + ;(scrollElement as any).scrollHeight = instance.getTotalSize() + instance._willUpdate() + } + + virtualizer.resizeItem(4, 120) + + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(200) + expect(virtualizer['_clampedAdjustment']).toBeNull() +}) + +test('#1258: a partially grown sizer re-issues and stays pending until the target fits', () => { + const { virtualizer, scrollElement, scrollToFn, emitScroll } = + clampedGrowthSetup() + emitScroll(130) + + // Sizer grew only to 360 → max 160, still short of the 200 target. + ;(scrollElement as any).scrollHeight = 360 + virtualizer._willUpdate() + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(200) + expect(virtualizer['_clampedAdjustment']).toEqual({ + target: 200, + maxAtWrite: 160, + }) + + // The browser clamps that write to the new max; its read-back keeps it pending. + emitScroll(160) + expect(virtualizer['_clampedAdjustment']).not.toBeNull() + ;(scrollElement as any).scrollHeight = 400 + virtualizer._willUpdate() + expect(scrollToFn).toHaveBeenCalledTimes(3) + expect(scrollToFn.mock.calls[2]![0]).toBe(200) + expect(virtualizer['_clampedAdjustment']).toBeNull() +}) + +test('#1258: cleanup drops a pending clamped write', () => { + const { virtualizer } = clampedGrowthSetup() + expect(virtualizer['_clampedAdjustment']).not.toBeNull() + + virtualizer['cleanup']() + + expect(virtualizer['_clampedAdjustment']).toBeNull() +})