diff --git a/.changeset/tidy-points-heal.md b/.changeset/tidy-points-heal.md new file mode 100644 index 000000000..7f6f63bfa --- /dev/null +++ b/.changeset/tidy-points-heal.md @@ -0,0 +1,6 @@ +--- +'@tanstack/virtual-core': patch +'@tanstack/marko-virtual': patch +--- + +Retry a clamped end-anchor sync after the scrollable content grows during a prepend. diff --git a/packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts b/packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts index 824449dab..bf870e62f 100644 --- a/packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts +++ b/packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts @@ -193,22 +193,110 @@ test('scrolling near the top auto-loads older history', async ({ page }) => { .toBeGreaterThan(heightBefore) }) -// FIXME(#1267): deterministic on CI. The click lands before the 180ms auto history -// load fires, the prepend then resolves after the jump, and the adapter's anchor -// write is clamped against the not-yet-grown sizer — the view strands one prepend -// (~870px) above the bottom. Re-enable once #1267 is fixed. -test.fixme('Latest returns to the bottom and status flips back to At latest', async ({ +test('Latest returns to the bottom and status flips back to At latest after an in-flight prepend', async ({ page, }) => { + // Hold the example's 180ms history timer so the test controls the ordering: + // click Latest first, then release the prepend. A wall-clock sleep alone can + // let the prepend finish before the click on a slow CI worker. + await page.addInitScript(() => { + const nativeSetTimeout = window.setTimeout.bind(window) + const testWindow = window as typeof window & { + __armHistoryPrependHold?: () => void + __historyPrependIsHeld?: () => boolean + __releaseHistoryPrepend?: () => void + } + let shouldHoldPrepend = false + let releasePrepend: (() => void) | null = null + let holdingTimer: number | null = null + + testWindow.__armHistoryPrependHold = () => { + shouldHoldPrepend = true + } + testWindow.__historyPrependIsHeld = () => releasePrepend !== null + testWindow.__releaseHistoryPrepend = () => { + if (!releasePrepend) { + throw new Error('No history prepend timer is being held') + } + if (holdingTimer !== null) window.clearTimeout(holdingTimer) + const callback = releasePrepend + releasePrepend = null + holdingTimer = null + callback() + } + + window.setTimeout = (( + handler: TimerHandler, + timeout?: number, + ...args: unknown[] + ) => { + if ( + shouldHoldPrepend && + timeout === 180 && + typeof handler === 'function' && + releasePrepend === null + ) { + shouldHoldPrepend = false + const callback = handler as (...callbackArgs: unknown[]) => void + releasePrepend = () => callback(...args) + holdingTimer = nativeSetTimeout(() => {}, 60_000) + return holdingTimer + } + return nativeSetTimeout(handler, timeout, ...args) + }) as typeof window.setTimeout + }) + await page.goto('/chat-pretext') await waitForPin(page) + await page.waitForTimeout(400) + // Arm the hook only immediately before scrolling to the top, so unrelated + // 180ms timers during app startup cannot be mistaken for this prepend. + await page.evaluate(() => { + ;( + window as Window & { + __armHistoryPrependHold?: () => void + } + ).__armHistoryPrependHold?.() + }) await page.locator('.messages').evaluate((el) => { el.scrollTop = 0 }) await expect(page.locator('[data-testid="status"]')).toHaveText( - /Reading history|Loading history/, + 'Loading history', ) + await expect + .poll(() => + page.evaluate( + () => + ( + window as Window & { + __historyPrependIsHeld?: () => boolean + } + ).__historyPrependIsHeld?.() ?? false, + ), + ) + .toBe(true) + + const heightBeforeClick = await page + .locator('.messages') + .evaluate((el) => el.scrollHeight) await page.locator('[data-testid="latest"]').click() + + // Release the prepend only after Latest has been clicked, guaranteeing the + // delayed content growth happens after the explicit end-scroll command. + await page.evaluate(() => { + ;( + window as Window & { + __releaseHistoryPrepend?: () => void + } + ).__releaseHistoryPrepend?.() + }) + + await expect + .poll(() => page.locator('.messages').evaluate((el) => el.scrollHeight), { + timeout: 3000, + }) + .toBeGreaterThan(heightBeforeClick) await waitForPin(page) await expect(page.locator('[data-testid="status"]')).toHaveText('At latest') await expect(page.locator('[data-key^="message-4"]').last()).toBeVisible() diff --git a/packages/marko-virtual/src/tags/virtualizer/index.marko b/packages/marko-virtual/src/tags/virtualizer/index.marko index 720ff6e54..9430a41f4 100644 --- a/packages/marko-virtual/src/tags/virtualizer/index.marko +++ b/packages/marko-virtual/src/tags/virtualizer/index.marko @@ -65,6 +65,7 @@ export interface Input { | null)/> void) | null)/> + onMount() { @@ -97,9 +98,23 @@ export interface Input { current.setOptions({ ...current.options, ...buildOptions(input, notify) }) current._willUpdate() notify() + + // Marko commits the new sizer after this lifecycle callback. Retry once + // on the next frame so the browser has the expanded scroll range. + if (retryFrame !== null && current.targetWindow) { + current.targetWindow.cancelAnimationFrame(retryFrame) + } + retryFrame = current.targetWindow?.requestAnimationFrame(() => { + retryFrame = null + current._retryClampedAdjustment() + }) ?? null } onDestroy() { + if (retryFrame !== null && v?.targetWindow) { + v.targetWindow.cancelAnimationFrame(retryFrame) + } + retryFrame = null cleanup?.() v = null cleanup = null diff --git a/packages/marko-virtual/src/tags/window-virtualizer/index.marko b/packages/marko-virtual/src/tags/window-virtualizer/index.marko index 995c02680..1946d2707 100644 --- a/packages/marko-virtual/src/tags/window-virtualizer/index.marko +++ b/packages/marko-virtual/src/tags/window-virtualizer/index.marko @@ -62,6 +62,7 @@ export interface Input { | null)/> void) | null)/> + onMount() { @@ -94,9 +95,23 @@ export interface Input { current.setOptions({ ...current.options, ...buildOptions(input, notify) }) current._willUpdate() notify() + + // Marko commits the new sizer after this lifecycle callback. Retry once + // on the next frame so the browser has the expanded scroll range. + if (retryFrame !== null && current.targetWindow) { + current.targetWindow.cancelAnimationFrame(retryFrame) + } + retryFrame = current.targetWindow?.requestAnimationFrame(() => { + retryFrame = null + current._retryClampedAdjustment() + }) ?? null } onDestroy() { + if (retryFrame !== null && v?.targetWindow) { + v.targetWindow.cancelAnimationFrame(retryFrame) + } + retryFrame = null cleanup?.() v = null cleanup = null diff --git a/packages/virtual-core/src/index.ts b/packages/virtual-core/src/index.ts index cb5011392..3a76cd559 100644 --- a/packages/virtual-core/src/index.ts +++ b/packages/virtual-core/src/index.ts @@ -1146,7 +1146,12 @@ export class Virtualizer< // A smooth scroll that has already landed (offset at its target, // reconcile not yet retired it) still gets the anchor sync. } else { - this._scrollToOffset(this.getScrollOffset(), { + // The consumer may not have committed the new sizer size yet. If the + // browser clamps this anchor sync to the old scroll max, keep the + // intended target so _willUpdate can retry after the sizer grows. + const target = this.getScrollOffset() + this._clampedAdjustment = this._clampedBy(target) + this._scrollToOffset(target, { adjustments: undefined, behavior: undefined, }) @@ -1168,7 +1173,9 @@ export class Virtualizer< // 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 = () => { + // Framework adapters may call this after their rendered sizer commits. + // The pending target is cleared by any newer explicit scroll command. + _retryClampedAdjustment = () => { if ( this._clampedAdjustment === null || !this.scrollElement || @@ -2180,6 +2187,11 @@ export class Virtualizer< behavior: ScrollBehavior, kind: 'absolute' | 'relative', ) { + // A new explicit scroll command takes ownership of the viewport. Do not + // let a queued prepend/follow anchor or a previously clamped adjustment + // retry later and pull the user back toward its stale target. + this.pendingScrollAnchor = null + this._clampedAdjustment = null if (kind === 'absolute') { this._iosDeferredAdjustment = 0 } diff --git a/packages/virtual-core/tests/index.test.ts b/packages/virtual-core/tests/index.test.ts index 290d810a3..522cf7a8d 100644 --- a/packages/virtual-core/tests/index.test.ts +++ b/packages/virtual-core/tests/index.test.ts @@ -3899,12 +3899,20 @@ function createChatVirtualizer({ virtualizer, scrollElement, scrollToFn, - setMessages(nextMessages: Array<{ id: string }>) { + setMessages( + nextMessages: Array<{ id: string }>, + { + growSizer = true, + update = true, + }: { growSizer?: boolean; update?: boolean } = {}, + ) { currentMessages = nextMessages virtualizer.setOptions(makeOptions()) - ;(scrollElement as any).scrollHeight = - nextMessages.length * itemSize + paddingEnd - virtualizer._willUpdate() + if (growSizer) { + ;(scrollElement as any).scrollHeight = + nextMessages.length * itemSize + paddingEnd + } + if (update) virtualizer._willUpdate() }, // Simulate the browser's scroll event after a write (or a gesture). emitScroll(nextOffset: number, isScrolling = false) { @@ -3914,6 +3922,98 @@ function createChatVirtualizer({ } } +test('#1267: end-anchor sync retries after the sizer grows following a prepend', () => { + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, scrollElement, scrollToFn, setMessages } = + createChatVirtualizer({ + messages, + offset: 50, + }) + + // The new anchor target is 150, but the old sizer still only allows 50. + // This simulates the adapter's layout effect running before the prepend's + // larger scrollable container has been committed. + setMessages([{ id: 'm--2' }, { id: 'm--1' }, ...messages], { + growSizer: false, + }) + + expect(scrollToFn).toHaveBeenCalledTimes(1) + expect(scrollToFn.mock.calls[0]![0]).toBe(150) + expect(virtualizer['_clampedAdjustment']).toEqual({ + target: 150, + maxAtWrite: 50, + }) + + // Once the consumer commits the larger sizer, the target can be reached. + ;(scrollElement as any).scrollHeight = 350 + virtualizer._willUpdate() + + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(150) + expect(virtualizer['_clampedAdjustment']).toBeNull() +}) + +test('#1267: a new explicit scroll cancels a pending clamped anchor retry', () => { + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, scrollElement, scrollToFn, setMessages } = + createChatVirtualizer({ + messages, + offset: 50, + }) + + setMessages([{ id: 'm--2' }, { id: 'm--1' }, ...messages], { + growSizer: false, + }) + expect(virtualizer['_clampedAdjustment']).not.toBeNull() + + // A newer explicit command must take ownership rather than be overwritten + // when the old prepend's sizer eventually grows. + virtualizer.scrollToOffset(20) + expect(virtualizer['_clampedAdjustment']).toBeNull() + expect(scrollToFn).toHaveBeenCalledTimes(2) + ;(scrollElement as any).scrollHeight = 350 + virtualizer._willUpdate() + + // No third write to the stale prepend target is allowed. + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(20) +}) + +test('#1267: an explicit scroll cancels a queued prepend anchor before update', () => { + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, scrollElement, scrollToFn, setMessages } = + createChatVirtualizer({ + messages, + offset: 50, + }) + + // Queue the prepend anchor but delay _willUpdate, which is where that + // anchor would otherwise be written to the DOM. + setMessages([{ id: 'm--2' }, { id: 'm--1' }, ...messages], { + growSizer: false, + update: false, + }) + expect(virtualizer['pendingScrollAnchor']).toEqual({ + kind: 'anchor', + delta: 100, + }) + + // A new explicit destination must supersede the not-yet-applied anchor. + virtualizer.scrollToOffset(20) + expect(virtualizer['pendingScrollAnchor']).toBeNull() + expect(scrollToFn).toHaveBeenCalledTimes(1) + expect(scrollToFn.mock.calls[0]![0]).toBe(20) + + virtualizer._willUpdate() + expect(scrollToFn).toHaveBeenCalledTimes(1) + + // Growing the sizer later must not resurrect the discarded anchor either. + ;(scrollElement as any).scrollHeight = 350 + virtualizer._willUpdate() + expect(scrollToFn).toHaveBeenCalledTimes(1) + expect(scrollToFn.mock.calls[0]![0]).toBe(20) +}) + test('anchorTo:end keeps visible content stable when older items are prepended', () => { const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) const { setMessages, scrollToFn } = createChatVirtualizer({