mirror of
https://github.com/coder/coder.git
synced 2026-10-16 06:37:58 +08:00
Jaayden Halko
e421c2f747
## Problem The GitPanel's DiffViewer had several performance and correctness issues that manifested as CPU spikes and scrollbar jank, especially on Safari with large diffs: 1. The `onScroll` handler called `getBoundingClientRect()` on every file wrapper per scroll tick (O(N) forced layouts per frame). 2. `setActiveFile()` re-rendered the entire DiffViewer tree on every active file change. 3. `parsePatchFiles()` ran on every render in `LocalDiffPanel`, `RemoteDiffPanel`, and `GitPanel`. 4. File header `rootMargin` used a percentage, which resolves against the root's width (not height), collapsing the observation strip in wide viewports. 5. The IntersectionObserver effect didn't re-run when the viewport mounted after an initial empty state, or when the viewport was resized. ## Fix ### Scroll tracking Replaced the `getBoundingClientRect` scroll handler with an `IntersectionObserver` watching a narrow strip at the top of the viewport. The observation strip is a pixel value derived from `viewport.clientHeight` (the previous `-95%` margin was broken in wide viewports because CSS margin percentages resolve against width). A `ResizeObserver` tracks viewport height so the strip adapts to layout changes, and the effect keys off a stable `fileListKey` string so scroll-driven re-renders don't tear down the observer. ### Memoization: React Compiler `src/pages/AgentsPage/` is opted into the React Compiler via `site/vite.config.mts`. The compiler automatically memoizes values, callbacks, and JSX at build time. This PR removed the manual `useMemo` / `useCallback` wrappers that were added earlier in the review cycle and lets the compiler handle memoization. `React.memo()` is retained on `FileTreeNodeView` and `LazyFileDiff` — the documented list-item exception from `site/AGENTS.md`. Their `memo()` effectiveness depends on the compiler stabilizing prop references; moving these components outside the compiler scope without adding manual memoization would silently regress scroll performance. `useParsedDiff` uses explicit `useMemo` as a documented exception: the compiler cannot prove purity of the external `parsePatchFiles` function from `@pierre/diffs`, so without `useMemo` the parser would run on every render even when inputs are unchanged. ### `activeCommentBoxRef` (stable annotation handler) `CommentableDiffViewer` wraps `activeCommentBox` in a ref that's synced in event handlers (not during render). This gives `renderAnnotation`, `handleSubmitComment`, and the annotation getters stable identities via the compiler, so comment-box toggles no longer force every `LazyFileDiff` to re-render. ### CSS containment for Safari Added `will-change: transform` on the scroll container and `contain: layout style` on each file wrapper. Programmatic `scrollIntoView` / `scrollBy` calls use `behavior: "instant"` to avoid fighting Safari's scroll compositor. ### Hook extraction Extracted `useActiveFileTracking` (observer setup, viewport sizing, scroll-to-file) and `useParsedDiff` (shared diff parsing with memoization) to keep `DiffViewer` focused on layout and eliminate duplication between `LocalDiffPanel` and `RemoteDiffPanel`. ### Testing Added a `LargeDiff` Storybook story (40 files × 60+ context lines, ~2,400 diff lines) with `isExpanded: true` so the observer code path is exercised, plus a `play` function that scrolls the viewport and asserts the sidebar highlight updates. ## Expected impact - Scroll handler: O(N) `getBoundingClientRect` calls per frame → 0 - Re-renders on scroll: full DiffViewer tree → sidebar only - Diff parsing: on every render → only when `diffString` changes