fix(diff): keep combined diff position stable across worktree switches (#30950)

A hidden worktree is display:none, so ResizeObserver reported every mounted
diff row as 0px and the virtualizer cached that. Rows above the viewport
collapsed, and the unchanged scrollTop showed files two rows further down
after each switch back. Keep the last real row size while the scroller has
no layout box, and skip Monaco height reports from a hidden (0-width) editor.
This commit is contained in:
Neil
2026-10-19 21:13:46 -07:00
committed by GitHub
parent 81b8bfb890
commit 0e7cefbb72
4 changed files with 176 additions and 0 deletions
@@ -18,6 +18,7 @@ import { useDiffSectionFallbackCleanup } from './useDiffSectionFallbackCleanup'
import { submitDiffSectionComment } from './diff-section-comment-submit'
import type { DiffSectionItemProps } from './diff-section-item-props'
import { useDiffSectionModelLifecycle } from './use-diff-section-model-lifecycle'
import { hasLayoutBox } from '@/hooks/virtualized-hidden-row-measurement'
export function DiffSectionItem({
section,
@@ -177,6 +178,10 @@ export function DiffSectionItem({
let diffLayoutReady = false
let pendingHeightFrame: number | null = null
const updateHeight = (): void => {
// Why: a hidden (0-width) side-by-side editor falls back to inline and reports that taller height.
if (!hasLayoutBox(editor.getContainerDomNode())) {
return
}
const contentHeight = editor.getModifiedEditor().getContentHeight()
setSectionHeights((prev) => {
if (prev[index] === contentHeight) {
@@ -2,6 +2,7 @@ import { useLayoutEffect } from 'react'
import type React from 'react'
import { elementScroll, useVirtualizer, type Virtualizer } from '@tanstack/react-virtual'
import type { ProgrammaticScrollMarks } from '@/hooks/programmatic-scroll-marks'
import { measureVirtualRowRetainingHiddenSize } from '@/hooks/virtualized-hidden-row-measurement'
import type { DiffSection } from '../../diff-section-types'
import { getDiffSectionRowEstimatedHeight } from '../../diff-section-layout'
import { getCombinedDiffRenderRange } from './combined-diff-render-range'
@@ -37,6 +38,8 @@ export function useCombinedDiffVirtualizer({
count: sections.length,
getScrollElement: () => scrollContainerRef.current,
estimateSize,
// Why: a hidden worktree reports every mounted row as 0px; keep the visible sizes so the anchor survives re-show.
measureElement: measureVirtualRowRetainingHiddenSize,
// Offscreen Monaco sections render their full height; bound that work by pixels, too.
rangeExtractor: (range) =>
getCombinedDiffRenderRange(
@@ -0,0 +1,137 @@
// @vitest-environment happy-dom
import {
measureElement,
observeElementOffset,
observeElementRect,
Virtualizer
} from '@tanstack/react-virtual'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { measureVirtualRowRetainingHiddenSize } from './virtualized-hidden-row-measurement'
const ROW_COUNT = 20
const MEASURED_ROW_HEIGHT = 1162
const UNMEASURED_ESTIMATE = 400
const SCROLL_TOP = 5250
type RowMeasurer = (
element: Element,
entry: ResizeObserverEntry | undefined,
instance: Virtualizer<HTMLDivElement, Element>
) => number
function sizeEntry(target: Element, size: number): ResizeObserverEntry {
const box = [{ blockSize: size, inlineSize: size }]
return {
target,
borderBoxSize: box,
contentBoxSize: box,
devicePixelContentBoxSize: box,
contentRect: new DOMRect(0, 0, size, size)
}
}
function setHidden(element: HTMLElement): void {
// Why: happy-dom always reports one client rect; a display:none subtree reports none.
const empty = element.getClientRects()
Array.prototype.splice.call(empty, 0, empty.length)
vi.spyOn(element, 'getClientRects').mockReturnValue(empty)
}
function createViewport(measure: RowMeasurer): {
rowElement: (index: number) => HTMLElement
scrollElement: HTMLDivElement
virtualizer: Virtualizer<HTMLDivElement, Element>
} {
const scrollElement = document.createElement('div')
document.body.appendChild(scrollElement)
const virtualizer = new Virtualizer<HTMLDivElement, Element>({
count: ROW_COUNT,
estimateSize: () => UNMEASURED_ESTIMATE,
getScrollElement: () => scrollElement,
initialOffset: SCROLL_TOP,
measureElement: measure,
observeElementOffset,
observeElementRect,
// Why: the browser keeps a hidden scroller's scrollTop; adjustments must not move it here either.
scrollToFn: () => {}
})
virtualizer._willUpdate()
const rowElement = (index: number): HTMLElement => {
const row = document.createElement('div')
row.setAttribute('data-index', String(index))
scrollElement.appendChild(row)
return row
}
// The user scrolled through rows 0-6, so each has a real measured height.
for (let index = 0; index <= 6; index++) {
const row = rowElement(index)
virtualizer.resizeItem(index, measure(row, sizeEntry(row, MEASURED_ROW_HEIGHT), virtualizer))
}
return { rowElement, scrollElement, virtualizer }
}
/** Hides the scroller, delivers the 0px ResizeObserver reports for the mounted rows, and re-shows it. */
function runHideCycle(measure: RowMeasurer): { topKey: unknown; totalSize: number } {
const { rowElement, scrollElement, virtualizer } = createViewport(measure)
setHidden(scrollElement)
for (const index of [4, 5]) {
const row = rowElement(index)
virtualizer.resizeItem(index, measure(row, sizeEntry(row, 0), virtualizer))
}
vi.restoreAllMocks()
return {
topKey: virtualizer.getVirtualItemForOffset(SCROLL_TOP)?.key,
totalSize: virtualizer.getTotalSize()
}
}
afterEach(() => {
vi.restoreAllMocks()
document.body.replaceChildren()
})
describe('measureVirtualRowRetainingHiddenSize', () => {
const visibleState = (): { topKey: unknown; totalSize: number } => {
const { virtualizer } = createViewport(measureVirtualRowRetainingHiddenSize)
return {
topKey: virtualizer.getVirtualItemForOffset(SCROLL_TOP)?.key,
totalSize: virtualizer.getTotalSize()
}
}
it('reproduces the drift with the default TanStack measurement', () => {
const before = visibleState()
const after = runHideCycle(measureElement)
expect(before.topKey).toBe(4)
// Rows above the fold collapsed to 0px, so the same scrollTop now lands two files later.
expect(after.topKey).toBe(6)
expect(after.totalSize).toBe(before.totalSize - 2 * MEASURED_ROW_HEIGHT)
})
it('keeps the same row under the same scrollTop across a hide/re-show', () => {
const before = visibleState()
const after = runHideCycle(measureVirtualRowRetainingHiddenSize)
expect(after).toEqual(before)
})
it('measures normally while the scroller is laid out', () => {
const { rowElement, virtualizer } = createViewport(measureVirtualRowRetainingHiddenSize)
const row = rowElement(4)
expect(measureVirtualRowRetainingHiddenSize(row, sizeEntry(row, 560), virtualizer)).toBe(560)
})
it('falls back to the estimate for a row first seen while hidden', () => {
const { rowElement, scrollElement, virtualizer } = createViewport(
measureVirtualRowRetainingHiddenSize
)
setHidden(scrollElement)
const row = rowElement(12)
expect(measureVirtualRowRetainingHiddenSize(row, sizeEntry(row, 0), virtualizer)).toBe(
UNMEASURED_ESTIMATE
)
})
})
@@ -0,0 +1,31 @@
import { measureElement, type Virtualizer } from '@tanstack/react-virtual'
/** False under a `display: none` ancestor, where every size reads as 0. */
export function hasLayoutBox(element: Element): boolean {
return element.getClientRects().length > 0
}
/**
* TanStack `measureElement` that keeps each row's last size while the scroll
* element has no layout box (a `display: none` ancestor, e.g. a hidden worktree).
*
* Hidden rows report 0px through ResizeObserver. Caching that collapses rows
* above the viewport, so the unchanged scrollTop shows later content on re-show.
* Unlike `useCachedMeasurements`, this reads the DOM, so it needs no visibility
* prop and covers every hiding ancestor.
*/
export function measureVirtualRowRetainingHiddenSize<TScrollElement extends Element>(
element: Element,
entry: ResizeObserverEntry | undefined,
instance: Virtualizer<TScrollElement, Element>
): number {
const scrollElement = instance.scrollElement
if (scrollElement && !hasLayoutBox(scrollElement)) {
const index = instance.indexFromElement(element)
return (
instance.itemSizeCache.get(instance.options.getItemKey(index)) ??
instance.options.estimateSize(index)
)
}
return measureElement(element, entry, instance)
}