fix(diff): keep scroll restore armed through layout shifts (#10615)

* fix(diff): keep scroll restore armed through layout shifts

* test: add scroll-restore convergence and user-scroll disarm cases

Verify that a converging restore withstands layout shifts and continues
retrying, while unmarked user scroll disarms the restore attempt. Stabilize
marks and offset objects across renders to preserve the bookkeeping state
that guards restoration retries.
This commit is contained in:
Jinjing 2026-07-25 15:11:42 -07:00 committed by GitHub
parent e564603d54
commit 97175ed92b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 109 additions and 1 deletions

View File

@ -135,6 +135,113 @@ describe('useVirtualizedScrollAnchor with marks + restoreSignal', () => {
expect(virtualizer.scrollToIndex).not.toHaveBeenCalled()
})
it('keeps retrying a restore that is still converging when layout moves the viewport', async () => {
const { harness, useVirtualizedScrollAnchor } = await loadHook()
let rowTop = -3_000
const rowElement: FakeRowElement = {
getBoundingClientRect: () => ({
bottom: rowTop + 4_000,
height: 4_000,
top: rowTop
}),
isConnected: true,
key: 'row-1'
}
const { el } = createScrollElement({
rowElements: [rowElement],
scrollTop: 746
})
const anchorRef = { current: { key: 'row-1', offset: 3_358, scrollTop: 746 } }
const virtualizer = virtualizerWithRow1()
// Stable across rerenders, like the real caller: fresh objects would reset
// the marks and offset bookkeeping the first pass left behind.
const programmaticScrollMarks = createProgrammaticScrollMarks()
const scrollElementRef = { current: el }
const scrollOffsetRef = { current: 746 }
const render = (): void => {
harness.beginRender()
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
useVirtualizedScrollAnchor({
anchorRef,
getItemElementKey: (element: FakeRowElement) => element.key,
getRowKey: (row: string) => row,
itemElementSelector: '[data-row]',
programmaticScrollMarks,
recordAnchorOnScroll: false,
restoreSignal: 'signal-a',
rows: ['row-0', 'row-1'],
scrollElementRef,
scrollOffsetRef,
totalSize: 30_000,
virtualizer
} as never)
harness.effects[1]?.effect()
}
render()
const attemptsAfterFirstPass = el.querySelectorAll.mock.calls.length
expect(attemptsAfterFirstPass).toBeGreaterThan(0)
expect(el.scrollTop).toBe(1_104)
// Monaco grows content above while browser anchoring keeps the row pinned.
rowTop = -3_358
el.scrollTop += 196
render()
expect(el.querySelectorAll.mock.calls.length).toBeGreaterThan(attemptsAfterFirstPass)
const attemptsAfterConfirm = el.querySelectorAll.mock.calls.length
expect(anchorRef.current.scrollTop).toBe(1_300)
render()
expect(el.querySelectorAll.mock.calls.length).toBe(attemptsAfterConfirm)
})
it('lets an unmarked user scroll disarm a restore that is still converging', async () => {
const { harness, useVirtualizedScrollAnchor } = await loadHook()
const { el, emitScroll } = createScrollElement({
rowElements: [anchoredRowElement('row-1', -3_000)],
scrollTop: 746
})
const anchorRef = { current: { key: 'row-1', offset: 3_358, scrollTop: 746 } }
const programmaticScrollMarks = createProgrammaticScrollMarks()
const scrollElementRef = { current: el }
const scrollOffsetRef = { current: 746 }
const virtualizer = virtualizerWithRow1()
const render = (): void => {
harness.beginRender()
// oxlint-disable-next-line react-hooks/rules-of-hooks -- test harness mocks React's hook dispatcher directly.
useVirtualizedScrollAnchor({
anchorRef,
getItemElementKey: (element: FakeRowElement) => element.key,
getRowKey: (row: string) => row,
itemElementSelector: '[data-row]',
programmaticScrollMarks,
recordAnchorOnScroll: false,
restoreSignal: 'signal-a',
rows: ['row-0', 'row-1'],
scrollElementRef,
scrollOffsetRef,
totalSize: 30_000,
virtualizer
} as never)
}
render()
harness.effects[0]?.effect()
harness.effects[1]?.effect()
const attemptsAfterFirstPass = el.querySelectorAll.mock.calls.length
expect(el.scrollTop).toBe(1_104)
emitScroll(1_400)
render()
harness.effects[1]?.effect()
expect(el.scrollTop).toBe(1_400)
expect(el.querySelectorAll.mock.calls.length).toBe(attemptsAfterFirstPass)
})
it('lets the user position win when the viewport moved after the anchor was recorded', async () => {
const { harness, useVirtualizedScrollAnchor } = await loadHook()
const { el } = createScrollElement({

View File

@ -254,7 +254,8 @@ export function useVirtualizedScrollAnchor<
// adjustment; restoring here would fight concurrent user scrolling.
return
}
if (anchor.scrollTop !== undefined) {
// Why: pending restores own layout shifts; unmarked user scrolls disarm them in the listener.
if (anchor.scrollTop !== undefined && !pendingRestoreRef.current) {
const maxScrollTop = Math.max(0, el.scrollHeight - el.clientHeight)
const clampExplained =
anchor.scrollTop > maxScrollTop + 1 && el.scrollTop >= maxScrollTop - 2