Skip to content

Code review UI: typing lag and load-time stalls on large diffs #659

@backnotprop

Description

@backnotprop

Users report the review UI getting laggy on moderately sized diffs — specifically while typing in a comment. One user attached Chrome DevTools "Forced reflow while executing JavaScript" violations (~49 occurrences, 80–200ms each) and suggested adding React Compiler.

What we proved

Typing a single character into a comment textarea re-renders AllFilesDiffView (verified with a console.count probe at the top of the component — counter ticks up per keystroke).

Why: commentText state lives in the useAnnotationToolbar hook (packages/review-editor/hooks/useAnnotationToolbar.ts:62), which is called from inside AllFilesDiffView (packages/review-editor/components/AllFilesDiffView.tsx:122). So setCommentText on every keystroke re-renders the entire all-files view → every LazyFileDiff (not memoized) → every Pierre <FileDiff> (not memoized, props unstable). With many files visible, this is the lag.

What we didn't prove (but is suspicious)

  • Forced reflow violations on type. Initial hypothesis was capture-phase window scroll listeners in AnnotationToolbar / CommentPopover / FloatingQuickLabelPicker firing on every keystroke (textarea internal scroll bubbles in capture phase). User-tested: scroll only fires on newline (Enter / Shift+Enter), not per character. So the 49 reflow violations are real but probably triggered by the textarea auto-resizing on newline + the unbatched getBoundingClientRect reads in those listeners — not the per-keystroke lag.
  • Load-time stall on big diffs. AllFilesDiffView:151–155 calls getSingularPatch(file.patch) for every file up front in a useMemo. The "lazy" mount in LazyFileDiff only defers the React mount + the heavier processFile call (which needs full file contents); the structural parse runs eagerly for everything. Untested at scale, but the code path is clearly O(files) on load.

Why the suggested React Compiler fix isn't the whole story

React Compiler reduces re-renders by auto-memoizing — useful, and worth doing — but it doesn't fix the state-placement issue: commentText lives at a level where its updates should re-render the parent. Compiler will memoize children downstream, which would help, but the cleaner fix is to keep textbox state where the textbox lives.

Fix plan

Now

  1. Move commentText (and the rest of the toolbar form state: suggestedCode, conventionalLabel, decorations, showSuggestedCode) local to AnnotationToolbar itself. Bubble up only on submit. Parent never sees keystrokes → diff list stays still. Most of the typing-lag win is here.

Next, if cheap

  1. Stabilize props going into LazyFileDiffoptions, annotations filter (AllFilesDiffView:374–396), and the two render callbacks (AllFilesDiffView:442–498) are all built inline per render. Wrap LazyFileDiff in React.memo. Defensive: keeps the diff list still even when other parent state legitimately changes.

Later

  1. React Compiler. React 19.2.3 is already in use; apps/review/vite.config.ts would need babel-plugin-react-compiler added. Auto-memoizes components and stabilizes prop identity, makes most of OpenCode (awaiting for better plan capability) #2 unnecessary.
  2. Server-side prerender the review diff via preloadPatchFile. @pierre/diffs/ssr is already wired up for CodeFilePopout (packages/server/reference-handlers.ts:20,93) but not for the main review path. Prerendering per-file HTML on the server would skip the client-side parse entirely for non-visible files and eliminate the load stall.
  3. Throttle / drop capture-phase on the floating-UI scroll listeners in AnnotationToolbar (packages/ui/components/AnnotationToolbar.tsx:104), CommentPopover (packages/ui/components/CommentPopover.tsx:74), FloatingQuickLabelPicker (packages/ui/components/FloatingQuickLabelPicker.tsx:60) — wrap updatePosition in requestAnimationFrame. Addresses the reported reflow violations on newline / scroll.
  4. rAF-batch AllFilesDiffView's scroll handler (AllFilesDiffView.tsx:178–206) — currently does an O(N) getBoundingClientRect walk over every expanded file header per scroll event.

Notes

  • The parseDiffToFiles split in App.tsx:95–125 is fine — it's a single O(lines) pass, not a hot spot.
  • Pierre's FileDiff component is not wrapped in React.memo upstream (/Users/ramos/oss/pierre/packages/diffs/src/react/FileDiff.tsx). If we end up needing it, a fix in the library is preferable to wrapping at the consumer level.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions