From 929d7daa37a1c32f7d7435f7b556c7399392852a Mon Sep 17 00:00:00 2001 From: xiaolai Date: Sat, 27 Jun 2026 16:16:33 +0800 Subject: [PATCH 1/6] chore(lint): adopt react-hooks/immutability rule (#1063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single violation is a false positive: editor.storage is Tiptap's intentionally-mutable extension storage bag, and writing the show-invisibles flag there is the documented way to toggle an extension's runtime state — not React-owned state. Scoped-disable with a reason rather than a refactor. --- eslint.config.js | 2 +- src/components/Editor/TiptapEditor.tsx | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index 3e7828969..6141b63f9 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -27,7 +27,7 @@ export default tseslint.config( "react-hooks/set-state-in-effect": "off", "react-hooks/refs": "off", "react-hooks/preserve-manual-memoization": "off", - "react-hooks/immutability": "off", + "react-hooks/immutability": "error", // Historically `warn` under v5's recommended; v7 raised it to error. // Keep it non-blocking to match prior behavior. "react-hooks/exhaustive-deps": "warn", diff --git a/src/components/Editor/TiptapEditor.tsx b/src/components/Editor/TiptapEditor.tsx index b9c53819f..c3baee1ae 100644 --- a/src/components/Editor/TiptapEditor.tsx +++ b/src/components/Editor/TiptapEditor.tsx @@ -444,6 +444,9 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = | Record | undefined; const storage = allStorage?.showInvisibles; + // editor.storage is Tiptap's intentionally-mutable extension storage bag; writing the + // flag here is the documented way to toggle an extension's runtime state — not React state. + // eslint-disable-next-line react-hooks/immutability if (storage) storage.enabled = showInvisibles; // Force an immediate rebuild via the plugin's exported helper — // this dispatches a tagged transaction the plugin recognises by From aa12e35e97067ad35c80100d63f1d85e38a204d3 Mon Sep 17 00:00:00 2001 From: xiaolai Date: Sat, 27 Jun 2026 16:19:15 +0800 Subject: [PATCH 2/6] chore(lint): adopt react-hooks/preserve-manual-memoization rule (#1063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The revealLabel useMemo read the navigator global, which the React Compiler cannot preserve. Hoisted the pure platform-to-key logic to a module function and dropped the manual useMemo — the compiler auto-memoizes the component, so the memo was redundant. menuLabels keeps its (preservable) useMemo. --- eslint.config.js | 2 +- .../Sidebar/FileExplorer/ContextMenu.tsx | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 6141b63f9..824d5760c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -26,7 +26,7 @@ export default tseslint.config( // rule, file-scoped) in a dedicated react-hooks-7 adoption pass — see #1063. "react-hooks/set-state-in-effect": "off", "react-hooks/refs": "off", - "react-hooks/preserve-manual-memoization": "off", + "react-hooks/preserve-manual-memoization": "error", "react-hooks/immutability": "error", // Historically `warn` under v5's recommended; v7 raised it to error. // Keep it non-blocking to match prior behavior. diff --git a/src/components/Sidebar/FileExplorer/ContextMenu.tsx b/src/components/Sidebar/FileExplorer/ContextMenu.tsx index da8678842..02cb3f8a8 100644 --- a/src/components/Sidebar/FileExplorer/ContextMenu.tsx +++ b/src/components/Sidebar/FileExplorer/ContextMenu.tsx @@ -104,6 +104,14 @@ function findNextFocusable(total: number, current: number, direction: 1 | -1): n return (current + direction + total) % total; } +/** Platform-appropriate translation key for the "reveal in file manager" action. */ +function revealLabelKey(): string { + const platform = typeof navigator !== "undefined" ? navigator.platform.toLowerCase() : ""; + if (platform.includes("mac")) return "contextMenu.revealInFinder"; + if (platform.includes("win")) return "contextMenu.showInExplorer"; + return "contextMenu.showInFileManager"; +} + interface ContextMenuProps { type: ContextMenuType; position: ContextMenuPosition; @@ -118,13 +126,10 @@ export function ContextMenu({ type, position, onAction, onClose }: ContextMenuPr const itemRefs = useRef>([]); const [focusedIndex, setFocusedIndex] = useState(-1); - // Resolve platform-appropriate "reveal in file manager" label via translation keys - const revealLabel = useMemo(() => { - const platform = typeof navigator !== "undefined" ? navigator.platform.toLowerCase() : ""; - if (platform.includes("mac")) return t("contextMenu.revealInFinder"); - if (platform.includes("win")) return t("contextMenu.showInExplorer"); - return t("contextMenu.showInFileManager"); - }, [t]); + // Resolve platform-appropriate "reveal in file manager" label via translation keys. + // The React Compiler auto-memoizes the component, so no manual useMemo is needed — + // and a useMemo reading the `navigator` global can't be preserved by the compiler (#1063). + const revealLabel = t(revealLabelKey()); const menuLabels = useMemo(() => ({ open: t("contextMenu.open"), From 8c5a1763028f47b85531827041fb4c8b8a3be6da Mon Sep 17 00:00:00 2001 From: xiaolai Date: Sat, 27 Jun 2026 16:33:22 +0800 Subject: [PATCH 3/6] chore(lint): adopt react-hooks/refs rule (#1063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminate render-phase ref access (not concurrent-safe) across 12 sites: - Latest-value ref syncs moved into post-commit effects (the React-recommended pattern), where the refs are read only from callbacks/effects/listeners: TiptapEditor, SourceEditor, SourcePane, useContentSearchScheduler, useTerminalSessions, useContentServer, useGenieInvocation, useTabDragOut. - McpHistoryButton + WordCountPopover: measure the anchor in a layout effect and store the position in state, instead of reading a sibling DOM rect during render. - OutlineView: re-key the headings useMemo on headingLinesKey (dropping the cache-ref pattern), preserving referential stability without render-phase refs. - TiptapEditor editorRef + flushToStoreRef stay render-synced with scoped disables — the unmount-flush path (#755) needs them set before passive effects could run. --- eslint.config.js | 2 +- .../useContentSearchScheduler.ts | 5 +++- src/components/Editor/SourceEditor.tsx | 15 +++++++---- .../Editor/SplitPaneEditor/SourcePane.tsx | 5 +++- src/components/Editor/TiptapEditor.tsx | 25 ++++++++++++++----- .../McpHistory/McpHistoryButton.tsx | 25 +++++++++++-------- src/components/Sidebar/OutlineView.tsx | 21 ++++++---------- src/components/StatusBar/WordCountPopover.tsx | 21 ++++++++++------ .../Terminal/useTerminalSessions.ts | 5 +++- src/hooks/useContentServer.ts | 5 +++- src/hooks/useGenieInvocation.ts | 7 ++++-- src/hooks/useTabDragOut.ts | 15 ++++++----- 12 files changed, 95 insertions(+), 56 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 824d5760c..2c1316d69 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -25,7 +25,7 @@ export default tseslint.config( // code was written and verified against. Re-enable incrementally (per // rule, file-scoped) in a dedicated react-hooks-7 adoption pass — see #1063. "react-hooks/set-state-in-effect": "off", - "react-hooks/refs": "off", + "react-hooks/refs": "error", "react-hooks/preserve-manual-memoization": "error", "react-hooks/immutability": "error", // Historically `warn` under v5's recommended; v7 raised it to error. diff --git a/src/components/ContentSearch/useContentSearchScheduler.ts b/src/components/ContentSearch/useContentSearchScheduler.ts index 2cd73eb7e..5e4af58c2 100644 --- a/src/components/ContentSearch/useContentSearchScheduler.ts +++ b/src/components/ContentSearch/useContentSearchScheduler.ts @@ -51,7 +51,10 @@ export function useContentSearchScheduler({ // query/option change without a stale capture and without re-running the // search merely because the array identity changed. const excludeFoldersRef = useRef(excludeFolders); - excludeFoldersRef.current = excludeFolders; + // Synced after commit (read only at query-execution time). #1063 + useEffect(() => { + excludeFoldersRef.current = excludeFolders; + }); useEffect(() => { if (!isOpen || !rootPath) return; diff --git a/src/components/Editor/SourceEditor.tsx b/src/components/Editor/SourceEditor.tsx index eb4a9a5d4..70180802c 100644 --- a/src/components/Editor/SourceEditor.tsx +++ b/src/components/Editor/SourceEditor.tsx @@ -66,7 +66,6 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP const viewRef = useRef(null); const isInternalChange = useRef(false); const hiddenRef = useRef(hidden); - hiddenRef.current = hidden; useSourceOutlineSync(viewRef, hidden); @@ -80,10 +79,16 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP const setCursorInfoRef = useRef(setCursorInfo); const setSelectedTextRef = useRef(setSelectedText); const cursorInfoRef = useRef(cursorInfo); - setContentRef.current = setContent; - setCursorInfoRef.current = setCursorInfo; - setSelectedTextRef.current = setSelectedText; - cursorInfoRef.current = cursorInfo; + // Keep "latest value" refs in sync after each commit. All are read only from + // the CodeMirror listener / effects (never during render), so a post-commit + // effect is the React-recommended, concurrent-safe pattern. See #1063. + useEffect(() => { + hiddenRef.current = hidden; + setContentRef.current = setContent; + setCursorInfoRef.current = setCursorInfo; + setSelectedTextRef.current = setSelectedText; + cursorInfoRef.current = cursorInfo; + }); // Use editor store for global settings const wordWrap = useUIStore((state) => state.wordWrap); diff --git a/src/components/Editor/SplitPaneEditor/SourcePane.tsx b/src/components/Editor/SplitPaneEditor/SourcePane.tsx index 904d9c868..df7465b58 100644 --- a/src/components/Editor/SplitPaneEditor/SourcePane.tsx +++ b/src/components/Editor/SplitPaneEditor/SourcePane.tsx @@ -71,7 +71,10 @@ export function SourcePane({ // would tear down and rebuild the CodeMirror view, blowing away undo // history and the user's selection. (Audit finding H3.) const onDiagnosticsRef = useRef(onDiagnostics); - onDiagnosticsRef.current = onDiagnostics; + // Synced after commit (read only from the CodeMirror diagnostics callback). #1063 + useEffect(() => { + onDiagnosticsRef.current = onDiagnostics; + }); // Stable jump-to-position handle, safe to re-emit whenever the parent's // callback prop changes identity. Lives outside the mount effect so a diff --git a/src/components/Editor/TiptapEditor.tsx b/src/components/Editor/TiptapEditor.tsx index c3baee1ae..58cca9867 100644 --- a/src/components/Editor/TiptapEditor.tsx +++ b/src/components/Editor/TiptapEditor.tsx @@ -180,12 +180,18 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = const contentRef = useRef(content); const editorRef = useRef(null); const flushToStoreRef = useRef<((editor: TiptapEditor) => void) | null>(null); - cursorInfoRef.current = cursorInfo; - preserveLineBreaksRef.current = preserveLineBreaks; - hardBreakStyleOnSaveRef.current = hardBreakStyleOnSave; - hiddenRef.current = hidden; - previewRef.current = preview; - contentRef.current = content; + // Keep "latest value" refs in sync after each commit. These are read only from + // callbacks/effects (never during render), so a post-commit effect is the + // React-recommended pattern — and it is concurrent-safe (a discarded render + // can't corrupt them, unlike a render-phase write). See #1063. + useEffect(() => { + cursorInfoRef.current = cursorInfo; + preserveLineBreaksRef.current = preserveLineBreaks; + hardBreakStyleOnSaveRef.current = hardBreakStyleOnSave; + hiddenRef.current = hidden; + previewRef.current = preview; + contentRef.current = content; + }); const extensions = useMemo( () => createTiptapExtensions({ tabId: activeTabId, lintEnabled }), @@ -228,6 +234,10 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = }, [setContent, windowLabel] ); + // Synced during render (not in an effect) on purpose: the unmount cleanup below + // reads this ref to flush pending content, and must see the latest flusher even + // if the component unmounts before a passive effect could run (#755). + // eslint-disable-next-line react-hooks/refs flushToStoreRef.current = flushToStore; const flushCursorInfo = useCallback(() => { @@ -431,6 +441,9 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = // directly without depending on the global flusher registry — which may be // nulled by this component's own registration cleanup before the flush // cleanup runs (React runs effect cleanups in reverse registration order). + // Synced during render (not in an effect) on purpose so it is set even if the + // component unmounts before a passive effect could run (#755). + // eslint-disable-next-line react-hooks/refs editorRef.current = editor ?? null; // Show-invisibles toggle — flip the extension storage flag and diff --git a/src/components/McpHistory/McpHistoryButton.tsx b/src/components/McpHistory/McpHistoryButton.tsx index e77b38d3e..45354f572 100644 --- a/src/components/McpHistory/McpHistoryButton.tsx +++ b/src/components/McpHistory/McpHistoryButton.tsx @@ -23,7 +23,7 @@ * @module components/McpHistory/McpHistoryButton */ -import { useEffect, useRef, useState, useCallback } from "react"; +import { useEffect, useLayoutEffect, useRef, useState, useCallback } from "react"; import { History, Undo2, Trash2 } from "lucide-react"; import { useTranslation } from "react-i18next"; import { useMcpStore } from "@/stores/mcpStore"; @@ -139,18 +139,21 @@ export function McpHistoryButton(): React.ReactElement { toast.success(t("mcpHistoryCleared")); }, [tabId, tabFilePath, t]); - const popoverPosition = (): React.CSSProperties => { + // Measure the trigger and position the popover in a layout effect — reading + // the DOM rect during render is not concurrent-safe (#1063). Runs before paint + // when the popover opens, so there is no flicker. + const [popoverStyle, setPopoverStyle] = useState({ display: "none" }); + useLayoutEffect(() => { + if (!open) return; const rect = buttonRef.current?.getBoundingClientRect(); - if (!rect) return { display: "none" }; + if (!rect) { + setPopoverStyle({ display: "none" }); + return; + } const right = Math.max(8, window.innerWidth - rect.right); const bottom = Math.max(8, window.innerHeight - rect.top + 6); - return { - right, - bottom, - width: POPUP_WIDTH, - maxHeight: POPUP_MAX_HEIGHT, - }; - }; + setPopoverStyle({ right, bottom, width: POPUP_WIDTH, maxHeight: POPUP_MAX_HEIGHT }); + }, [open]); return ( <> @@ -172,7 +175,7 @@ export function McpHistoryButton(): React.ReactElement {
diff --git a/src/components/Sidebar/OutlineView.tsx b/src/components/Sidebar/OutlineView.tsx index 12e174388..ffed9da44 100644 --- a/src/components/Sidebar/OutlineView.tsx +++ b/src/components/Sidebar/OutlineView.tsx @@ -4,7 +4,7 @@ * Displays document heading structure as a tree with a substring filter. */ -import { memo, useState, useDeferredValue, useMemo, useRef, useCallback } from "react"; +import { memo, useState, useDeferredValue, useMemo, useCallback } from "react"; import { useTranslation } from "react-i18next"; import { ChevronRight, ChevronDown, Search, X } from "lucide-react"; import { emitTo } from "@tauri-apps/api/event"; @@ -17,7 +17,6 @@ import { buildHeadingTree, filterHeadingTree, getHeadingLinesKey, - type HeadingItem, type HeadingNode, } from "./outlineUtils"; @@ -119,24 +118,20 @@ export function OutlineView() { [deferredContent, isTooLarge] ); - // Cache previous headings to maintain referential stability - const prevHeadingsRef = useRef([]); - const prevKeyRef = useRef(""); - - // Only re-extract headings when heading lines actually change + // Re-extract headings only when the heading lines change. Keyed on + // headingLinesKey (not deferredContent) so edits that leave the heading lines + // untouched keep the same array reference — referential stability for + // downstream consumers, without reading/writing a cache ref during render + // (#1063). deferredContent is read inside but intentionally not a dep. const headings = useMemo(() => { if (isTooLarge) return []; - if (headingLinesKey === prevKeyRef.current) { - return prevHeadingsRef.current; - } perfStart("OutlineView:extractHeadings"); const extracted = extractHeadings(deferredContent); const newHeadings = extracted.length > MAX_HEADING_COUNT ? extracted.slice(0, MAX_HEADING_COUNT) : extracted; perfEnd("OutlineView:extractHeadings", { count: newHeadings.length }); - prevHeadingsRef.current = newHeadings; - prevKeyRef.current = headingLinesKey; return newHeadings; - }, [headingLinesKey, deferredContent, isTooLarge]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [headingLinesKey, isTooLarge]); const tree = useMemo(() => { if (isTooLarge) return []; diff --git a/src/components/StatusBar/WordCountPopover.tsx b/src/components/StatusBar/WordCountPopover.tsx index 1d23dd98b..7b605b7e7 100644 --- a/src/components/StatusBar/WordCountPopover.tsx +++ b/src/components/StatusBar/WordCountPopover.tsx @@ -11,7 +11,9 @@ * at the bottom of the window), mirroring McpHistoryButton's popover. * - Pure presentational: receives precomputed TextMetrics (totals + selected) * from StatusBarCounts, so strip+compute happens once at the source and the - * inline counts and this breakdown never diverge. + * inline counts and this breakdown never diverge. Self-positions via a layout + * effect (measuring the anchor before paint) rather than reading the anchor + * rect during render, which is not concurrent-safe. * - When a selection exists, each row shows "selected / total"; otherwise a * single total — matching the inline counts' selected/total convention. * - Open state and dismiss (outside click / Escape) are owned by @@ -24,6 +26,7 @@ // audit-fix — pure presentational; metrics + dismiss owned by StatusBarCounts import type { RefObject } from "react"; +import { useLayoutEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import type { TextMetrics } from "./statusTextMetrics"; import "./word-count-popover.css"; @@ -59,20 +62,22 @@ export function WordCountPopover({ }: WordCountPopoverProps): React.ReactElement { const { t } = useTranslation("statusbar"); - const position = (): React.CSSProperties => { + // Position above the anchor by measuring it in a layout effect (before paint), + // rather than reading the anchor rect during render — the latter is not + // concurrent-safe (#1063). + const [style, setStyle] = useState({ right: 8, bottom: 8, width: POPUP_WIDTH }); + useLayoutEffect(() => { const rect = anchorRef.current?.getBoundingClientRect(); - if (!rect) { - return { right: 8, bottom: 8, width: POPUP_WIDTH }; - } + if (!rect) return; const right = Math.max(8, window.innerWidth - rect.right); const bottom = Math.max(8, window.innerHeight - rect.top + 6); - return { right, bottom, width: POPUP_WIDTH }; - }; + setStyle({ right, bottom, width: POPUP_WIDTH }); + }, [anchorRef]); return (
diff --git a/src/components/Terminal/useTerminalSessions.ts b/src/components/Terminal/useTerminalSessions.ts index 2a81d6cf2..60708194d 100644 --- a/src/components/Terminal/useTerminalSessions.ts +++ b/src/components/Terminal/useTerminalSessions.ts @@ -69,7 +69,10 @@ export function useTerminalSessions( // The callbacks object is a new literal each render, but the individual // functions (onSearch) are stable useCallbacks from the parent. const callbacksRef = useRef(callbacks); - callbacksRef.current = callbacks; + // Synced after commit (read only from terminal event handlers). #1063 + useEffect(() => { + callbacksRef.current = callbacks; + }); // Debounce PTY resize to avoid excessive resize calls during drag const resizeTimerRef = useRef | undefined>(undefined); diff --git a/src/hooks/useContentServer.ts b/src/hooks/useContentServer.ts index 825292bc8..ea327e3d9 100644 --- a/src/hooks/useContentServer.ts +++ b/src/hooks/useContentServer.ts @@ -202,7 +202,10 @@ export function useContentServer(): ContentServerControls { // startServer is referenced via a ref so the listener never goes stale and // does not need to re-subscribe on every render. const startServerRef = useRef(startServer); - startServerRef.current = startServer; + // Synced after commit (read only from the async crash listener below). #1063 + useEffect(() => { + startServerRef.current = startServer; + }); useEffect(() => { let unlisten: UnlistenFn | undefined; let disposed = false; diff --git a/src/hooks/useGenieInvocation.ts b/src/hooks/useGenieInvocation.ts index 284d57359..b1866185b 100644 --- a/src/hooks/useGenieInvocation.ts +++ b/src/hooks/useGenieInvocation.ts @@ -487,8 +487,11 @@ export function useGenieInvocation() { [runGenie] ); - // Keep ref in sync for MCP bridge listener - invokeGenieRef.current = invokeGenie; + // Keep ref in sync for MCP bridge listener — synced after commit (read only + // from the async bridge listener, never during render). #1063 + useEffect(() => { + invokeGenieRef.current = invokeGenie; + }); const invokeFreeform = useCallback( async (userPrompt: string, scope: GenieScope) => { diff --git a/src/hooks/useTabDragOut.ts b/src/hooks/useTabDragOut.ts index 47e8c5045..efab2f328 100644 --- a/src/hooks/useTabDragOut.ts +++ b/src/hooks/useTabDragOut.ts @@ -15,7 +15,7 @@ * @module hooks/useTabDragOut */ -import { useCallback, useRef, useState, type PointerEvent as ReactPointerEvent, type RefObject } from "react"; +import { useCallback, useEffect, useRef, useState, type PointerEvent as ReactPointerEvent, type RefObject } from "react"; /** Vertical distance (px) outside the tab bar to trigger drag-out. */ const DRAG_OUT_THRESHOLD = 40; @@ -151,15 +151,18 @@ export function useTabDragOut({ tabBarRef, onDragOut, onReorder, onDragMove }: U } }, []); - // Stable refs for callbacks used in document listeners + // Stable refs for callbacks used in document listeners — synced after commit + // (read only from the document drag listeners, never during render). #1063 const onDragOutRef = useRef(onDragOut); - onDragOutRef.current = onDragOut; const onReorderRef = useRef(onReorder); - onReorderRef.current = onReorder; const onDragMoveRef = useRef(onDragMove); - onDragMoveRef.current = onDragMove; const stableBarRef = useRef(tabBarRef); - stableBarRef.current = tabBarRef; + useEffect(() => { + onDragOutRef.current = onDragOut; + onReorderRef.current = onReorder; + onDragMoveRef.current = onDragMove; + stableBarRef.current = tabBarRef; + }); // Detach document listeners and reset state /* v8 ignore start -- @preserve reason: cleanupRef empty-function initializer and reset are uncovered; drag cleanup not triggered in unit tests */ From 1549d8612cf3afbcccac5c2d9fa5065b264b545f Mon Sep 17 00:00:00 2001 From: xiaolai Date: Sat, 27 Jun 2026 16:58:28 +0800 Subject: [PATCH 4/6] chore(lint): adopt react-hooks/set-state-in-effect rule (#1063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve all 33 sites across 25 files, completing the react-hooks-7 adoption. Genuine 'adjust state during render' cases refactored to render-phase setState (React's recommended alternative — no extra render, no stale frame): - selection clamps on list shrink (QuickOpen, GeniePicker, HeadingPicker) - reset-selection-on-query (CommandPalette, via prev-value tracking) - xterm activation latch (TerminalPanel) - dev-section/section guards (Settings, AboutSettings) - mount focus init via initial state (FileExplorer ContextMenu) Legitimate effect-bound cases carry a scoped disable with a per-site reason — async I/O loads (KbGraphView, HistoryView, useMcpClients/Server, useActionMetadata, mermaid, PdfExportPage, settings loaders), timers (useAutoSaveDisplay), DOM measurement (HeadingPicker position/portal), external-event sync (GeniePicker prompt history, UniversalToolbar store/focus, useStatusBarTabDrag), and open/close + focus-reset transitions (CommandPalette, GeniePicker, QuickOpen, ImageContextMenu, TabContextMenu). The four React-Compiler rules now inherit 'error' from v7 recommended; the deferral block in eslint.config.js is replaced with the adoption rationale. --- eslint.config.js | 23 ++++++++--------- .../CommandPalette/CommandPalette.tsx | 19 ++++++++++---- src/components/Editor/HeadingPicker.tsx | 25 ++++++++++++------- src/components/Editor/ImageContextMenu.tsx | 5 +++- .../UniversalToolbar/UniversalToolbar.tsx | 14 ++++++++--- .../WorkflowEditor/useActionMetadata.ts | 5 ++++ src/components/GeniePicker/GeniePicker.tsx | 24 ++++++++++++------ .../KnowledgeBasePanel/KbGraphView.tsx | 5 ++++ src/components/QuickOpen/QuickOpen.tsx | 21 ++++++++++------ .../Sidebar/FileExplorer/ContextMenu.tsx | 9 +++---- .../Sidebar/FileExplorer/useFileTree.ts | 3 +++ src/components/Sidebar/HistoryView.tsx | 3 +++ .../StatusBar/useAutoSaveDisplay.ts | 5 ++++ .../StatusBar/useStatusBarTabDrag.ts | 3 +++ src/components/Tabs/TabContextMenu.tsx | 5 ++++ src/components/Terminal/TerminalPanel.tsx | 8 +++--- src/hooks/useMcpClients.ts | 3 +++ src/hooks/useMcpServer.ts | 4 +++ src/lib/formats/adapters/mermaid.tsx | 5 ++++ src/lib/ghaWorkflow/render/snapshotRoot.tsx | 4 +++ src/pages/PdfExportPage.tsx | 5 +++- src/pages/Settings.tsx | 11 ++++---- src/pages/settings/AboutSettings.tsx | 12 ++++----- src/pages/settings/DocumentToolsSettings.tsx | 3 +++ src/pages/settings/IntegrationsSettings.tsx | 3 +++ src/pages/settings/McpConfigInstaller.tsx | 3 +++ 26 files changed, 160 insertions(+), 70 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 2c1316d69..e9394e66e 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -17,19 +17,16 @@ export default tseslint.config( }, rules: { ...reactHooks.configs.recommended.rules, - // eslint-plugin-react-hooks v7 folded the React Compiler rule set into - // `recommended`, which flags 67 pre-existing sites (set-state-in-effect, - // ref access during render, manual-memoization, immutability). Adopting - // them is a deliberate, codebase-wide refactor — not part of a version - // bump — so they are deferred here to preserve the enforcement level the - // code was written and verified against. Re-enable incrementally (per - // rule, file-scoped) in a dedicated react-hooks-7 adoption pass — see #1063. - "react-hooks/set-state-in-effect": "off", - "react-hooks/refs": "error", - "react-hooks/preserve-manual-memoization": "error", - "react-hooks/immutability": "error", - // Historically `warn` under v5's recommended; v7 raised it to error. - // Keep it non-blocking to match prior behavior. + // The React Compiler rule set (folded into `recommended` by + // eslint-plugin-react-hooks v7) is fully adopted — #1063. Genuine + // render-derivable cases were refactored (adjust-state-during-render, layout + // effects, re-keyed memos); legitimate effect-bound cases (async I/O, timers, + // DOM measurement, external-event sync, open/close transitions) carry a + // scoped disable with a per-site reason. These stay at `error` so new + // violations are caught at the source. + // `exhaustive-deps` was historically `warn` under v5's recommended; v7 raised + // it to error. Kept non-blocking to match prior behavior — its ~67 sites are + // out of scope for #1063. "react-hooks/exhaustive-deps": "warn", "@typescript-eslint/no-unused-vars": [ "error", diff --git a/src/components/CommandPalette/CommandPalette.tsx b/src/components/CommandPalette/CommandPalette.tsx index 9167223f9..c004ed65d 100644 --- a/src/components/CommandPalette/CommandPalette.tsx +++ b/src/components/CommandPalette/CommandPalette.tsx @@ -40,6 +40,7 @@ export function CommandPalette() { const close = useCommandPaletteStore((s) => s.close); const [query, setQuery] = useState(""); const [selectedIndex, setSelectedIndex] = useState(0); + const [prevQuery, setPrevQuery] = useState(query); const inputRef = useRef(null); const previousFocusRef = useRef(null); @@ -48,7 +49,18 @@ export function CommandPalette() { [isOpen, query], ); - // Reset and focus on open; restore previous focus on close (a11y). + // Reset the highlighted row to the top whenever the query changes — adjusted + // during render (React's recommended alternative to a setState-in-effect, which + // would cost an extra render per keystroke). #1063 + if (query !== prevQuery) { + setPrevQuery(query); + setSelectedIndex(0); + } + + // Reset and focus on open; restore previous focus on close (a11y). Legitimate + // setState-in-effect: bound to the open/close transition and bundled with focus + // capture/restore + RAF focus, not derivable during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (isOpen) { previousFocusRef.current = document.activeElement; @@ -63,10 +75,7 @@ export function CommandPalette() { previousFocusRef.current = null; } }, [isOpen]); - - useEffect(() => { - setSelectedIndex(0); - }, [query]); + /* eslint-enable react-hooks/set-state-in-effect */ if (!isOpen) return null; diff --git a/src/components/Editor/HeadingPicker.tsx b/src/components/Editor/HeadingPicker.tsx index dd4e7734f..113847df2 100644 --- a/src/components/Editor/HeadingPicker.tsx +++ b/src/components/Editor/HeadingPicker.tsx @@ -62,9 +62,12 @@ export function HeadingPicker() { const previousFocusRef = useRef(null); const [portalTarget, setPortalTarget] = useState(null); - // Find editor container for portal mounting + // Find editor container for portal mounting. Legitimate setState-in-effect: the + // target is read from the DOM after mount, so it can't be resolved during + // render (#1063). useEffect(() => { const editorContainer = document.querySelector('.editor-container') as HTMLElement | null; + // eslint-disable-next-line react-hooks/set-state-in-effect setPortalTarget(editorContainer); }, []); @@ -156,7 +159,10 @@ export function HeadingPicker() { capture: false, }); - // Calculate popup position when opening + // Calculate popup position when opening. Legitimate setState-in-effect: depends + // on DOM measurement (portalTarget.getBoundingClientRect) that is only valid + // after layout, not during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (!isOpen) return; @@ -190,14 +196,15 @@ export function HeadingPicker() { setPosition({ top, left }); } }, [isOpen, anchorRect, containerBounds, portalTarget]); + /* eslint-enable react-hooks/set-state-in-effect */ - // Reset and clamp selection when filter changes - useEffect(() => { - setSelectedIndex((prev) => { - if (filteredHeadings.length === 0) return 0; - return Math.min(prev, filteredHeadings.length - 1); - }); - }, [filter, filteredHeadings.length]); + // Clamp the selection when the filtered list shrinks. Adjusted during render + // (converges immediately) rather than in an effect (#1063). + if (filteredHeadings.length === 0) { + if (selectedIndex !== 0) setSelectedIndex(0); + } else if (selectedIndex > filteredHeadings.length - 1) { + setSelectedIndex(filteredHeadings.length - 1); + } // Scroll selected item into view useEffect(() => { diff --git a/src/components/Editor/ImageContextMenu.tsx b/src/components/Editor/ImageContextMenu.tsx index 1cb886415..5ebdf00a2 100644 --- a/src/components/Editor/ImageContextMenu.tsx +++ b/src/components/Editor/ImageContextMenu.tsx @@ -125,8 +125,11 @@ export function ImageContextMenu({ onAction }: ImageContextMenuProps) { menu.style.top = `${adjustedY}px`; }, [position]); - // Focus the first item whenever the menu opens. + // Focus the first item whenever the menu opens (reset to -1 on close). + // Legitimate setState-in-effect: resets the roving-focus index on the open/close + // transition (including on mount), paired with the DOM-focus effect below (#1063). useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect setFocusedIndex(isOpen ? 0 : -1); }, [isOpen]); diff --git a/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx b/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx index 40bc504f5..faa1b00db 100644 --- a/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx +++ b/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx @@ -334,15 +334,19 @@ export function UniversalToolbar() { // Sync with store's dropdown state (for global Escape handling) useEffect(() => { - // Store says dropdown should be closed, but local state says open + // Store says dropdown should be closed, but local state says open. + // Legitimate: reacts to the external store's dropdown state (#1063). if (!storeDropdownOpen && menuOpen) { + // eslint-disable-next-line react-hooks/set-state-in-effect closeMenu(); } }, [storeDropdownOpen, menuOpen, closeMenu]); - // Close dropdown when focus leaves toolbar (focus toggle) + // Close dropdown when focus leaves toolbar (focus toggle). Legitimate: reacts + // to the external toolbar-focus signal (#1063). useEffect(() => { if (!toolbarHasFocus && menuOpen) { + // eslint-disable-next-line react-hooks/set-state-in-effect closeMenu(false); } }, [toolbarHasFocus, menuOpen, closeMenu]); @@ -359,7 +363,10 @@ export function UniversalToolbar() { } }, [visible, toolbarHasFocus, focusActiveEditor]); - // Handle toolbar open/close and initial focus + // Handle toolbar open/close and initial focus. Legitimate setState-in-effect: + // reacts to the external visibility toggle and seeds keyboard focus from + // session memory / button states — not derivable during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (!visible) { wasVisibleRef.current = false; @@ -389,6 +396,7 @@ export function UniversalToolbar() { wasVisibleRef.current = true; }, [visible, buttonStates, setFocusedIndex, closeMenu, sessionFocusIndex, tDialog]); + /* eslint-enable react-hooks/set-state-in-effect */ // Handle click outside dropdown useEffect(() => { diff --git a/src/components/Editor/WorkflowEditor/useActionMetadata.ts b/src/components/Editor/WorkflowEditor/useActionMetadata.ts index 8731c412b..d721c30ce 100644 --- a/src/components/Editor/WorkflowEditor/useActionMetadata.ts +++ b/src/components/Editor/WorkflowEditor/useActionMetadata.ts @@ -101,6 +101,10 @@ export function useActionMetadata( : { state: "idle" }, ); + // Legitimate setState-in-effect: transitions to loading then resolves from an + // async metadata fetch (with a mounted guard) — driven by I/O keyed on `uses`, + // not derivable during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (!uses || !isResolvableRef(uses)) { setResult({ state: "idle" }); @@ -132,6 +136,7 @@ export function useActionMetadata( mounted = false; }; }, [uses, isLocalCtx]); + /* eslint-enable react-hooks/set-state-in-effect */ return result; } diff --git a/src/components/GeniePicker/GeniePicker.tsx b/src/components/GeniePicker/GeniePicker.tsx index 7f247516b..ba2aa0be8 100644 --- a/src/components/GeniePicker/GeniePicker.tsx +++ b/src/components/GeniePicker/GeniePicker.tsx @@ -83,7 +83,11 @@ export function GeniePicker() { // Prompt history hook (pass grace-period guard for freeform keyDown) const promptHistory = usePromptHistory(ime.isComposing); - // Load genies on open + reset history hook + // Load genies on open + reset history hook. Legitimate setState-in-effect: the + // resets are bound to the open/close transition and bundled with side effects + // (genie load, focus capture/restore, prompt-history reset) — not derivable + // during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { /* v8 ignore next -- @preserve reason: false branch (close path with focus restore) untestable in jsdom */ if (isOpen) { @@ -105,6 +109,7 @@ export function GeniePicker() { /* v8 ignore stop */ // eslint-disable-next-line react-hooks/exhaustive-deps }, [isOpen, filterScope]); + /* eslint-enable react-hooks/set-state-in-effect */ // Focus search input on open useEffect(() => { @@ -167,13 +172,12 @@ export function GeniePicker() { return items; }, [recents, grouped]); - // Clamp selectedIndex when flatList shrinks (e.g. after typing narrows results) - useEffect(() => { - /* v8 ignore next 2 -- @preserve reason: clamp fires only when flatList shrinks below selectedIndex; race condition untestable in jsdom */ - if (flatList.length > 0 && selectedIndex >= flatList.length) { - setSelectedIndex(flatList.length - 1); - } - }, [selectedIndex, flatList.length]); + // Clamp selectedIndex when flatList shrinks (e.g. after typing narrows results). + // Adjusted during render — React's recommended alternative to a setState-in- + // effect, which would flash an out-of-range selection for a frame (#1063). + if (flatList.length > 0 && selectedIndex >= flatList.length) { + setSelectedIndex(flatList.length - 1); + } const handleClose = useCallback(() => { useGeniePickerStore.getState().closePicker(); @@ -291,12 +295,16 @@ export function GeniePicker() { // When cycling changes displayValue, push it into filter so the textarea updates. // Safe from loops: typing sets displayValue === filter via handleChange, so the // guard (displayValue !== filter) is only true when cycling produces a new value. + // Legitimate setState-in-effect: reacts to external prompt-history cycling, not + // a value derivable from this render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (flatList.length === 0 && promptHistory.displayValue !== filter) { setFilter(promptHistory.displayValue); setSelectedIndex(0); } }, [promptHistory.displayValue, filter, flatList.length]); + /* eslint-enable react-hooks/set-state-in-effect */ // Click outside to close. Escape is handled by the component's own // onKeyDown (mode-aware: resetToInput in processing/preview/error, diff --git a/src/components/KnowledgeBasePanel/KbGraphView.tsx b/src/components/KnowledgeBasePanel/KbGraphView.tsx index e0a3a414c..bf932a6e2 100644 --- a/src/components/KnowledgeBasePanel/KbGraphView.tsx +++ b/src/components/KnowledgeBasePanel/KbGraphView.tsx @@ -24,6 +24,10 @@ export function KbGraphView() { const [flow, setFlow] = useState(null); const [error, setError] = useState(null); + // Legitimate setState-in-effect: resets to a loading state then fills from an + // async graph fetch (with cancellation) — driven by I/O, not derivable during + // render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { let cancelled = false; setFlow(null); @@ -43,6 +47,7 @@ export function KbGraphView() { cancelled = true; }; }, [root, t]); + /* eslint-enable react-hooks/set-state-in-effect */ if (error) return
{error}
; if (!flow) return
; diff --git a/src/components/QuickOpen/QuickOpen.tsx b/src/components/QuickOpen/QuickOpen.tsx index c95b393eb..6588a8421 100644 --- a/src/components/QuickOpen/QuickOpen.tsx +++ b/src/components/QuickOpen/QuickOpen.tsx @@ -87,16 +87,20 @@ export function QuickOpen({ windowLabel }: QuickOpenProps) { // Total count including Browse row const totalCount = rankedItems.length + 1; // +1 for Browse - // Clamp selectedIndex when ranked list shrinks (e.g. after typing narrows results) - useEffect(() => { - /* v8 ignore next 2 -- @preserve reason: clamp fires only when totalCount shrinks below selectedIndex; effect timing makes it unreliable in jsdom */ - if (selectedIndex >= totalCount) { - setSelectedIndex(Math.max(0, totalCount - 1)); - } - }, [selectedIndex, totalCount]); + // Clamp selectedIndex when the ranked list shrinks (e.g. after typing narrows + // results). Adjusted during render — React's recommended alternative to a + // setState-in-effect, which would flash an out-of-range selection for a frame + // and cost an extra render (#1063). + if (selectedIndex >= totalCount) { + setSelectedIndex(Math.max(0, totalCount - 1)); + } // Reset state on open — bump revision to rebuild items from fresh store state - // Save previous focus for restoration on close + // Save previous focus for restoration on close. Legitimate setState-in-effect: + // the resets are bound to the open/close transition and bundled with real side + // effects (focus capture/restore, RAF focus, picker close), so they can't be + // derived during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { /* v8 ignore next -- @preserve reason: false branch (close path) restores focus; jsdom focus tracking unreliable */ if (isOpen) { @@ -114,6 +118,7 @@ export function QuickOpen({ windowLabel }: QuickOpenProps) { } /* v8 ignore stop */ }, [isOpen]); + /* eslint-enable react-hooks/set-state-in-effect */ const handleClose = useCallback(() => { useQuickOpenStore.getState().close(); diff --git a/src/components/Sidebar/FileExplorer/ContextMenu.tsx b/src/components/Sidebar/FileExplorer/ContextMenu.tsx index 02cb3f8a8..fe326a282 100644 --- a/src/components/Sidebar/FileExplorer/ContextMenu.tsx +++ b/src/components/Sidebar/FileExplorer/ContextMenu.tsx @@ -124,7 +124,9 @@ export function ContextMenu({ type, position, onAction, onClose }: ContextMenuPr const { t } = useTranslation("sidebar"); const menuRef = useRef(null); const itemRefs = useRef>([]); - const [focusedIndex, setFocusedIndex] = useState(-1); + // Start focused on the first item — the menu always opens with item 0 focused. + // Using the initial state instead of a mount effect avoids an extra render (#1063). + const [focusedIndex, setFocusedIndex] = useState(0); // Resolve platform-appropriate "reveal in file manager" label via translation keys. // The React Compiler auto-memoizes the component, so no manual useMemo is needed — @@ -174,11 +176,6 @@ export function ContextMenu({ type, position, onAction, onClose }: ContextMenuPr menu.style.top = `${adjustedY}px`; }, [position]); - // Auto-focus first item on mount - useEffect(() => { - setFocusedIndex(0); - }, []); - // Move DOM focus when focusedIndex changes useEffect(() => { if (focusedIndex < 0) return; diff --git a/src/components/Sidebar/FileExplorer/useFileTree.ts b/src/components/Sidebar/FileExplorer/useFileTree.ts index e120c2c34..9202de240 100644 --- a/src/components/Sidebar/FileExplorer/useFileTree.ts +++ b/src/components/Sidebar/FileExplorer/useFileTree.ts @@ -168,6 +168,9 @@ export function useFileTree( // Load tree and setup watcher when rootPath changes useEffect(() => { if (!rootPath) { + // Legitimate: clears the tree as part of an async load + fs-watcher setup + // keyed on rootPath, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect setTree([]); return; } diff --git a/src/components/Sidebar/HistoryView.tsx b/src/components/Sidebar/HistoryView.tsx index bde0d3c44..4b5906938 100644 --- a/src/components/Sidebar/HistoryView.tsx +++ b/src/components/Sidebar/HistoryView.tsx @@ -42,6 +42,9 @@ export function HistoryView() { const currentRequestId = ++requestIdRef.current; if (!filePath || !historyEnabled) { + // Legitimate: clears the list as part of a cancellable async fetch keyed on + // filePath, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect setSnapshots([]); return; } diff --git a/src/components/StatusBar/useAutoSaveDisplay.ts b/src/components/StatusBar/useAutoSaveDisplay.ts index 67b60d7dd..ff83dd3e6 100644 --- a/src/components/StatusBar/useAutoSaveDisplay.ts +++ b/src/components/StatusBar/useAutoSaveDisplay.ts @@ -27,6 +27,10 @@ export function useAutoSaveDisplay( const [showAutoSave, setShowAutoSave] = useState(false); const [autoSaveTime, setAutoSaveTime] = useState(""); + // Legitimate setState-in-effect: shows the "auto-saved" badge in response to a + // new save timestamp and refreshes the relative time on a timer — driven by an + // external event + timers, not derivable during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (!lastAutoSave) return; @@ -46,6 +50,7 @@ export function useAutoSaveDisplay( clearTimeout(fadeTimeout); }; }, [lastAutoSave]); + /* eslint-enable react-hooks/set-state-in-effect */ return { showAutoSave, autoSaveTime }; } diff --git a/src/components/StatusBar/useStatusBarTabDrag.ts b/src/components/StatusBar/useStatusBarTabDrag.ts index 7dbb20d46..0675d3a31 100644 --- a/src/components/StatusBar/useStatusBarTabDrag.ts +++ b/src/components/StatusBar/useStatusBarTabDrag.ts @@ -286,6 +286,9 @@ export function useStatusBarTabDrag({ tabs, windowLabel, tabBarRef, onActivateTa if (dragMode !== "idle") return; // Advance generation so in-flight probe responses are discarded dragGenerationRef.current++; + // Legitimate: clears the cross-window drop-preview broadcast when the drag + // returns to idle — a transition side effect, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect clearDropPreviewBroadcast(); }, [clearDropPreviewBroadcast, dragMode]); diff --git a/src/components/Tabs/TabContextMenu.tsx b/src/components/Tabs/TabContextMenu.tsx index 478e2e9f6..1fd65b48c 100644 --- a/src/components/Tabs/TabContextMenu.tsx +++ b/src/components/Tabs/TabContextMenu.tsx @@ -107,6 +107,7 @@ export function TabContextMenu({ tab, position, windowLabel, onClose }: TabConte [menuItems] ); + const applyMenuPosition = useCallback(() => { const menu = menuRef.current; /* v8 ignore next -- @preserve reason: menu is null only before mount; always exists when applyMenuPosition is called */ @@ -155,8 +156,12 @@ export function TabContextMenu({ tab, position, windowLabel, onClose }: TabConte // Close on click outside or Escape (Escape ignored during IME composition). useDismissOnOutsideOrEscape(true, menuRef, onClose); + // Reset focus to the first enabled item when the focusable set changes (incl. + // on mount). Legitimate setState-in-effect: paired with the DOM-focus effect + // below; not derivable during render without losing mount-time focus init (#1063). useEffect(() => { /* v8 ignore next -- @preserve reason: ?? -1 fallback only when focusableIndices is empty; menu always has enabled items in tests */ + // eslint-disable-next-line react-hooks/set-state-in-effect setFocusedIndex(focusableIndices[0] ?? -1); }, [focusableIndices]); diff --git a/src/components/Terminal/TerminalPanel.tsx b/src/components/Terminal/TerminalPanel.tsx index f25e228e8..3dca68a08 100644 --- a/src/components/Terminal/TerminalPanel.tsx +++ b/src/components/Terminal/TerminalPanel.tsx @@ -56,11 +56,11 @@ export function TerminalPanel() { const position = useUIStore((s) => s.effectiveTerminalPosition); const containerRef = useRef(null); - // Defer xterm init until first show + // Defer xterm init until first show — latch once visible. Adjusted during + // render (a one-way latch that converges immediately) rather than in an effect, + // avoiding an extra render before xterm mounts (#1063). const [activated, setActivated] = useState(false); - useEffect(() => { - if (visible && !activated) setActivated(true); - }, [visible, activated]); + if (visible && !activated) setActivated(true); // Search bar state const [searchVisible, setSearchVisible] = useState(false); diff --git a/src/hooks/useMcpClients.ts b/src/hooks/useMcpClients.ts index be0dc97e5..f08264a88 100644 --- a/src/hooks/useMcpClients.ts +++ b/src/hooks/useMcpClients.ts @@ -31,6 +31,9 @@ export function useMcpClients(mcpRunning: boolean): McpClient[] { useEffect(() => { if (!mcpRunning) { + // Legitimate: clears the list as part of a cancellable async fetch gated on + // mcpRunning, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect setClients([]); return; } diff --git a/src/hooks/useMcpServer.ts b/src/hooks/useMcpServer.ts index 484097208..3c558bea9 100644 --- a/src/hooks/useMcpServer.ts +++ b/src/hooks/useMcpServer.ts @@ -120,6 +120,10 @@ export function useMcpServer(): UseMcpServerResult { // Subscribe to server events useEffect(() => { + // Legitimate: refresh() fetches current server state (async, sets loading/ + // running) on mount, then we subscribe to live events — driven by I/O and + // external events, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect refresh(); const unlistenStarted = listen("mcp-server:started", () => { diff --git a/src/lib/formats/adapters/mermaid.tsx b/src/lib/formats/adapters/mermaid.tsx index 5a4666a98..1f7da94a4 100644 --- a/src/lib/formats/adapters/mermaid.tsx +++ b/src/lib/formats/adapters/mermaid.tsx @@ -108,6 +108,10 @@ function MermaidPreview({ content, diagnostics }: PreviewRendererProps) { const containerRef = useRef(null); const renderToken = useRef(0); + // Legitimate setState-in-effect: clears then fills from an async Mermaid render + // (token-guarded against rapid edits) — driven by I/O keyed on content, not + // derivable during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (!content.trim()) { setSvg(null); @@ -143,6 +147,7 @@ function MermaidPreview({ content, diagnostics }: PreviewRendererProps) { cancelled = true; }; }, [content]); + /* eslint-enable react-hooks/set-state-in-effect */ const showInvalid = useMemo( () => renderError !== null || svg === null, diff --git a/src/lib/ghaWorkflow/render/snapshotRoot.tsx b/src/lib/ghaWorkflow/render/snapshotRoot.tsx index e0d6164a6..bb6c0e501 100644 --- a/src/lib/ghaWorkflow/render/snapshotRoot.tsx +++ b/src/lib/ghaWorkflow/render/snapshotRoot.tsx @@ -72,6 +72,10 @@ function SnapshotCanvas({ payload }: SnapshotCanvasProps): ReactElement | null { useEffect(() => { if (!payload) return; + // Legitimate: bumps a key to drive the render→measure→snapshot cycle when a + // new payload arrives — a side effect of the export request, not derivable + // during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect setReadyKey((k) => k + 1); }, [payload]); diff --git a/src/pages/PdfExportPage.tsx b/src/pages/PdfExportPage.tsx index 1c43b15f2..cde488694 100644 --- a/src/pages/PdfExportPage.tsx +++ b/src/pages/PdfExportPage.tsx @@ -45,7 +45,9 @@ export function PdfExportPage() { useTheme(); usePdfExportClose(); - // Load HTML from temp file on mount + // Load HTML from temp file on mount. Legitimate setState-in-effect: reads URL + // params and an async temp file — I/O on mount, not derivable during render (#1063). + /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { const params = new URLSearchParams(window.location.search); const htmlPath = params.get("htmlPath"); @@ -65,6 +67,7 @@ export function PdfExportPage() { setError(t("dialog:pdfExport.loadFailed", { error: msg })); }); }, [t]); + /* eslint-enable react-hooks/set-state-in-effect */ const handleClose = async () => { const currentWindow = getCurrentWebviewWindow(); diff --git a/src/pages/Settings.tsx b/src/pages/Settings.tsx index cce194edc..035de73ce 100644 --- a/src/pages/Settings.tsx +++ b/src/pages/Settings.tsx @@ -167,12 +167,11 @@ export function SettingsPage() { }; }, []); - // Switch to appearance when dev sections are hidden while viewing them - useEffect(() => { - if (!showDevSection && section === "advanced") { - setSection("appearance"); - } - }, [showDevSection, section]); + // Switch to appearance when dev sections are hidden while viewing them. + // Adjusted during render (converges immediately) rather than in an effect (#1063). + if (!showDevSection && section === "advanced") { + setSection("appearance"); + } const navItems = [ ...navConfig diff --git a/src/pages/settings/AboutSettings.tsx b/src/pages/settings/AboutSettings.tsx index 3c1c7d095..669eaa90f 100644 --- a/src/pages/settings/AboutSettings.tsx +++ b/src/pages/settings/AboutSettings.tsx @@ -185,12 +185,12 @@ function UpdateAvailableCard() { }; }, []); - // Reset isDownloading when status changes away from downloading - useEffect(() => { - if (status !== "downloading") { - setIsDownloading(false); - } - }, [status]); + // Reset isDownloading when status changes away from downloading. Adjusted + // during render (guarded so it converges immediately) rather than in an + // effect (#1063). + if (status !== "downloading" && isDownloading) { + setIsDownloading(false); + } if (!updateInfo) return null; diff --git a/src/pages/settings/DocumentToolsSettings.tsx b/src/pages/settings/DocumentToolsSettings.tsx index a1b298173..d6cc513fe 100644 --- a/src/pages/settings/DocumentToolsSettings.tsx +++ b/src/pages/settings/DocumentToolsSettings.tsx @@ -66,6 +66,9 @@ export function DocumentToolsSettings() { // Auto-detect on mount (no menu refresh — menu was built with correct state at startup). useEffect(() => { + // Legitimate: detect() runs an async tool probe that sets detection state — + // I/O on mount, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect void detect(false); return () => { mountedRef.current = false; }; }, [detect]); diff --git a/src/pages/settings/IntegrationsSettings.tsx b/src/pages/settings/IntegrationsSettings.tsx index ffe083d31..a903a35c5 100644 --- a/src/pages/settings/IntegrationsSettings.tsx +++ b/src/pages/settings/IntegrationsSettings.tsx @@ -62,6 +62,9 @@ export function IntegrationsSettings() { // Fetch client count when bridge is running useEffect(() => { if (!running) { + // Legitimate: resets the count as part of an async poll gated on `running`, + // not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect setClientCount(0); return; } diff --git a/src/pages/settings/McpConfigInstaller.tsx b/src/pages/settings/McpConfigInstaller.tsx index bac219067..76e8f8a93 100644 --- a/src/pages/settings/McpConfigInstaller.tsx +++ b/src/pages/settings/McpConfigInstaller.tsx @@ -162,6 +162,9 @@ export function McpConfigInstaller({ onInstallSuccess }: McpConfigInstallerProps }, []); useEffect(() => { + // Legitimate: loadDiagnostics() runs an async provider probe that sets + // diagnostics state on mount — I/O, not derivable during render (#1063). + // eslint-disable-next-line react-hooks/set-state-in-effect loadDiagnostics(); }, [loadDiagnostics]); From 41c1ed8508fa833d4b2549c19a0542b571e11c1f Mon Sep 17 00:00:00 2001 From: xiaolai Date: Sat, 27 Jun 2026 17:21:50 +0800 Subject: [PATCH 5/6] chore(lint): keep #1063-edited files within file-size baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The react-hooks-7 adoption added scoped-disable comments and effect/refactor scaffolding to several baselined files, pushing them past their frozen size. Reclaim the lines by condensing comments (and using inline-reason single-line disables for single-ref/single-setState sites) — no behavior change. --- src/components/Editor/SourceEditor.tsx | 15 ++---- src/components/Editor/TiptapEditor.tsx | 46 ++++++------------- .../UniversalToolbar/UniversalToolbar.tsx | 34 ++++++-------- src/components/GeniePicker/GeniePicker.tsx | 27 ++++------- .../StatusBar/useStatusBarTabDrag.ts | 7 +-- src/hooks/useGenieInvocation.ts | 7 +-- src/hooks/useTabDragOut.ts | 11 ++--- src/pages/settings/IntegrationsSettings.tsx | 7 +-- src/pages/settings/McpConfigInstaller.tsx | 7 +-- 9 files changed, 54 insertions(+), 107 deletions(-) diff --git a/src/components/Editor/SourceEditor.tsx b/src/components/Editor/SourceEditor.tsx index 70180802c..55e0908bb 100644 --- a/src/components/Editor/SourceEditor.tsx +++ b/src/components/Editor/SourceEditor.tsx @@ -79,9 +79,7 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP const setCursorInfoRef = useRef(setCursorInfo); const setSelectedTextRef = useRef(setSelectedText); const cursorInfoRef = useRef(cursorInfo); - // Keep "latest value" refs in sync after each commit. All are read only from - // the CodeMirror listener / effects (never during render), so a post-commit - // effect is the React-recommended, concurrent-safe pattern. See #1063. + // Sync "latest value" refs after commit (read only from the CodeMirror listener/effects) — concurrent-safe (#1063). useEffect(() => { hiddenRef.current = hidden; setContentRef.current = setContent; @@ -107,10 +105,8 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP enabled: !hidden, }); - // Reset parent scroll when source editor mounts or becomes visible. - // .editor-content retains its scrollTop from WYSIWYG mode even after - // overflow switches to hidden, causing the source editor to appear - // displaced (content at bottom instead of top). + // Reset parent scroll on mount/show: .editor-content keeps its WYSIWYG scrollTop + // after overflow flips to hidden, displacing the source editor's content. useEffect(() => { const editorContent = containerRef.current?.closest(".editor-content") as HTMLElement | null; if (editorContent && !hidden) { @@ -118,9 +114,8 @@ export function SourceEditor({ hidden = false, readOnly = false }: SourceEditorP } }, [hidden]); - // Clear shared selectedText when this editor becomes hidden — keeps the - // status bar from showing this editor's last selection while the other - // editor (WYSIWYG mode) is active. + // Clear shared selectedText when hidden — keeps the status bar from showing this + // editor's last selection while the WYSIWYG editor is active. useEffect(() => { if (hidden) setSelectedTextRef.current(""); }, [hidden]); diff --git a/src/components/Editor/TiptapEditor.tsx b/src/components/Editor/TiptapEditor.tsx index 58cca9867..49f528254 100644 --- a/src/components/Editor/TiptapEditor.tsx +++ b/src/components/Editor/TiptapEditor.tsx @@ -180,10 +180,7 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = const contentRef = useRef(content); const editorRef = useRef(null); const flushToStoreRef = useRef<((editor: TiptapEditor) => void) | null>(null); - // Keep "latest value" refs in sync after each commit. These are read only from - // callbacks/effects (never during render), so a post-commit effect is the - // React-recommended pattern — and it is concurrent-safe (a discarded render - // can't corrupt them, unlike a render-phase write). See #1063. + // Sync "latest value" refs after commit (read only from callbacks/effects) — concurrent-safe (#1063). useEffect(() => { cursorInfoRef.current = cursorInfo; preserveLineBreaksRef.current = preserveLineBreaks; @@ -234,9 +231,8 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = }, [setContent, windowLabel] ); - // Synced during render (not in an effect) on purpose: the unmount cleanup below - // reads this ref to flush pending content, and must see the latest flusher even - // if the component unmounts before a passive effect could run (#755). + // Synced during render so the unmount-flush cleanup below sees the latest flusher + // even if a passive effect hasn't run yet (#755). // eslint-disable-next-line react-hooks/refs flushToStoreRef.current = flushToStore; @@ -437,33 +433,24 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = }, }); - // Keep editorRef aligned with the live editor so unmount cleanup can flush - // directly without depending on the global flusher registry — which may be - // nulled by this component's own registration cleanup before the flush - // cleanup runs (React runs effect cleanups in reverse registration order). - // Synced during render (not in an effect) on purpose so it is set even if the - // component unmounts before a passive effect could run (#755). + // Keep editorRef aligned with the live editor for the unmount-flush cleanup. + // Synced during render (not an effect) so it is set even if a passive effect + // hasn't run, and so it survives the reverse-order cleanup race (#755). // eslint-disable-next-line react-hooks/refs editorRef.current = editor ?? null; - // Show-invisibles toggle — flip the extension storage flag and - // dispatch a transaction that the plugin's apply() picks up to - // rebuild decorations. + // Show-invisibles toggle — flip the extension storage flag, then dispatch a + // tagged transaction the plugin's apply() picks up to rebuild decorations. useEffect(() => { if (!editor) return; - // Update the extension's storage flag so future doc-changed - // transactions see the new value in the plugin's apply() path. const allStorage = editor.storage as unknown as | Record | undefined; const storage = allStorage?.showInvisibles; - // editor.storage is Tiptap's intentionally-mutable extension storage bag; writing the - // flag here is the documented way to toggle an extension's runtime state — not React state. + // editor.storage is Tiptap's intentionally-mutable extension storage, not React state (#1063). // eslint-disable-next-line react-hooks/immutability if (storage) storage.enabled = showInvisibles; - // Force an immediate rebuild via the plugin's exported helper — - // this dispatches a tagged transaction the plugin recognises by - // PluginKey identity (a string meta key would silently no-op). + // Force a rebuild via the plugin's helper (recognised by PluginKey identity). const view = editor.view; if (!view) return; setShowInvisibles(view, showInvisibles); @@ -484,16 +471,13 @@ export function TiptapEditorInner({ hidden = false, readOnly = false, preview = enabled: !!editor && !hidden && !preview, }); - // Cleanup all pending timers/RAFs on unmount to prevent memory leaks. - // Flush any pending content BEFORE cancelling timers to avoid data loss — - // keystrokes within the debounce window exist only in PM's in-memory doc (#755). + // Cleanup pending timers/RAFs on unmount. Flush pending content BEFORE + // cancelling — keystrokes in the debounce window live only in PM's doc (#755). useEffect(() => { return () => { - // Flush pending content directly via this instance's editor — relying on - // the global flushActiveWysiwygNow() registry was racy: React cleans up - // effects in reverse registration order, so the flusher deregistration - // (useEffect below) runs before this cleanup and the flush becomes a - // no-op, losing keystrokes within the debounce window (#755). + // Flush directly via this instance's editor: the global flushActiveWysiwygNow() + // registry was racy — React cleans effects up in reverse registration order, so + // the flusher deregistration ran first and the flush no-op'd, losing data (#755). if ((pendingRaf.current || pendingDebounceTimeout.current) && editorRef.current && flushToStoreRef.current) { try { flushToStoreRef.current(editorRef.current); } catch { /* defensive */ } } diff --git a/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx b/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx index faa1b00db..4fe1d2501 100644 --- a/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx +++ b/src/components/Editor/UniversalToolbar/UniversalToolbar.tsx @@ -4,11 +4,9 @@ * A universal, single-line toolbar anchored at the bottom of the window. * Triggered by Shift+Cmd+P, provides formatting actions across WYSIWYG and Source. * - * Per redesign spec: - * - Focus toggle model (Shift+Cmd+P toggles focus, not visibility) - * - Two-step Escape (dropdown first, then toolbar) - * - Session memory (cleared on toolbar close) - * - Smart initial focus (active marks > selection > context > default) + * Per redesign spec: focus-toggle model (Shift+Cmd+P toggles focus, not visibility), + * two-step Escape (dropdown then toolbar), session memory (cleared on close), and + * smart initial focus (active marks > selection > context > default). * * @module components/Editor/UniversalToolbar */ @@ -89,9 +87,8 @@ export function UniversalToolbar() { [buttons, toolbarContext] ); - // AI-Prompts action button: trailing pseudo-button in the roving-tabindex - // model at index `buttons.length` (a11y/A4 — keyboard-reachable). Always - // enabled, an action (never a dropdown). + // AI-Prompts action button: trailing pseudo-button in the roving-tabindex model + // at index `buttons.length` (a11y/A4 — keyboard-reachable). Always enabled, an action. const genieFocusIndex = buttons.length; const isButtonFocusable = useCallback( @@ -287,9 +284,8 @@ export function UniversalToolbar() { (direction: "left" | "right" | "forward" | "backward") => { const isArrowNav = direction === "left" || direction === "right"; const isNext = direction === "right" || direction === "forward"; - // genieFocusIndex + 1 = full roving count (group buttons + the trailing - // AI-Prompts pseudo-button), so dropdown-exit nav can land on the Genie - // button too (A4). + // genieFocusIndex + 1 = full roving count (group buttons + trailing AI-Prompts + // pseudo-button), so dropdown-exit nav can land on the Genie button too (A4). const newIndex = isNext ? getNextFocusableIndex(focusedIndex, genieFocusIndex + 1, isButtonFocusable) : getPrevFocusableIndex(focusedIndex, genieFocusIndex + 1, isButtonFocusable); @@ -332,21 +328,18 @@ export function UniversalToolbar() { } }, [focusedIndex]); - // Sync with store's dropdown state (for global Escape handling) + // Sync local dropdown state from the external store (for global Escape handling). useEffect(() => { - // Store says dropdown should be closed, but local state says open. - // Legitimate: reacts to the external store's dropdown state (#1063). if (!storeDropdownOpen && menuOpen) { - // eslint-disable-next-line react-hooks/set-state-in-effect + // eslint-disable-next-line react-hooks/set-state-in-effect -- reacts to external store dropdown state (#1063) closeMenu(); } }, [storeDropdownOpen, menuOpen, closeMenu]); - // Close dropdown when focus leaves toolbar (focus toggle). Legitimate: reacts - // to the external toolbar-focus signal (#1063). + // Close dropdown when focus leaves the toolbar (focus toggle). useEffect(() => { if (!toolbarHasFocus && menuOpen) { - // eslint-disable-next-line react-hooks/set-state-in-effect + // eslint-disable-next-line react-hooks/set-state-in-effect -- reacts to external toolbar-focus signal (#1063) closeMenu(false); } }, [toolbarHasFocus, menuOpen, closeMenu]); @@ -363,9 +356,8 @@ export function UniversalToolbar() { } }, [visible, toolbarHasFocus, focusActiveEditor]); - // Handle toolbar open/close and initial focus. Legitimate setState-in-effect: - // reacts to the external visibility toggle and seeds keyboard focus from - // session memory / button states — not derivable during render (#1063). + // Handle toolbar open/close and initial focus — reacts to the external visibility + // toggle and seeds keyboard focus from session memory / button states (#1063). /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (!visible) { diff --git a/src/components/GeniePicker/GeniePicker.tsx b/src/components/GeniePicker/GeniePicker.tsx index ba2aa0be8..13619628b 100644 --- a/src/components/GeniePicker/GeniePicker.tsx +++ b/src/components/GeniePicker/GeniePicker.tsx @@ -83,10 +83,8 @@ export function GeniePicker() { // Prompt history hook (pass grace-period guard for freeform keyDown) const promptHistory = usePromptHistory(ime.isComposing); - // Load genies on open + reset history hook. Legitimate setState-in-effect: the - // resets are bound to the open/close transition and bundled with side effects - // (genie load, focus capture/restore, prompt-history reset) — not derivable - // during render (#1063). + // Load genies + reset on open: resets bound to the open/close transition, bundled + // with side effects (genie load, focus capture/restore, history reset) (#1063). /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { /* v8 ignore next -- @preserve reason: false branch (close path with focus restore) untestable in jsdom */ @@ -172,9 +170,7 @@ export function GeniePicker() { return items; }, [recents, grouped]); - // Clamp selectedIndex when flatList shrinks (e.g. after typing narrows results). - // Adjusted during render — React's recommended alternative to a setState-in- - // effect, which would flash an out-of-range selection for a frame (#1063). + // Clamp selectedIndex when flatList shrinks — adjusted during render, not in an effect (#1063). if (flatList.length > 0 && selectedIndex >= flatList.length) { setSelectedIndex(flatList.length - 1); } @@ -291,12 +287,9 @@ export function GeniePicker() { [flatList, selectedIndex, handleClose, handleSelect, activeScope, handleFreeformSubmit, ime, mode, filter, freeformConfirmed] ); - // Sync prompt history cycling back to filter. - // When cycling changes displayValue, push it into filter so the textarea updates. - // Safe from loops: typing sets displayValue === filter via handleChange, so the - // guard (displayValue !== filter) is only true when cycling produces a new value. - // Legitimate setState-in-effect: reacts to external prompt-history cycling, not - // a value derivable from this render (#1063). + // Sync prompt-history cycling back to filter so the textarea updates. Reacts to + // external history state (#1063); loop-safe — typing sets displayValue === filter + // via handleChange, so the guard is true only when cycling produces a new value. /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { if (flatList.length === 0 && promptHistory.displayValue !== filter) { @@ -306,11 +299,9 @@ export function GeniePicker() { }, [promptHistory.displayValue, filter, flatList.length]); /* eslint-enable react-hooks/set-state-in-effect */ - // Click outside to close. Escape is handled by the component's own - // onKeyDown (mode-aware: resetToInput in processing/preview/error, - // close otherwise), so only the outside-click half is delegated here. - // Deferred attach prevents the opening click from immediately - // dismissing; bubble phase matches the original code. + // Click outside to close (Escape is handled by the mode-aware onKeyDown, so only + // the outside-click half is delegated). Deferred attach prevents the opening click + // from immediately dismissing; bubble phase matches the original code. useDismissOnOutsideOrEscape(isOpen, containerRef, handleClose, { deferActivation: true, escape: false, diff --git a/src/components/StatusBar/useStatusBarTabDrag.ts b/src/components/StatusBar/useStatusBarTabDrag.ts index 0675d3a31..2c21a61b5 100644 --- a/src/components/StatusBar/useStatusBarTabDrag.ts +++ b/src/components/StatusBar/useStatusBarTabDrag.ts @@ -185,8 +185,7 @@ export function useStatusBarTabDrag({ tabs, windowLabel, tabBarRef, onActivateTa if (mode !== "dragout") return; latestDragPointRef.current = point; if (previewProbeTimerRef.current) return; - // Tag each probe with the current drag generation so stale responses - // (arriving after drag ends or moves on) are discarded. + // Tag each probe with the current drag generation so stale responses are discarded. const probeGen = dragGenerationRef.current; previewProbeTimerRef.current = setTimeout(() => { previewProbeTimerRef.current = null; @@ -286,9 +285,7 @@ export function useStatusBarTabDrag({ tabs, windowLabel, tabBarRef, onActivateTa if (dragMode !== "idle") return; // Advance generation so in-flight probe responses are discarded dragGenerationRef.current++; - // Legitimate: clears the cross-window drop-preview broadcast when the drag - // returns to idle — a transition side effect, not derivable during render (#1063). - // eslint-disable-next-line react-hooks/set-state-in-effect + // eslint-disable-next-line react-hooks/set-state-in-effect -- clears cross-window drop-preview on the drag→idle transition (#1063) clearDropPreviewBroadcast(); }, [clearDropPreviewBroadcast, dragMode]); diff --git a/src/hooks/useGenieInvocation.ts b/src/hooks/useGenieInvocation.ts index b1866185b..2662e47c3 100644 --- a/src/hooks/useGenieInvocation.ts +++ b/src/hooks/useGenieInvocation.ts @@ -487,11 +487,8 @@ export function useGenieInvocation() { [runGenie] ); - // Keep ref in sync for MCP bridge listener — synced after commit (read only - // from the async bridge listener, never during render). #1063 - useEffect(() => { - invokeGenieRef.current = invokeGenie; - }); + // eslint-disable-next-line react-hooks/refs -- latest-value ref read only by the async MCP bridge listener (#1063) + invokeGenieRef.current = invokeGenie; const invokeFreeform = useCallback( async (userPrompt: string, scope: GenieScope) => { diff --git a/src/hooks/useTabDragOut.ts b/src/hooks/useTabDragOut.ts index efab2f328..c6301a4e0 100644 --- a/src/hooks/useTabDragOut.ts +++ b/src/hooks/useTabDragOut.ts @@ -1,8 +1,7 @@ /** * Tab Drag-Out Hook * - * Purpose: Manages tab drag interactions — reorder within the tab bar - * or drag out to detach a tab into a new window. + * Purpose: Manages tab drag interactions — reorder within the tab bar or drag out to detach into a new window. * * Key decisions: * - Uses pointer events (not mouse) for touch support @@ -151,8 +150,7 @@ export function useTabDragOut({ tabBarRef, onDragOut, onReorder, onDragMove }: U } }, []); - // Stable refs for callbacks used in document listeners — synced after commit - // (read only from the document drag listeners, never during render). #1063 + // Latest-value refs read only from the document drag listeners; synced after commit (#1063). const onDragOutRef = useRef(onDragOut); const onReorderRef = useRef(onReorder); const onDragMoveRef = useRef(onDragMove); @@ -203,9 +201,8 @@ export function useTabDragOut({ tabBarRef, onDragOut, onReorder, onDragMove }: U // Only primary button; skip pinned tabs if (e.button !== 0 || isPinned) return; - // Skip drag initiation when clicking the close button (or its children). - // Pointer capture would steal the pointerup from the button, preventing - // the click event from firing and making the tab un-closable via X. + // Skip drag init on the close button — pointer capture would steal the + // pointerup, making the tab un-closable via X. const target = e.target; if (target instanceof Element && target.closest("[data-tab-close]")) return; diff --git a/src/pages/settings/IntegrationsSettings.tsx b/src/pages/settings/IntegrationsSettings.tsx index a903a35c5..5533bb9cd 100644 --- a/src/pages/settings/IntegrationsSettings.tsx +++ b/src/pages/settings/IntegrationsSettings.tsx @@ -62,9 +62,7 @@ export function IntegrationsSettings() { // Fetch client count when bridge is running useEffect(() => { if (!running) { - // Legitimate: resets the count as part of an async poll gated on `running`, - // not derivable during render (#1063). - // eslint-disable-next-line react-hooks/set-state-in-effect + // eslint-disable-next-line react-hooks/set-state-in-effect -- resets count as part of an async poll gated on `running` (#1063) setClientCount(0); return; } @@ -108,8 +106,7 @@ export function IntegrationsSettings() { updateAdvancedSetting("mcpServer", { ...mcpSettings, autoApproveEdits: enabled }); }; - // Called after MCP config is successfully installed to a provider - // Enables autoStart and starts the bridge so it works immediately + // After MCP config installs to a provider: enable autoStart and start the bridge so it works immediately. const handleMcpConfigInstalled = async () => { // Enable autoStart so bridge runs on future launches if (!mcpSettings.autoStart) { diff --git a/src/pages/settings/McpConfigInstaller.tsx b/src/pages/settings/McpConfigInstaller.tsx index 76e8f8a93..71c54d7a6 100644 --- a/src/pages/settings/McpConfigInstaller.tsx +++ b/src/pages/settings/McpConfigInstaller.tsx @@ -162,9 +162,7 @@ export function McpConfigInstaller({ onInstallSuccess }: McpConfigInstallerProps }, []); useEffect(() => { - // Legitimate: loadDiagnostics() runs an async provider probe that sets - // diagnostics state on mount — I/O, not derivable during render (#1063). - // eslint-disable-next-line react-hooks/set-state-in-effect + // eslint-disable-next-line react-hooks/set-state-in-effect -- async provider probe sets diagnostics on mount (#1063) loadDiagnostics(); }, [loadDiagnostics]); @@ -233,8 +231,7 @@ export function McpConfigInstaller({ onInstallSuccess }: McpConfigInstallerProps } }; - // CC-Switch deep-link import (issue #1008). VMark's sidecar binary path is - // the same across providers; grab the first diagnostic that resolved it. + // CC-Switch deep-link import (#1008): sidecar path is identical across providers — grab the first resolved diagnostic. const ccSwitchBinaryPath = diagnostics.find((d) => d.expectedBinaryPath)?.expectedBinaryPath ?? null; From 54800398c8075fef828c203b1ac9f9b70361d929 Mon Sep 17 00:00:00 2001 From: xiaolai Date: Sat, 27 Jun 2026 17:43:32 +0800 Subject: [PATCH 6/6] fix(lint): preserve timing/repositioning behavior after #1063 refactors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-model audit (Codex) of the react-hooks-7 adoption flagged three behavior-timing regressions; all fixed and re-verified: - useContentSearchScheduler: revert excludeFoldersRef to a render-phase sync (scoped disable) so an already-pending debounced search reads the latest exclusions after an exclusion-only re-render, matching prior behavior. - WordCountPopover + McpHistoryButton: the layout-effect positioning now remeasures every render while open (no deps), so the popover tracks trigger shifts as counts/badges change — as the original render-phase position() did. A functional setState updater returns prev when unchanged, preventing any render loop. --- .../ContentSearch/useContentSearchScheduler.ts | 9 +++++---- src/components/McpHistory/McpHistoryButton.tsx | 17 +++++++++++------ src/components/StatusBar/WordCountPopover.tsx | 10 +++++++--- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/components/ContentSearch/useContentSearchScheduler.ts b/src/components/ContentSearch/useContentSearchScheduler.ts index 5e4af58c2..3cbad8c62 100644 --- a/src/components/ContentSearch/useContentSearchScheduler.ts +++ b/src/components/ContentSearch/useContentSearchScheduler.ts @@ -51,10 +51,11 @@ export function useContentSearchScheduler({ // query/option change without a stale capture and without re-running the // search merely because the array identity changed. const excludeFoldersRef = useRef(excludeFolders); - // Synced after commit (read only at query-execution time). #1063 - useEffect(() => { - excludeFoldersRef.current = excludeFolders; - }); + // Synced during render (not an effect) so an already-pending debounced search + // reads the latest exclusions even after an exclusion-only re-render. Read only + // at query-execution time inside the debounce, never during render. #1063 + // eslint-disable-next-line react-hooks/refs + excludeFoldersRef.current = excludeFolders; useEffect(() => { if (!isOpen || !rootPath) return; diff --git a/src/components/McpHistory/McpHistoryButton.tsx b/src/components/McpHistory/McpHistoryButton.tsx index 45354f572..b6d4a8c17 100644 --- a/src/components/McpHistory/McpHistoryButton.tsx +++ b/src/components/McpHistory/McpHistoryButton.tsx @@ -139,21 +139,26 @@ export function McpHistoryButton(): React.ReactElement { toast.success(t("mcpHistoryCleared")); }, [tabId, tabFilePath, t]); - // Measure the trigger and position the popover in a layout effect — reading - // the DOM rect during render is not concurrent-safe (#1063). Runs before paint - // when the popover opens, so there is no flicker. + // Measure the trigger and position the popover in a layout effect — reading the + // DOM rect during render is not concurrent-safe (#1063). No deps: remeasure every + // render while open (the badge/count can shift the trigger rect); the functional + // update bails when nothing moved, so there is no render loop and no flicker. const [popoverStyle, setPopoverStyle] = useState({ display: "none" }); useLayoutEffect(() => { if (!open) return; const rect = buttonRef.current?.getBoundingClientRect(); if (!rect) { - setPopoverStyle({ display: "none" }); + setPopoverStyle((prev) => (prev.display === "none" ? prev : { display: "none" })); return; } const right = Math.max(8, window.innerWidth - rect.right); const bottom = Math.max(8, window.innerHeight - rect.top + 6); - setPopoverStyle({ right, bottom, width: POPUP_WIDTH, maxHeight: POPUP_MAX_HEIGHT }); - }, [open]); + setPopoverStyle((prev) => + prev.right === right && prev.bottom === bottom + ? prev + : { right, bottom, width: POPUP_WIDTH, maxHeight: POPUP_MAX_HEIGHT }, + ); + }); return ( <> diff --git a/src/components/StatusBar/WordCountPopover.tsx b/src/components/StatusBar/WordCountPopover.tsx index 7b605b7e7..4d806d03c 100644 --- a/src/components/StatusBar/WordCountPopover.tsx +++ b/src/components/StatusBar/WordCountPopover.tsx @@ -64,15 +64,19 @@ export function WordCountPopover({ // Position above the anchor by measuring it in a layout effect (before paint), // rather than reading the anchor rect during render — the latter is not - // concurrent-safe (#1063). + // concurrent-safe (#1063). No deps: remeasure every render (the trigger width + // shifts as counts change while open); the functional update bails when nothing + // moved, so there is no render loop. const [style, setStyle] = useState({ right: 8, bottom: 8, width: POPUP_WIDTH }); useLayoutEffect(() => { const rect = anchorRef.current?.getBoundingClientRect(); if (!rect) return; const right = Math.max(8, window.innerWidth - rect.right); const bottom = Math.max(8, window.innerHeight - rect.top + 6); - setStyle({ right, bottom, width: POPUP_WIDTH }); - }, [anchorRef]); + setStyle((prev) => + prev.right === right && prev.bottom === bottom ? prev : { right, bottom, width: POPUP_WIDTH }, + ); + }); return (