diff --git a/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx b/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx index 9b97869df8..387e81eb95 100644 --- a/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx +++ b/site/src/pages/AgentsPage/components/DiffViewer/DiffViewer.tsx @@ -4,7 +4,8 @@ import type { FileDiffMetadata, SelectedLineRange, } from "@pierre/diffs"; -import { FileDiff } from "@pierre/diffs/react"; +import { Virtualizer } from "@pierre/diffs"; +import { FileDiff, VirtualizerContext } from "@pierre/diffs/react"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { DIFFS_FONT_STYLE, @@ -19,6 +20,7 @@ import { type FC, memo, type ReactNode, + useCallback, useEffect, useRef, useState, @@ -333,6 +335,77 @@ const FileTreeNodeView: FC<{ ); }; +// ------------------------------------------------------------------- +// Virtualized scroll container +// ------------------------------------------------------------------- + +/** + * Wraps the diff list in a Radix ScrollArea and wires up the + * @pierre/diffs Virtualizer. Extracted into its own component so + * that the useCallback for ref-callback stability lives here + * instead of in DiffViewer. This lets the React Compiler skip + * only this small wrapper (where it can't preserve useCallback) + * while still optimizing the much larger parent. + * + * The ref callback is placed on a content div *inside* the + * ScrollArea, then walks up with closest() to the Radix viewport. + * React fires children's refs bottom-up during commit, so every + * VirtualizedFileDiff instance has already connected to the + * virtualizer by the time this ref fires and calls setup(). + */ +const DiffScrollContainer: FC<{ + children: ReactNode; + className?: string; + diffViewportRef: React.RefObject; + onViewportHeight: (height: number) => void; +}> = ({ children, className, diffViewportRef, onViewportHeight }) => { + const [virtualizer] = useState(() => new Virtualizer()); + + // useCallback is required for correctness: in React 19 an + // unstable ref callback triggers old-cleanup → new-callback + // on every render, which calls virtualizer.cleanUp() and + // wipes the observer map. The compiler can't preserve this + // useCallback, but that only causes it to skip this small + // wrapper — not the entire DiffViewer. + const contentRef = useCallback( + (node: HTMLDivElement | null) => { + const viewport = node?.closest( + "[data-radix-scroll-area-viewport]", + ); + if (!viewport) return; + + diffViewportRef.current = viewport; + virtualizer.setup(viewport); + + onViewportHeight(viewport.clientHeight); + const ro = new ResizeObserver(([entry]) => { + onViewportHeight(entry.contentRect.height); + }); + ro.observe(viewport); + return () => { + ro.disconnect(); + virtualizer.cleanUp(); + diffViewportRef.current = null; + }; + }, + [virtualizer, diffViewportRef, onViewportHeight], + ); + + return ( + + +
+ {children} +
+
+
+ ); +}; + // ------------------------------------------------------------------- // Lazy file diff wrapper // ------------------------------------------------------------------- @@ -678,34 +751,7 @@ export const DiffViewer: FC = ({ } }, [scrollToFile, onScrollToFileComplete]); - // --------------------------------------------------------------- - // Viewport height for the last-file min-height trick: setting - // min-height on the last file wrapper lets CSS handle the - // "be at least viewport-tall" logic, removing the need for a - // separate spacer div and a second ResizeObserver. Uses a ref - // callback (same pattern as containerRef) so the measurement - // lands during commit — before useEffect-based scroll logic. - // --------------------------------------------------------------- const [viewportHeight, setViewportHeight] = useState(0); - const [scrollAreaEl, setScrollAreaEl] = useState(null); - - useEffect(() => { - const vp = scrollAreaEl?.querySelector( - "[data-radix-scroll-area-viewport]", - ); - diffViewportRef.current = vp ?? null; - if (!vp) return; - setViewportHeight(vp.clientHeight); - const ro = new ResizeObserver(([entry]) => { - setViewportHeight(entry.contentRect.height); - }); - ro.observe(vp); - return () => { - ro.disconnect(); - diffViewportRef.current = null; - }; - }, [scrollAreaEl]); - // --------------------------------------------------------------- // Loading state // --------------------------------------------------------------- @@ -772,47 +818,41 @@ export const DiffViewer: FC = ({ )} - {/* Diff list */} - -
- {sortedFiles.map((fileDiff, i) => { - const isLast = i === sortedFiles.length - 1; - return ( -
setFileRef(fileDiff.name, el)} - style={isLast ? { minHeight: viewportHeight } : undefined} - > - - {isLast && ( -
- {`${sortedFiles.length} ${sortedFiles.length === 1 ? "file" : "files"} changed`} -
- )} -
- ); - })} -
-
+ {sortedFiles.map((fileDiff, i) => { + const isLast = i === sortedFiles.length - 1; + return ( +
setFileRef(fileDiff.name, el)} + style={isLast ? { minHeight: viewportHeight } : undefined} + > + + {isLast && ( +
+ {`${sortedFiles.length} ${sortedFiles.length === 1 ? "file" : "files"} changed`} +
+ )} +
+ ); + })} + )} diff --git a/site/src/testHelpers/pierreDiffsReactMock.tsx b/site/src/testHelpers/pierreDiffsReactMock.tsx index 3021e770ab..d0c8a0234c 100644 --- a/site/src/testHelpers/pierreDiffsReactMock.tsx +++ b/site/src/testHelpers/pierreDiffsReactMock.tsx @@ -1,4 +1,4 @@ -import type { FC, PropsWithChildren } from "react"; +import { createContext, type FC, type PropsWithChildren } from "react"; export type WorkerInitializationRenderOptions = { theme?: { @@ -18,6 +18,8 @@ export const WorkerPoolContextProvider: FC = ({ children, }) => <>{children}; +export const VirtualizerContext = createContext(undefined); + export const FileDiff: FC = () => null; export const File: FC = () => null;