diff --git a/.changeset/marko-prepend-anchor-retry.md b/.changeset/marko-prepend-anchor-retry.md new file mode 100644 index 000000000..8a4b0b10b --- /dev/null +++ b/.changeset/marko-prepend-anchor-retry.md @@ -0,0 +1,6 @@ +--- +'@tanstack/virtual-core': patch +'@tanstack/marko-virtual': patch +--- + +Keep an end-anchored Marko list at the bottom when a prepend lands right after `scrollToEnd()`. The prepend's anchor write is made before Marko commits the grown sizer, so the browser clamped it to the old end; core now records that clamp and the Marko tags retry the write once the new total size is rendered. 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..54a82b7ab 100644 --- a/packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts +++ b/packages/marko-virtual/e2e/app/e2e/chat-pretext.spec.ts @@ -193,11 +193,7 @@ 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', async ({ page, }) => { await page.goto('/chat-pretext') @@ -214,6 +210,41 @@ test.fixme('Latest returns to the bottom and status flips back to At latest', as await expect(page.locator('[data-key^="message-4"]').last()).toBeVisible() }) +// #1267: Latest clicked while a history load is in flight, so the prepend lands +// after the jump. Its anchor write is made before Marko commits the grown sizer and +// is clamped to the old max; unless it is retried once the sizer has grown, the view +// strands one prepend above the bottom with no scroll event to recover it. The +// scroll to the top and the click run in the page, well inside the 180ms history +// timer, so the prepend always resolves after the click. +test('Latest stays at the bottom when an in-flight history load lands after it', async ({ + page, +}) => { + await page.goto('/chat-pretext') + await waitForPin(page) + await page.waitForTimeout(400) // the near-top auto history load arms after 250ms + const heightBefore = await page + .locator('.messages') + .evaluate((el) => el.scrollHeight) + const statusAtClick = await page.locator('.messages').evaluate(async (el) => { + const frame = () => new Promise((resolve) => requestAnimationFrame(resolve)) + el.scrollTop = 0 + await frame() + await frame() + await new Promise((resolve) => setTimeout(resolve)) + const status = document.querySelector('[data-testid="status"]')!.textContent + document.querySelector('[data-testid="latest"]')!.click() + return status?.trim() + }) + expect(statusAtClick).toBe('Loading history') + await expect + .poll(() => page.locator('.messages').evaluate((el) => el.scrollHeight), { + timeout: 3000, + }) + .toBeGreaterThan(heightBefore) + await waitForPin(page) + await expect(page.locator('[data-testid="status"]')).toHaveText('At latest') +}) + test('a reply streamed from the server grows progressively and stays pinned', async ({ page, }) => { diff --git a/packages/marko-virtual/e2e/app/e2e/chat.spec.ts b/packages/marko-virtual/e2e/app/e2e/chat.spec.ts index 56af4c8cb..69cba609a 100644 --- a/packages/marko-virtual/e2e/app/e2e/chat.spec.ts +++ b/packages/marko-virtual/e2e/app/e2e/chat.spec.ts @@ -197,12 +197,7 @@ test('scrolling near the top auto-loads older history', async ({ page }) => { .toBeGreaterThan(heightBefore) }) -// FIXME(#1267): same prepend race as the chat-pretext sibling (#1268), flaky on CI. -// Latest is clicked while the near-top auto history load is in flight; the prepend -// resolves after the jump and the adapter's anchor write is clamped against the -// not-yet-grown sizer, so the status stays "Reading history". 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', async ({ page, }) => { await page.goto('/chat') @@ -219,6 +214,41 @@ test.fixme('Latest returns to the bottom and status flips back to At latest', as await expect(page.locator('[data-key^="message-4"]').last()).toBeVisible() }) +// #1267: Latest clicked while a history load is in flight, so the prepend lands +// after the jump. Its anchor write is made before Marko commits the grown sizer and +// is clamped to the old max; unless it is retried once the sizer has grown, the view +// strands one prepend above the bottom with no scroll event to recover it. The +// scroll to the top and the click run in the page, well inside the 180ms history +// timer, so the prepend always resolves after the click. +test('Latest stays at the bottom when an in-flight history load lands after it', async ({ + page, +}) => { + await page.goto('/chat') + await waitForPin(page) + await page.waitForTimeout(400) // the near-top auto history load arms after 250ms + const heightBefore = await page + .locator('.messages') + .evaluate((el) => el.scrollHeight) + const statusAtClick = await page.locator('.messages').evaluate(async (el) => { + const frame = () => new Promise((resolve) => requestAnimationFrame(resolve)) + el.scrollTop = 0 + await frame() + await frame() + await new Promise((resolve) => setTimeout(resolve)) + const status = document.querySelector('[data-testid="status"]')!.textContent + document.querySelector('[data-testid="latest"]')!.click() + return status?.trim() + }) + expect(statusAtClick).toBe('Loading history') + await expect + .poll(() => page.locator('.messages').evaluate((el) => el.scrollHeight), { + timeout: 3000, + }) + .toBeGreaterThan(heightBefore) + await waitForPin(page) + await expect(page.locator('[data-testid="status"]')).toHaveText('At latest') +}) + test('a reply streamed from the server grows progressively and stays pinned', async ({ page, }) => { diff --git a/packages/marko-virtual/src/tags/virtualizer/index.marko b/packages/marko-virtual/src/tags/virtualizer/index.marko index 720ff6e54..5fec216eb 100644 --- a/packages/marko-virtual/src/tags/virtualizer/index.marko +++ b/packages/marko-virtual/src/tags/virtualizer/index.marko @@ -106,6 +106,15 @@ export interface Input { } /> +// `onUpdate` runs before the new `size` reaches the consumer's sizer: Marko commits +// it in a later batch. A prepend's anchor write made in between is clamped to the old +// scroll max, so retry it here, after every render that committed a new total size +// (#1267). A no-op when nothing was clamped. + + +// `onUpdate` runs before the new `size` reaches the consumer's sizer: Marko commits +// it in a later batch. A prepend's anchor write made in between is clamped to the old +// scroll max, so retry it here, after every render that committed a new total size +// (#1267). A no-op when nothing was clamped. + + { + _retryClampedAdjustment = () => { if ( this._clampedAdjustment === null || !this.scrollElement || diff --git a/packages/virtual-core/tests/anchor-end.test.ts b/packages/virtual-core/tests/anchor-end.test.ts index 0a95bfb73..f3df27579 100644 --- a/packages/virtual-core/tests/anchor-end.test.ts +++ b/packages/virtual-core/tests/anchor-end.test.ts @@ -94,17 +94,27 @@ function createChatVirtualizer({ virtualizer['getMeasurements']() scrollToFn.mockClear() + const growSizer = () => { + ;(scrollElement as any).scrollHeight = + scrollMargin + currentMessages.length * itemSize + paddingEnd + } + return { virtualizer, scrollElement, scrollToFn, - setMessages(nextMessages: Array<{ id: string }>) { + // `commitSizer: false` models an adapter whose framework commits the + // grown sizer only after `_willUpdate` (Marko); `growSizer()` does it. + setMessages( + nextMessages: Array<{ id: string }>, + { commitSizer = true }: { commitSizer?: boolean } = {}, + ) { currentMessages = nextMessages virtualizer.setOptions(makeOptions()) - ;(scrollElement as any).scrollHeight = - scrollMargin + nextMessages.length * itemSize + paddingEnd + if (commitSizer) growSizer() virtualizer._willUpdate() }, + growSizer, // Simulate the browser's scroll event after a write (or a gesture). emitScroll(nextOffset: number, isScrolling = false) { scrollElement.scrollTop = nextOffset @@ -700,3 +710,77 @@ test('#1258: cleanup drops a pending clamped write', () => { expect(virtualizer['_clampedAdjustment']).toBeNull() }) + +// ─── #1267: prepend anchor sync clamped by a sizer committed after it ─────── +// 5 × 50px = 250px sizer, 200px viewport, pinned to the bottom at 50. Two +// rows prepended above move the anchor to 150, but an adapter that commits +// the 350px sizer only after `_willUpdate` (Marko) has the write clamped to +// the old max, a whole prepend short of the anchor. + +function clampedPrependSetup() { + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const setup = createChatVirtualizer({ messages, offset: 50 }) + setup.setMessages([{ id: 'm--2' }, { id: 'm--1' }, ...messages], { + commitSizer: false, + }) + expect(setup.scrollToFn).toHaveBeenCalledTimes(1) + expect(setup.scrollToFn.mock.calls[0]![0]).toBe(150) + return setup +} + +test('#1267: a clamped prepend anchor sync is re-issued once the sizer commits after _willUpdate', () => { + const { virtualizer, scrollToFn, emitScroll, growSizer } = + clampedPrependSetup() + expect(virtualizer['_clampedAdjustment']).toEqual({ + target: 150, + maxAtWrite: 50, + }) + + // Browser read-back of the clamped write: it landed on the old max. + emitScroll(50) + // The framework commits the grown sizer, then the adapter retries. + growSizer() + virtualizer._retryClampedAdjustment() + + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(150) + expect(virtualizer['_clampedAdjustment']).toBeNull() +}) + +test('#1267: a retry before the sizer commits keeps the clamped prepend anchor sync pending', () => { + const { virtualizer, scrollToFn, growSizer } = clampedPrependSetup() + + virtualizer._retryClampedAdjustment() + expect(scrollToFn).toHaveBeenCalledTimes(1) + expect(virtualizer['_clampedAdjustment']).not.toBeNull() + + growSizer() + virtualizer._retryClampedAdjustment() + expect(scrollToFn).toHaveBeenCalledTimes(2) + expect(scrollToFn.mock.calls[1]![0]).toBe(150) +}) + +test('#1267: an explicit scroll after a clamped prepend anchor sync drops the retry', () => { + const { virtualizer, scrollToFn, growSizer } = clampedPrependSetup() + + virtualizer.scrollToOffset(0) + expect(scrollToFn).toHaveBeenCalledTimes(2) + growSizer() + virtualizer._retryClampedAdjustment() + + expect(scrollToFn).toHaveBeenCalledTimes(2) +}) + +test('#1267: a prepend anchor sync within the committed sizer records no retry', () => { + const messages = Array.from({ length: 5 }, (_, i) => ({ id: `m-${i}` })) + const { virtualizer, setMessages, scrollToFn } = createChatVirtualizer({ + messages, + offset: 50, + }) + + setMessages([{ id: 'm--2' }, { id: 'm--1' }, ...messages]) + + expect(scrollToFn).toHaveBeenCalledTimes(1) + expect(scrollToFn.mock.calls[0]![0]).toBe(150) + expect(virtualizer['_clampedAdjustment']).toBeNull() +})