Fix useEffect anti-patterns across renderer components

Replace 11 useEffect anti-patterns with idiomatic React alternatives:
- useState+useEffect derived state → "adjusting state during render" pattern
- Self-referential effect deps → render-time validation
- Effect chains → merged single effect with requestAnimationFrame
- Unnecessary callback deps → inlined options

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Neil
2026-03-21 15:10:59 -07:00
co-authored by Claude Opus 4.6
parent abcdc6d451
commit 65054f5978
6 changed files with 100 additions and 89 deletions
+22 -21
View File
@@ -55,7 +55,6 @@ function UIZoomControl(): React.JSX.Element {
useEffect(() => {
return window.api.ui.onTerminalZoom(() => {
setZoomLevel(window.api.ui.getZoomLevel())
setTimeout(() => setZoomLevel(window.api.ui.getZoomLevel()), 50)
})
}, [])
@@ -306,12 +305,14 @@ function FontAutocomplete({
onChange
}: FontAutocompleteProps): React.JSX.Element {
const [query, setQuery] = useState(value)
const [prevValue, setPrevValue] = useState(value)
const [open, setOpen] = useState(false)
const rootRef = useRef<HTMLDivElement | null>(null)
useEffect(() => {
if (value !== prevValue) {
setPrevValue(value)
setQuery(value)
}, [value])
}
useEffect(() => {
if (!open) return
@@ -442,6 +443,7 @@ function Settings(): React.JSX.Element {
const [themeSearchLight, setThemeSearchLight] = useState('')
const [systemPrefersDark, setSystemPrefersDark] = useState(getSystemPrefersDark())
const [scrollbackMode, setScrollbackMode] = useState<'preset' | 'custom'>('preset')
const [prevSettings, setPrevSettings] = useState(settings)
const [terminalFontSuggestions, setTerminalFontSuggestions] = useState<string[]>(
getFallbackTerminalFonts()
)
@@ -490,16 +492,17 @@ function Settings(): React.JSX.Element {
}
}, [selectedPane])
useEffect(() => {
if (!settings) return
const scrollbackMb = Math.max(1, Math.round(settings.terminalScrollbackBytes / 1_000_000))
setScrollbackMode(
SCROLLBACK_PRESETS_MB.includes(scrollbackMb as (typeof SCROLLBACK_PRESETS_MB)[number])
? 'preset'
: 'custom'
)
}, [settings])
if (settings !== prevSettings) {
setPrevSettings(settings)
if (settings) {
const scrollbackMb = Math.max(1, Math.round(settings.terminalScrollbackBytes / 1_000_000))
setScrollbackMode(
SCROLLBACK_PRESETS_MB.includes(scrollbackMb as (typeof SCROLLBACK_PRESETS_MB)[number])
? 'preset'
: 'custom'
)
}
}
useEffect(() => {
let stale = false
@@ -603,17 +606,15 @@ function Settings(): React.JSX.Element {
}
}, [selectedRepoId, baseRefQuery])
useEffect(() => {
if (repos.length === 0) {
// Validate selectedRepoId against current repos (adjusting state during render)
if (repos.length === 0) {
if (selectedRepoId !== null) {
setSelectedRepoId(null)
if (selectedPane === 'repo') setSelectedPane('general')
return
}
if (!selectedRepoId || !repos.some((repo) => repo.id === selectedRepoId)) {
setSelectedRepoId(repos[0].id)
}
}, [repos, selectedRepoId])
} else if (!selectedRepoId || !repos.some((repo) => repo.id === selectedRepoId)) {
setSelectedRepoId(repos[0].id)
}
const applyTheme = useCallback((theme: 'system' | 'dark' | 'light') => {
const root = document.documentElement
+5 -7
View File
@@ -25,6 +25,11 @@ export default function Terminal(): React.JSX.Element | null {
const allWorktrees = Object.values(worktreesByRepo).flat()
const prevTabCountRef = useRef(tabs.length)
// Ensure activeTabId is valid (adjusting state during render)
if (tabs.length > 0 && (!activeTabId || !tabs.find((t) => t.id === activeTabId))) {
setActiveTab(tabs[0].id)
}
// Track which worktrees have been activated during this app session.
// Only mount TerminalPanes for visited worktrees to prevent mass PTY
// spawning when restoring a session with many saved worktree tabs.
@@ -57,13 +62,6 @@ export default function Terminal(): React.JSX.Element | null {
createTab(activeWorktreeId)
}, [workspaceSessionReady, activeWorktreeId, tabs.length, createTab])
// Ensure activeTabId is valid
useEffect(() => {
if (tabs.length > 0 && (!activeTabId || !tabs.find((t) => t.id === activeTabId))) {
setActiveTab(tabs[0].id)
}
}, [tabs, activeTabId, setActiveTab])
// Animate tab bar height with grid transition
useEffect(() => {
const el = tabBarRef.current
@@ -49,9 +49,9 @@ export default function TerminalSearch({
return
}
if (searchAddon && isOpen) {
searchAddon.findNext(query, searchOptions(true))
searchAddon.findNext(query, { caseSensitive, regex, incremental: true })
}
}, [query, searchAddon, isOpen, searchOptions])
}, [query, searchAddon, isOpen, caseSensitive, regex])
const handleKeyDown = useCallback(
(e: React.KeyboardEvent) => {
@@ -51,6 +51,8 @@ const AddWorktreeDialog = React.memo(function AddWorktreeDialog() {
const nameInputRef = useRef<HTMLInputElement>(null)
const lastSuggestedNameRef = useRef('')
const resetTimeoutRef = useRef<number | null>(null)
const prevIsOpenRef = useRef(false)
const prevSuggestedNameRef = useRef('')
const isOpen = activeModal === 'create-worktree'
const preselectedRepoId =
@@ -65,6 +67,33 @@ const AddWorktreeDialog = React.memo(function AddWorktreeDialog() {
[repoId, worktreesByRepo, settings?.nestWorkspaces]
)
// Auto-select repo when dialog opens (adjusting state during render)
if (isOpen && !prevIsOpenRef.current && repos.length > 0) {
if (preselectedRepoId && repos.some((repo) => repo.id === preselectedRepoId)) {
setRepoId(preselectedRepoId)
} else if (activeWorktreeRepoId && repos.some((repo) => repo.id === activeWorktreeRepoId)) {
setRepoId(activeWorktreeRepoId)
} else if (activeRepoId && repos.some((repo) => repo.id === activeRepoId)) {
setRepoId(activeRepoId)
} else {
setRepoId(repos[0].id)
}
}
prevIsOpenRef.current = isOpen
// Auto-fill name from suggestion (adjusting state during render)
if (isOpen && repoId && suggestedName && suggestedName !== prevSuggestedNameRef.current) {
const shouldApplySuggestion = !name.trim() || name === lastSuggestedNameRef.current
prevSuggestedNameRef.current = suggestedName
if (shouldApplySuggestion) {
setName(suggestedName)
lastSuggestedNameRef.current = suggestedName
}
}
if (!isOpen) {
prevSuggestedNameRef.current = ''
}
const handleOpenChange = useCallback(
(open: boolean) => {
if (!open) {
@@ -151,45 +180,16 @@ const AddWorktreeDialog = React.memo(function AddWorktreeDialog() {
}
}, [isOpen])
React.useEffect(() => {
if (!isOpen || repos.length === 0) return
if (preselectedRepoId && repos.some((repo) => repo.id === preselectedRepoId)) {
setRepoId(preselectedRepoId)
return
}
if (activeWorktreeRepoId && repos.some((repo) => repo.id === activeWorktreeRepoId)) {
setRepoId(activeWorktreeRepoId)
return
}
if (activeRepoId && repos.some((repo) => repo.id === activeRepoId)) {
setRepoId(activeRepoId)
return
}
if (!repoId) {
setRepoId(repos[0].id)
}
}, [isOpen, repos, repoId, preselectedRepoId, activeWorktreeRepoId, activeRepoId])
// Focus and select name input when suggestion is applied
React.useEffect(() => {
if (!isOpen || !repoId || !suggestedName) return
const shouldApplySuggestion = !name.trim() || name === lastSuggestedNameRef.current
if (!shouldApplySuggestion) return
setName(suggestedName)
lastSuggestedNameRef.current = suggestedName
requestAnimationFrame(() => {
const input = nameInputRef.current
if (!input) return
input.focus()
input.select()
})
}, [isOpen, repoId, suggestedName, name])
}, [isOpen, repoId, suggestedName])
// Safety guard: creating a worktree requires at least one repo.
React.useEffect(() => {
@@ -269,23 +269,6 @@ const WorktreeList = React.memo(function WorktreeList() {
return result
}, [groupBy, worktrees, repoMap, prCache, collapsedGroups, tabsByWorktree])
React.useEffect(() => {
if (!pendingRevealWorktreeId || groupBy === 'none') return
const targetWorktree = worktrees.find((worktree) => worktree.id === pendingRevealWorktreeId)
if (!targetWorktree) return
const groupKey = getGroupKeyForWorktree(groupBy, targetWorktree, repoMap, prCache)
if (!groupKey) return
setCollapsedGroups((prev) => {
if (!prev.has(groupKey)) return prev
const next = new Set(prev)
next.delete(groupKey)
return next
})
}, [pendingRevealWorktreeId, groupBy, worktrees, repoMap, prCache])
// ── TanStack Virtual ──────────────────────────────────────────
const virtualizer = useVirtualizer({
count: rows.length,
@@ -301,14 +284,42 @@ const WorktreeList = React.memo(function WorktreeList() {
React.useEffect(() => {
if (!pendingRevealWorktreeId) return
const targetIndex = rows.findIndex(
(row) => row.type === 'item' && row.worktree.id === pendingRevealWorktreeId
)
if (targetIndex === -1) return
// Uncollapse the group containing the target worktree
if (groupBy !== 'none') {
const targetWorktree = worktrees.find((w) => w.id === pendingRevealWorktreeId)
if (targetWorktree) {
const groupKey = getGroupKeyForWorktree(groupBy, targetWorktree, repoMap, prCache)
if (groupKey) {
setCollapsedGroups((prev) => {
if (!prev.has(groupKey)) return prev
const next = new Set(prev)
next.delete(groupKey)
return next
})
}
}
}
virtualizer.scrollToIndex(targetIndex, { align: 'center' })
clearPendingRevealWorktreeId()
}, [pendingRevealWorktreeId, rows, virtualizer, clearPendingRevealWorktreeId])
// Scroll to the target after the group uncollapse re-render settles
requestAnimationFrame(() => {
const targetIndex = rows.findIndex(
(row) => row.type === 'item' && row.worktree.id === pendingRevealWorktreeId
)
if (targetIndex !== -1) {
virtualizer.scrollToIndex(targetIndex, { align: 'center' })
}
clearPendingRevealWorktreeId()
})
}, [
pendingRevealWorktreeId,
groupBy,
worktrees,
repoMap,
prCache,
rows,
virtualizer,
clearPendingRevealWorktreeId
])
const handleCreateForRepo = useCallback(
(repoId: string) => {
@@ -1,4 +1,4 @@
import React, { useCallback, useEffect, useMemo, useState } from 'react'
import React, { useCallback, useMemo, useRef, useState } from 'react'
import { useAppStore } from '@/store'
import {
Dialog,
@@ -32,11 +32,12 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() {
const [commentInput, setCommentInput] = useState('')
const [saving, setSaving] = useState(false)
useEffect(() => {
if (!isOpen) return
const prevIsOpenRef = useRef(false)
if (isOpen && !prevIsOpenRef.current) {
setIssueInput(currentIssue)
setCommentInput(currentComment)
}, [isOpen, currentIssue, currentComment])
}
prevIsOpenRef.current = isOpen
const canSave = useMemo(() => {
if (!worktreeId) return false