From eecd4dc6d5e8b0787e2aa9c6c0a67194dbd8867e Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Thu, 5 Nov 2026 19:33:45 -0800 Subject: [PATCH] perf(renderer): derive state instead of syncing it in effects (surfaces) (#38797) Replace setState-only effects with render-time adjustment (prev-value or self-limiting guard) so prop changes no longer commit a stale render and then re-render. Drop the now-obsolete no-derived-state-effect suppression for use-combined-diff-view-preferences.ts. Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../check-changed-code-quality.mjs | 1 - .../annotate/browser-page-annotation-tray.tsx | 19 ++++++++----------- .../assemble-chrome/BrowserToolbarMenu.tsx | 15 ++++++++------- .../use-combined-diff-view-preferences.ts | 16 +++++++++++----- .../MarkdownTableOfContentsPanel.tsx | 7 +++++-- .../emulator-pane/emulator-device-frame.tsx | 11 ++++++----- .../use-emulator-pane-session.ts | 8 +++----- .../native-chat/NativeChatResolvedView.tsx | 6 ++++-- .../use-native-chat-context-menu.tsx | 11 +++-------- .../use-native-chat-pending-delivery.ts | 6 +++++- .../terminal-pane/TerminalOverlaySlot.tsx | 10 ++++------ .../composer-state/initial-target-state.ts | 8 +++----- 12 files changed, 60 insertions(+), 58 deletions(-) diff --git a/config/scripts/preflight/changed-checks/check-changed-code-quality.mjs b/config/scripts/preflight/changed-checks/check-changed-code-quality.mjs index e5b1439d63f..a6186e49e5c 100644 --- a/config/scripts/preflight/changed-checks/check-changed-code-quality.mjs +++ b/config/scripts/preflight/changed-checks/check-changed-code-quality.mjs @@ -86,7 +86,6 @@ const SUPPRESSED_REACT_DOCTOR_DIAGNOSTICS = new Map([ [ 'react-doctor(no-derived-state-effect)', new Set([ - 'src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts', // The open-cycle reset effect predates this entry; it surfaced when its body changed. 'src/renderer/src/components/right-sidebar/source-control/ai/useSourceControlAgentActionDialog.ts' ]) diff --git a/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx b/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx index 1db962ae9d6..4f0e4c161e8 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx +++ b/src/renderer/src/components/browser-pane/annotate/browser-page-annotation-tray.tsx @@ -1,5 +1,5 @@ import { ImeTextarea } from '@/lib/ime-text-field' -import { useEffect, useState } from 'react' +import { useState } from 'react' import { ChevronDown, ChevronRight, @@ -59,20 +59,17 @@ export function BrowserPageAnnotationTray({ const [editComment, setEditComment] = useState('') const [editIntent, setEditIntent] = useState('change') const editImages = useBrowserAnnotationImages(worktreeId) - const resetEditImages = editImages.resetImages const [expanded, setExpanded] = useState(true) const ToggleChevron = expanded ? ChevronDown : ChevronRight // Why: a delete or clear while a row is mid-edit must not leave edit state pointing at nothing. - useEffect(() => { - if ( - editingAnnotationId && - !browserAnnotations.some((annotation) => annotation.id === editingAnnotationId) - ) { - setEditingAnnotationId(null) - resetEditImages() - } - }, [browserAnnotations, editingAnnotationId, resetEditImages]) + if ( + editingAnnotationId && + !browserAnnotations.some((annotation) => annotation.id === editingAnnotationId) + ) { + setEditingAnnotationId(null) + editImages.resetImages() + } const handleStartEdit = (annotation: BrowserPageAnnotation): void => { setEditingAnnotationId(annotation.id) diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/BrowserToolbarMenu.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/BrowserToolbarMenu.tsx index afc540dd584..ee79f4216ab 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/BrowserToolbarMenu.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/BrowserToolbarMenu.tsx @@ -1,4 +1,4 @@ -import { useEffect, useLayoutEffect, useState } from 'react' +import { useEffect, useState } from 'react' import { toast } from 'sonner' import { emitBrowserCookieImportToast } from '@/lib/browser-cookie-import-toast' import { useAppStore } from '@/store' @@ -80,7 +80,13 @@ export function BrowserToolbarMenu({ const [pendingSwitchProfileId, setPendingSwitchProfileId] = useState( undefined ) - const [menuOpen, setMenuOpen] = useState(false) + // Only expose the hidden Import Cookies row when the tour reaches its fallback step. + const [menuOpen, setMenuOpen] = useState(shouldForceMenuOpen) + const [prevShouldForceMenuOpen, setPrevShouldForceMenuOpen] = useState(shouldForceMenuOpen) + if (prevShouldForceMenuOpen !== shouldForceMenuOpen) { + setPrevShouldForceMenuOpen(shouldForceMenuOpen) + setMenuOpen(shouldForceMenuOpen) + } const [siteDisplay, setSiteDisplay] = useState(null) const mountedRef = useMountedRef() @@ -112,11 +118,6 @@ export function BrowserToolbarMenu({ }) }, [browserPageId, menuOpen, mountedRef]) - useLayoutEffect(() => { - // Only expose the hidden Import Cookies row when the tour reaches its fallback step. - setMenuOpen(shouldForceMenuOpen) - }, [shouldForceMenuOpen]) - const handleMenuOpenChange = (open: boolean): void => { if (shouldForceMenuOpen && !open) { return diff --git a/src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts b/src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts index f11115ca6d4..b3694397718 100644 --- a/src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts +++ b/src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useState } from 'react' import type React from 'react' import type { DiffSection } from '../../diff/sections/diff-section-types' import { combinedDiffViewPreferences } from '../remember-view/combined-diff-view-memory' @@ -44,20 +44,26 @@ export function useCombinedDiffViewPreferences({ ) // Use settings until the first session toolbar choice, then retain that choice over global defaults. - useEffect(() => { + const [prevDiffDefaultView, setPrevDiffDefaultView] = useState(diffDefaultView) + if (prevDiffDefaultView !== diffDefaultView) { + setPrevDiffDefaultView(diffDefaultView) if (diffDefaultView !== undefined && combinedDiffViewPreferences.sideBySide === null) { setSideBySide(diffDefaultView === 'side-by-side') } - }, [diffDefaultView]) + } - useEffect(() => { + const [prevFileTreeVisibleByDefault, setPrevFileTreeVisibleByDefault] = useState( + combinedDiffFileTreeVisibleByDefault + ) + if (prevFileTreeVisibleByDefault !== combinedDiffFileTreeVisibleByDefault) { + setPrevFileTreeVisibleByDefault(combinedDiffFileTreeVisibleByDefault) if ( combinedDiffFileTreeVisibleByDefault !== undefined && combinedDiffViewPreferences.fileTreeCollapsed === null ) { setFileTreeCollapsedState(combinedDiffFileTreeVisibleByDefault === false) } - }, [combinedDiffFileTreeVisibleByDefault]) + } const setFileTreeCollapsed = useCallback((collapsed: boolean) => { combinedDiffViewPreferences.fileTreeCollapsed = collapsed diff --git a/src/renderer/src/components/editor/markdown/navigation/MarkdownTableOfContentsPanel.tsx b/src/renderer/src/components/editor/markdown/navigation/MarkdownTableOfContentsPanel.tsx index 1522f5a9512..765c8eb9e69 100644 --- a/src/renderer/src/components/editor/markdown/navigation/MarkdownTableOfContentsPanel.tsx +++ b/src/renderer/src/components/editor/markdown/navigation/MarkdownTableOfContentsPanel.tsx @@ -53,9 +53,12 @@ export function MarkdownTableOfContentsPanel({ setWidth: setMarkdownTocPanelWidth }) - useEffect(() => { + // Prune on change, not in a derivation, so a heading that is retyped does not resurrect its collapse. + const [prevItems, setPrevItems] = useState(items) + if (prevItems !== items) { + setPrevItems(items) setCollapsedIds((current) => pruneMarkdownTocCollapsedIds(current, items)) - }, [items]) + } useEffect(() => { const container = containerRef.current diff --git a/src/renderer/src/components/emulator-pane/emulator-device-frame.tsx b/src/renderer/src/components/emulator-pane/emulator-device-frame.tsx index 58a894e30f2..fcd2d6ebab3 100644 --- a/src/renderer/src/components/emulator-pane/emulator-device-frame.tsx +++ b/src/renderer/src/components/emulator-pane/emulator-device-frame.tsx @@ -85,6 +85,12 @@ export function EmulatorDeviceFrame({ const wheelGestureRef = useRef(null) const [streamError, setStreamError] = useState(false) const [streamSize, setStreamSize] = useState(null) + const [prevStream, setPrevStream] = useState({ previewUrl, streamKey }) + if (prevStream.previewUrl !== previewUrl || prevStream.streamKey !== streamKey) { + setPrevStream({ previewUrl, streamKey }) + setStreamError(false) + setStreamSize(null) + } const canInteract = isLive && !loading && !streamError const scrcpyDeviceId = scrcpyDeviceIdFromStreamUrl(previewUrl) const controlStream = useEmulatorControlStream(wsUrl, canInteract, scrcpyDeviceId) @@ -98,11 +104,6 @@ export function EmulatorDeviceFrame({ const { enableKeyboardCapture, handleBlur, handleKeyDown, handlePaste, keyboardCaptureActive } = useEmulatorScreenKeyboard({ ...controlStream, canInteract }) - useEffect(() => { - setStreamError(false) - setStreamSize(null) - }, [previewUrl, streamKey]) - const mapEventToScreenPoint = useCallback( (event: ScreenCoordinateEvent): EmulatorScreenPoint | null => mapClientPointToSimulatorScreen( diff --git a/src/renderer/src/components/emulator-pane/use-emulator-pane-session.ts b/src/renderer/src/components/emulator-pane/use-emulator-pane-session.ts index 8b9b42790df..300989d561d 100644 --- a/src/renderer/src/components/emulator-pane/use-emulator-pane-session.ts +++ b/src/renderer/src/components/emulator-pane/use-emulator-pane-session.ts @@ -251,11 +251,9 @@ export function useEmulatorPaneSession({ ] ) - useEffect(() => { - if (!selectedUdid && configuredDefaultUdid) { - setSelectedUdid(configuredDefaultUdid) - } - }, [configuredDefaultUdid, selectedUdid]) + if (!selectedUdid && configuredDefaultUdid) { + setSelectedUdid(configuredDefaultUdid) + } const shutdown = useEmulatorPaneShutdown({ loading, diff --git a/src/renderer/src/components/native-chat/NativeChatResolvedView.tsx b/src/renderer/src/components/native-chat/NativeChatResolvedView.tsx index c55435715d5..bdc993001dd 100644 --- a/src/renderer/src/components/native-chat/NativeChatResolvedView.tsx +++ b/src/renderer/src/components/native-chat/NativeChatResolvedView.tsx @@ -140,10 +140,12 @@ export function NativeChatResolvedView({ const [commandMarkers, setCommandMarkers] = useState(() => readCommandMarkerCache(commandMarkerScope) ) - useEffect(() => { + const [prevCommandMarkerScope, setPrevCommandMarkerScope] = useState(commandMarkerScope) + if (prevCommandMarkerScope !== commandMarkerScope) { + setPrevCommandMarkerScope(commandMarkerScope) setCommandMarkers(readCommandMarkerCache(commandMarkerScope)) setWorkingInterrupted(false) - }, [commandMarkerScope]) + } useEffect(() => { if (!paneLaunchPrompt || !shouldPruneLaunchPrompt(paneLaunchPrompt, session.messages)) { return diff --git a/src/renderer/src/components/native-chat/use-native-chat-context-menu.tsx b/src/renderer/src/components/native-chat/use-native-chat-context-menu.tsx index 2191a68da04..32444cc3975 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-context-menu.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-context-menu.tsx @@ -1,6 +1,5 @@ import { useCallback, - useEffect, useRef, useState, type MouseEventHandler, @@ -180,13 +179,9 @@ export function useNativeChatContextMenu({ const hasItems = hasEditItems || hasSessionItems || chatPane !== undefined || resolveOrcaSessionId !== undefined - useEffect(() => { - if (!enabled) { - setState((current) => - current.open ? { ...current, open: false, image: undefined } : current - ) - } - }, [enabled]) + if (!enabled && state.open) { + setState({ ...state, open: false, image: undefined }) + } const onContextMenuCapture = useCallback( (event: React.MouseEvent) => { diff --git a/src/renderer/src/components/native-chat/use-native-chat-pending-delivery.ts b/src/renderer/src/components/native-chat/use-native-chat-pending-delivery.ts index 4c8d6efc654..6366aa039f2 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-pending-delivery.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-pending-delivery.ts @@ -51,7 +51,11 @@ export function useNativeChatPendingDelivery(args: { () => releaseEmptyTranscriptClaims(cachedPending, sessionId), [cachedPending, sessionId] ) - useEffect(() => setPending(readPendingSendCache(scope)), [scope]) + const [prevScope, setPrevScope] = useState(scope) + if (prevScope !== scope) { + setPrevScope(scope) + setPending(readPendingSendCache(scope)) + } const save = useCallback( (update: (entries: NativeChatPendingSend[]) => NativeChatPendingSend[]) => { const current = readPendingSendCache(scope) diff --git a/src/renderer/src/components/terminal-pane/TerminalOverlaySlot.tsx b/src/renderer/src/components/terminal-pane/TerminalOverlaySlot.tsx index dbb47cc895f..e6886e3bf43 100644 --- a/src/renderer/src/components/terminal-pane/TerminalOverlaySlot.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalOverlaySlot.tsx @@ -1,4 +1,4 @@ -import { memo, useLayoutEffect, useState } from 'react' +import { memo, useState } from 'react' import { createPortal } from 'react-dom' import { useAppStore } from '../../store' import { isProvenProcessExit } from '../../../../shared/terminal/pty/lifecycle/terminal-exit-cause' @@ -50,11 +50,9 @@ export const TerminalOverlaySlot = memo(function TerminalOverlaySlot({ const [shouldMeasureHiddenStartup, setShouldMeasureHiddenStartup] = useState( () => useAppStore.getState().pendingStartupByTabId[terminalTabId] !== undefined ) - useLayoutEffect(() => { - if (isVisible && shouldMeasureHiddenStartup) { - setShouldMeasureHiddenStartup(false) - } - }, [isVisible, shouldMeasureHiddenStartup]) + if (isVisible && shouldMeasureHiddenStartup) { + setShouldMeasureHiddenStartup(false) + } // Drop with the reveal in one render, or a later lease release re-runs resume and steals focus. const prewarm = isVisible || activityTerminalPortal !== null ? null : hiddenPrewarm diff --git a/src/renderer/src/hooks/composer-state/initial-target-state.ts b/src/renderer/src/hooks/composer-state/initial-target-state.ts index e0e8a9d41da..38eeb80d51e 100644 --- a/src/renderer/src/hooks/composer-state/initial-target-state.ts +++ b/src/renderer/src/hooks/composer-state/initial-target-state.ts @@ -153,11 +153,9 @@ export function useComposerInitialTargetState(input: ComposerInitialTargetStateI [actionableHostIds, projectGroups, selectedProjectGroupId] ) - useEffect(() => { - if (selectedProjectGroupId && !selectedProjectGroup) { - setSelectedProjectGroupId(null) - } - }, [selectedProjectGroup, selectedProjectGroupId]) + if (selectedProjectGroupId && !selectedProjectGroup) { + setSelectedProjectGroupId(null) + } useEffect(() => { if (