fix(editor): extract diff first-change auto-scroll to a hook to unblock main (#21738)

2026-09-21 16:02:20 +00:00 · 2026-09-19 17:06:58 -07:00
parent b8f67a6266
commit b766f512ec
2 changed files with 93 additions and 62 deletions
@@ -21,6 +21,7 @@ import { diffEditorScrollbarOptions } from './diff-editor-scrollbar-options'
 import { LargeDiffFallback } from './LargeDiffFallback'
 import { getLargeDiffRenderLimit } from './large-diff-render-limit'
 import { useDiffViewerLargeDiffLifecycle } from './useDiffViewerLargeDiffLifecycle'
+import { useDiffViewerFirstChangeAutoScroll } from './useDiffViewerFirstChangeAutoScroll'
 import { getDiffViewerLargeDiffSaveAction } from './diff-viewer-large-diff-save-action'
 import type { DiffViewerProps } from './diff-viewer-props'
 import { buildDiffEditorWhitespaceOptions } from './diff-editor-whitespace-options'
@@ -156,68 +157,12 @@ export default function DiffViewer({
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [modifiedEditor, popover?.lineNumber])

-  // Why: center the first diff from a dedicated effect (not handleMount) so it runs after the decorator's view zones, which would otherwise shift content downward.
-  const didAutoScrollFirstDiffRef = useRef(false)
-  const didAutoScrollModelKeyRef = useRef(modelKey)
-  useEffect(() => {
-    if (didAutoScrollModelKeyRef.current !== modelKey) {
-      didAutoScrollModelKeyRef.current = modelKey
-      // Why: reset the per-modelKey one-shot here before the first-diff guard runs for the new file.
-      didAutoScrollFirstDiffRef.current = false
-    }
-    const diffEditor = diffEditorRef.current
-    if (!diffEditor || !modifiedEditor) {
-      return
-    }
-    if (didAutoScrollFirstDiffRef.current) {
-      return
-    }
-    if (diffViewStateCache.get(modelKey)) {
-      return
-    }
-    if (pendingScrollForThisViewer) {
-      // Why: decorator owns this scroll, so set the one-shot flag; else we'd re-run and overwrite it when pendingScroll flips back to null.
-      didAutoScrollFirstDiffRef.current = true
-      return
-    }
-    let rafId: number | null = null
-    const run = (): void => {
-      if (didAutoScrollFirstDiffRef.current) {
-        return
-      }
-      const changes = diffEditor.getLineChanges()
-      if (!changes || changes.length === 0) {
-        return
-      }
-      const line = Math.max(1, changes[0].modifiedStartLineNumber)
-      // Defer one frame so view zones are laid out before measuring; cancel any earlier rAF to avoid a redundant scroll.
-      if (rafId !== null) {
-        cancelAnimationFrame(rafId)
-      }
-      rafId = requestAnimationFrame(() => {
-        rafId = null
-        if (didAutoScrollFirstDiffRef.current || !modifiedEditor.getModel()) {
-          return
-        }
-        const top = modifiedEditor.getTopForLineNumber(line, true)
-        const editorHeight = modifiedEditor.getLayoutInfo().height
-        modifiedEditor.setPosition({ lineNumber: line, column: 1 })
-        modifiedEditor.setScrollTop(Math.max(0, top - editorHeight / 2))
-        didAutoScrollFirstDiffRef.current = true
-      })
-    }
-    // Run now if the diff is ready; otherwise onDidUpdateDiff fires once the computation lands.
-    if (diffEditor.getLineChanges()) {
-      run()
-    }
-    const sub = diffEditor.onDidUpdateDiff(() => run())
-    return () => {
-      sub.dispose()
-      if (rafId !== null) {
-        cancelAnimationFrame(rafId)
-      }
-    }
-  }, [modifiedEditor, modelKey, pendingScrollForThisViewer])
+  useDiffViewerFirstChangeAutoScroll({
+    diffEditorRef,
+    modifiedEditor,
+    modelKey,
+    pendingScrollCommentId: pendingScrollForThisViewer
+  })

  const handleEnterLargeDiffFallback = useCallback(() => {
    // Why: on fallback transition, drop stale Monaco refs so decorators/save handlers don't talk to disposed UI.
@@ -0,0 +1,86 @@
+import { useEffect, useRef } from 'react'
+import type { RefObject } from 'react'
+import type { editor } from 'monaco-editor'
+import { diffViewStateCache } from '@/lib/scroll-cache'
+
+type DiffViewerFirstChangeAutoScrollInput = {
+  diffEditorRef: RefObject<editor.IStandaloneDiffEditor | null>
+  modifiedEditor: editor.ICodeEditor | null
+  modelKey: string
+  pendingScrollCommentId: string | null
+}
+
+/**
+ * Centers the viewport on a diff's first change, once per modelKey.
+ *
+ * Why: lives outside handleMount so it sequences after the comment decorator's
+ * view zones, which would otherwise shift the measured content downward.
+ */
+export function useDiffViewerFirstChangeAutoScroll({
+  diffEditorRef,
+  modifiedEditor,
+  modelKey,
+  pendingScrollCommentId
+}: DiffViewerFirstChangeAutoScrollInput): void {
+  const didAutoScrollFirstDiffRef = useRef(false)
+  const didAutoScrollModelKeyRef = useRef(modelKey)
+  useEffect(() => {
+    if (didAutoScrollModelKeyRef.current !== modelKey) {
+      didAutoScrollModelKeyRef.current = modelKey
+      // Why: reset the per-modelKey one-shot here before the first-diff guard runs for the new file.
+      didAutoScrollFirstDiffRef.current = false
+    }
+    const diffEditor = diffEditorRef.current
+    if (!diffEditor || !modifiedEditor) {
+      return
+    }
+    if (didAutoScrollFirstDiffRef.current) {
+      return
+    }
+    if (diffViewStateCache.get(modelKey)) {
+      return
+    }
+    if (pendingScrollCommentId) {
+      // Why: decorator owns this scroll, so set the one-shot flag; else we'd re-run and overwrite it when pendingScroll flips back to null.
+      didAutoScrollFirstDiffRef.current = true
+      return
+    }
+    let rafId: number | null = null
+    const run = (): void => {
+      if (didAutoScrollFirstDiffRef.current) {
+        return
+      }
+      const changes = diffEditor.getLineChanges()
+      if (!changes || changes.length === 0) {
+        return
+      }
+      const line = Math.max(1, changes[0].modifiedStartLineNumber)
+      // Defer one frame so view zones are laid out before measuring; cancel any earlier rAF to avoid a redundant scroll.
+      if (rafId !== null) {
+        cancelAnimationFrame(rafId)
+      }
+      rafId = requestAnimationFrame(() => {
+        rafId = null
+        if (didAutoScrollFirstDiffRef.current || !modifiedEditor.getModel()) {
+          return
+        }
+        const top = modifiedEditor.getTopForLineNumber(line, true)
+        const editorHeight = modifiedEditor.getLayoutInfo().height
+        modifiedEditor.setPosition({ lineNumber: line, column: 1 })
+        modifiedEditor.setScrollTop(Math.max(0, top - editorHeight / 2))
+        didAutoScrollFirstDiffRef.current = true
+      })
+    }
+    // Run now if the diff is ready; otherwise onDidUpdateDiff fires once the computation lands.
+    if (diffEditor.getLineChanges()) {
+      run()
+    }
+    const sub = diffEditor.onDidUpdateDiff(() => run())
+    return () => {
+      sub.dispose()
+      if (rafId !== null) {
+        cancelAnimationFrame(rafId)
+      }
+    }
+  }, [diffEditorRef, modifiedEditor, modelKey, pendingScrollCommentId])
+}