chore(lint): adopt eslint-plugin-react-hooks v7 React-Compiler rules (#1063) - #1068
Merged
Merged
Conversation
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.
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.
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.
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.
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.
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.
xiaolai
added a commit
that referenced
this pull request
Aug 12, 2026
…1063) (#1068) * chore(lint): adopt react-hooks/immutability rule (#1063) 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. * chore(lint): adopt react-hooks/preserve-manual-memoization rule (#1063) 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. * chore(lint): adopt react-hooks/refs rule (#1063) 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. * chore(lint): adopt react-hooks/set-state-in-effect rule (#1063) 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. * chore(lint): keep #1063-edited files within file-size baseline 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. * fix(lint): preserve timing/repositioning behavior after #1063 refactors 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the deferred react-hooks-7 adoption from #1062. The four React-Compiler rules folded into
recommended(which flagged 68 pre-existing sites) are now fully on aterror, adopted one rule at a time (one commit per rule).Approach
Each site was classified, not blanket-disabled:
setState(React's recommended alternative — no extra render, no stale frame),useLayoutEffectfor DOM measurement, re-keyed memos, or initial state.Rules adopted
react-hooks/immutabilityreact-hooks/preserve-manual-memoizationnavigator-readinguseMemothe compiler can't preserve; hoisted pure logicreact-hooks/refsreact-hooks/set-state-in-effectexhaustive-depsstays atwarn(its v5 level; ~67 unrelated sites out of scope).Verification
Full
pnpm check:allis green — lint chain, file-size (reclaimed added lines by condensing comments to respect the ratchet-down baseline), knip, coverage thresholds met, sidecar (185), content-server (142), build, eager-chunk, size. No behavior changes; all affected component/hook test suites pass.Closes #1063