From 9c6182f044d2960d0c8fce38042665d045e9bcf4 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 24 Sep 2026 11:24:28 +0000 Subject: [PATCH 1/2] perf: let React Compiler compile RightSidebar, ReviewPanel, and the startup gate --- scripts/check_react_compiler_coverage.ts | 3 - .../components/AppLoader/AppLoader.tsx | 4 +- .../RightSidebar/CodeReview/ReviewPanel.tsx | 62 ++++++++++++------- .../features/RightSidebar/RightSidebar.tsx | 20 ++++-- 4 files changed, 58 insertions(+), 31 deletions(-) diff --git a/scripts/check_react_compiler_coverage.ts b/scripts/check_react_compiler_coverage.ts index a4430c2f404..6adfdcf32e9 100644 --- a/scripts/check_react_compiler_coverage.ts +++ b/scripts/check_react_compiler_coverage.ts @@ -52,9 +52,6 @@ const HOT_COMPONENTS: Record = { */ const KNOWN_SKIPPED: ReadonlySet = new Set([ "src/browser/components/ProjectSidebar/ProjectSidebar.tsx#ProjectSidebarInner", - "src/browser/components/AppLoader/AppLoader.tsx#UserPreferencesStartupGate", - "src/browser/features/RightSidebar/RightSidebar.tsx#RightSidebarComponent", - "src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx#ReviewPanel", "src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx#ImmersiveReviewView", ]); diff --git a/src/browser/components/AppLoader/AppLoader.tsx b/src/browser/components/AppLoader/AppLoader.tsx index e2b794aca01..13a52af414d 100644 --- a/src/browser/components/AppLoader/AppLoader.tsx +++ b/src/browser/components/AppLoader/AppLoader.tsx @@ -80,7 +80,9 @@ function UserPreferencesStartupGate(props: { children: ReactNode }) { }; }, [apiState.api]); - if (bootstrappedRef.current || ready) { + // bootstrappedRef is set together with `ready`, so `ready` alone decides here; reading + // the ref during render would make React Compiler skip this component. + if (ready) { return <>{props.children}; } diff --git a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx index 6ac9364d539..a1c2e66ac28 100644 --- a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx @@ -29,6 +29,7 @@ import React, { useEffect, useMemo, useCallback, + useLayoutEffect, useRef, useSyncExternalStore, } from "react"; @@ -106,6 +107,7 @@ import { useWorkspaceMetadata } from "@/browser/contexts/WorkspaceContext"; import { workspaceStore, useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; import { invalidateGitStatus } from "@/browser/stores/GitStatusStore"; import { getErrorMessage } from "@/common/utils/errors"; +import { runWithCatch, runWithCatchFinally } from "@/browser/utils/compilerSafeControlFlow"; /** Stats reported to parent for tab display */ interface ReviewPanelStats { @@ -924,8 +926,8 @@ export const ReviewPanel: React.FC = ({ } let cancelled = false; - void (async () => { - try { + void runWithCatch( + async () => { const branchResult = await api.projects.listBranches({ projectPath }); const detectedBase = toOriginDiffBase(branchResult.recommendedTrunk); if (cancelled) { @@ -949,10 +951,11 @@ export const ReviewPanel: React.FC = ({ setDiffBase(detectedBase); } } - } catch { + }, + () => { // Best effort only; keep WORKSPACE_DEFAULTS.reviewBase when detection fails. } - })(); + ); return () => { cancelled = true; @@ -974,10 +977,14 @@ export const ReviewPanel: React.FC = ({ // Refs for values that change frequently but are only read at callback invocation time. // Using refs allows callbacks to stay stable (same reference) while still accessing current values. // This prevents all HunkViewer components from re-rendering when these values change. + // Refs are synced in layout effects (same task as the commit) because React Compiler + // rejects ref writes during render; the same applies to the other latest-value refs below. const isReadRef = useRef(isRead); - isReadRef.current = isRead; const selectedHunkIdRef = useRef(selectedHunkId); - selectedHunkIdRef.current = selectedHunkId; + useLayoutEffect(() => { + isReadRef.current = isRead; + selectedHunkIdRef.current = selectedHunkId; + }); useEffect(() => { updatePersistedState(selectedHunkStorageKey, selectedHunkId); @@ -1206,9 +1213,12 @@ export const ReviewPanel: React.FC = ({ // mirrored only `filters.showReadHunks`, which caused the panel to // navigate away from a hunk that was actually still visible whenever // Assisted's override forced show-read true. - showReadHunksRef.current = filters.assistedOnly + const effectiveShowReadHunks = filters.assistedOnly ? filters.assistedShowReadHunks : filters.showReadHunks; + useLayoutEffect(() => { + showReadHunksRef.current = effectiveShowReadHunks; + }); // Track if user is drafting a review note (selection or editing an existing note). // We only pause scheduled refreshes while drafting so tool-driven refresh stays unified @@ -1218,6 +1228,14 @@ export const ReviewPanel: React.FC = ({ const composingHunksRef = useRef(new Set()); const editingReviewIdsRef = useRef(new Set()); + // Track if refresh button should be disabled (drafting or editing a review note) + const [isRefreshBlocked, setIsRefreshBlocked] = useState(false); + + // RefreshController - handles debouncing, in-flight guards, etc. + // Created in useEffect to survive React StrictMode double-mount. + // (StrictMode calls cleanup then re-mounts; refs persist but controller would be disposed) + const controllerRef = useRef(null); + const updateRefreshBlockState = useCallback(() => { const wasComposing = isComposingReviewNoteRef.current; const nowComposing = composingHunksRef.current.size > 0 || editingReviewIdsRef.current.size > 0; @@ -1285,13 +1303,6 @@ export const ReviewPanel: React.FC = ({ const [lastRefreshInfo, setLastRefreshInfo] = useState(null); // Last refresh failure for UI display (tooltip showing latest refresh error) const [lastRefreshFailure, setLastRefreshFailure] = useState(null); - // Track if refresh button should be disabled (drafting or editing a review note) - const [isRefreshBlocked, setIsRefreshBlocked] = useState(false); - - // RefreshController - handles debouncing, in-flight guards, etc. - // Created in useEffect to survive React StrictMode double-mount. - // (StrictMode calls cleanup then re-mounts; refs persist but controller would be disposed) - const controllerRef = useRef(null); useEffect(() => { const controller = new RefreshController({ @@ -1408,7 +1419,7 @@ export const ReviewPanel: React.FC = ({ const loadFileTree = async () => { setIsLoadingTree(true); - try { + const fetchFileTree = async () => { await ensureOriginFetched({ api, workspaceId, @@ -1486,13 +1497,18 @@ export const ReviewPanel: React.FC = ({ if (cancelled) return; lastFileTreeRefreshTriggerRef.current = refreshTrigger; setFileTree(tree); - } catch (err) { - console.error("Failed to load file tree:", err); - } finally { - if (!cancelled) { - setIsLoadingTree(false); + }; + await runWithCatchFinally( + fetchFileTree, + (err) => { + console.error("Failed to load file tree:", err); + }, + () => { + if (!cancelled) { + setIsLoadingTree(false); + } } - } + ); }; void loadFileTree(); @@ -1966,7 +1982,9 @@ export const ReviewPanel: React.FC = ({ : false); // Keep ref in sync so callbacks can access current filtered list without dependency - filteredHunksRef.current = filteredHunks; + useLayoutEffect(() => { + filteredHunksRef.current = filteredHunks; + }); // Ensure selectedHunkId is valid after filtering/sorting: // - If no selection or selection not in the validity list, select first visible hunk diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index 49562a178b5..b848f0eab0b 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -877,9 +877,12 @@ const RightSidebarComponent: React.FC = ({ const [layoutDraft, setLayoutDraft] = React.useState(null); const layoutDraftRef = React.useRef(null); - // Ref to access latest layoutRaw without causing callback recreation + // Ref to access latest layoutRaw without causing callback recreation. Synced in a + // layout effect (same task as the commit) because React Compiler rejects render-time ref writes. const layoutRawRef = React.useRef(layoutRaw); - layoutRawRef.current = layoutRaw; + React.useLayoutEffect(() => { + layoutRawRef.current = layoutRaw; + }); const isSidebarTabDragInProgressRef = React.useRef(false); @@ -1295,8 +1298,10 @@ const RightSidebarComponent: React.FC = ({ _setFocusTrigger((prev) => prev + 1); } + // A per-iteration copy: React Compiler can't lower `i++` on a variable a closure captures. + const tabIndex = i; setLayout((prev) => - selectTabByIndex(canReviewDiffs ? prev : removeTabEverywhere(prev, "review"), i) + selectTabByIndex(canReviewDiffs ? prev : removeTabEverywhere(prev, "review"), tabIndex) ); setCollapsed(false); return; @@ -1428,6 +1433,12 @@ const RightSidebarComponent: React.FC = ({ // Sync terminal tabs with backend sessions on workspace mount. // - Adds tabs for backend sessions that don't have tabs (restore after reload) // - Removes "ghost" tabs for sessions that no longer exist (cleanup after app restart) + // Runs only on workspace change, not layout change (layout.root as a dependency would + // loop), so it reads the latest layout through a ref when the session list arrives. + const layoutForSessionSyncRef = React.useRef(layout); + React.useLayoutEffect(() => { + layoutForSessionSyncRef.current = layout; + }); React.useEffect(() => { if (!api) return; @@ -1439,7 +1450,7 @@ const RightSidebarComponent: React.FC = ({ const backendSessionSet = new Set(backendSessionIds); // Get current terminal tabs in layout - const currentTabs = collectAllTabs(layout.root); + const currentTabs = collectAllTabs(layoutForSessionSyncRef.current.root); const currentTerminalTabs = currentTabs.filter(isTerminalTab); const currentTerminalSessionIds = new Set( currentTerminalTabs.map(getTerminalSessionId).filter(Boolean) @@ -1478,7 +1489,6 @@ const RightSidebarComponent: React.FC = ({ return () => { cancelled = true; }; - // eslint-disable-next-line react-hooks/exhaustive-deps -- Only run on workspace change, not layout change. layout.root would cause infinite loop. }, [api, workspaceId, setLayout]); // Handler to update a terminal's title (from OSC sequences) From 80c9520b6927f06756e42d4f3a1bd4a3c2632064 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 24 Sep 2026 12:06:26 +0000 Subject: [PATCH 2/2] perf: snapshot the layout when the terminal session request starts --- src/browser/features/RightSidebar/RightSidebar.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/browser/features/RightSidebar/RightSidebar.tsx b/src/browser/features/RightSidebar/RightSidebar.tsx index b848f0eab0b..4f64dd59785 100644 --- a/src/browser/features/RightSidebar/RightSidebar.tsx +++ b/src/browser/features/RightSidebar/RightSidebar.tsx @@ -1434,7 +1434,9 @@ const RightSidebarComponent: React.FC = ({ // - Adds tabs for backend sessions that don't have tabs (restore after reload) // - Removes "ghost" tabs for sessions that no longer exist (cleanup after app restart) // Runs only on workspace change, not layout change (layout.root as a dependency would - // loop), so it reads the latest layout through a ref when the session list arrives. + // loop), so it snapshots the layout through a ref when the request starts. Comparing the + // backend list with that snapshot (not the latest layout) keeps terminals created or + // closed while the request is in flight from being removed or re-added. const layoutForSessionSyncRef = React.useRef(layout); React.useLayoutEffect(() => { layoutForSessionSyncRef.current = layout; @@ -1443,6 +1445,7 @@ const RightSidebarComponent: React.FC = ({ if (!api) return; let cancelled = false; + const layoutAtRequestStart = layoutForSessionSyncRef.current; void api.terminal.listSessions({ workspaceId }).then((backendSessionIds) => { if (cancelled) return; @@ -1450,7 +1453,7 @@ const RightSidebarComponent: React.FC = ({ const backendSessionSet = new Set(backendSessionIds); // Get current terminal tabs in layout - const currentTabs = collectAllTabs(layoutForSessionSyncRef.current.root); + const currentTabs = collectAllTabs(layoutAtRequestStart.root); const currentTerminalTabs = currentTabs.filter(isTerminalTab); const currentTerminalSessionIds = new Set( currentTerminalTabs.map(getTerminalSessionId).filter(Boolean)