perf(renderer): unmount closed jump palette content (#13525)

This commit is contained in:
Neil
2026-08-10 00:18:54 -07:00
committed by GitHub
parent 8a3f704bd1
commit c498d763cd
2 changed files with 239 additions and 21 deletions
@@ -0,0 +1,198 @@
// @vitest-environment happy-dom
import { act } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type * as ReactI18Next from 'react-i18next'
import { useAppStore } from '@/store'
import WorktreeJumpPalette from './WorktreeJumpPalette'
const contentProbe = vi.hoisted(() => ({
renders: vi.fn(),
storeNotifications: vi.fn(),
subscriptions: vi.fn(),
unsubscriptions: vi.fn()
}))
vi.mock('react-i18next', async (importOriginal) => {
const actual = await importOriginal<typeof ReactI18Next>()
return {
...actual,
useTranslation: () => ({
t: (_key: string, fallback?: string) => fallback ?? _key
})
}
})
vi.mock('sonner', () => ({
toast: {
success: vi.fn(),
error: vi.fn(),
info: vi.fn(),
warning: vi.fn(),
message: vi.fn()
}
}))
vi.mock('@/hooks/useSettingsNavigationMetadata', async () => {
const React = await import('react')
return {
useSettingsNavigationMetadata: () => {
contentProbe.renders()
React.useEffect(() => {
contentProbe.subscriptions()
const unsubscribe = useAppStore.subscribe(() => contentProbe.storeNotifications())
return () => {
contentProbe.unsubscriptions()
unsubscribe()
}
}, [])
return []
}
}
})
vi.mock('@/components/sidebar/StatusIndicator', () => ({
default: () => <span data-status-indicator="true" />
}))
vi.mock('@/components/repo/RepoBadgeLabel', () => ({
RepoBadgeMark: () => <span data-repo-badge-mark="true" />
}))
vi.mock('@/components/cmd-j/palette-host-badge', () => ({
getPaletteHostBadge: () => null
}))
vi.mock('@/components/ui/command', async () => {
const React = await import('react')
return {
Command: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
CommandGroup: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
CommandDialog: ({ children, open }: { children: React.ReactNode; open?: boolean }) =>
open ? <div data-command-dialog="true">{children}</div> : null,
CommandInput: React.forwardRef(function CommandInput(
_props: Record<string, unknown>,
ref: React.ForwardedRef<HTMLInputElement>
) {
return <input ref={ref} data-command-input="true" />
}),
CommandList: React.forwardRef(function CommandList(
{ children }: { children: React.ReactNode },
ref: React.ForwardedRef<HTMLDivElement>
) {
return <div ref={ref}>{children}</div>
}),
CommandEmpty: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
CommandItem: ({ children }: { children: React.ReactNode }) => (
<button type="button">{children}</button>
)
}
})
const initialAppState = useAppStore.getInitialState()
let testContainer: HTMLDivElement
let testRoot: Root
async function flushEffects(): Promise<void> {
await act(async () => {
await Promise.resolve()
await Promise.resolve()
})
}
async function churnClosedStore(count: number): Promise<void> {
await act(async () => {
for (let index = 0; index < count; index += 1) {
useAppStore.setState({ activeWorktreeId: `background-${index}` })
}
})
}
function activeContentSubscriptions(): number {
return (
contentProbe.subscriptions.mock.calls.length - contentProbe.unsubscriptions.mock.calls.length
)
}
describe('WorktreeJumpPalette mount gating', () => {
beforeEach(() => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] })
contentProbe.renders.mockClear()
contentProbe.storeNotifications.mockClear()
contentProbe.subscriptions.mockClear()
contentProbe.unsubscriptions.mockClear()
useAppStore.setState(initialAppState, true)
useAppStore.setState({ activeModal: 'none', activeWorktreeId: null })
testContainer = document.createElement('div')
document.body.appendChild(testContainer)
testRoot = createRoot(testContainer)
})
afterEach(async () => {
await act(async () => testRoot.unmount())
document.body.replaceChildren()
useAppStore.setState(initialAppState, true)
vi.clearAllTimers()
vi.useRealTimers()
})
it('mounts content on demand and unmounts it after the close linger', async () => {
await act(async () => testRoot.render(<WorktreeJumpPalette />))
await flushEffects()
await churnClosedStore(1_000)
expect(contentProbe.renders).not.toHaveBeenCalled()
expect(contentProbe.subscriptions).not.toHaveBeenCalled()
expect(contentProbe.storeNotifications).not.toHaveBeenCalled()
await act(async () => {
useAppStore.getState().openModal('worktree-palette')
})
await flushEffects()
expect(testContainer.querySelector('[data-command-dialog="true"]')).not.toBeNull()
expect(contentProbe.renders).toHaveBeenCalled()
expect(activeContentSubscriptions()).toBe(1)
await act(async () => {
useAppStore.getState().closeModal()
})
expect(testContainer.querySelector('[data-command-dialog="true"]')).toBeNull()
expect(activeContentSubscriptions()).toBe(1)
await act(async () => vi.advanceTimersByTimeAsync(299))
expect(activeContentSubscriptions()).toBe(1)
const lingerNotifications = contentProbe.storeNotifications.mock.calls.length
await act(async () => useAppStore.setState({ activeWorktreeId: 'during-close-linger' }))
expect(contentProbe.storeNotifications).toHaveBeenCalledTimes(lingerNotifications + 1)
await act(async () => vi.advanceTimersByTimeAsync(1))
expect(activeContentSubscriptions()).toBe(0)
const rendersAfterUnmount = contentProbe.renders.mock.calls.length
const notificationsAfterUnmount = contentProbe.storeNotifications.mock.calls.length
await churnClosedStore(100)
expect(contentProbe.renders).toHaveBeenCalledTimes(rendersAfterUnmount)
expect(contentProbe.storeNotifications).toHaveBeenCalledTimes(notificationsAfterUnmount)
})
it('cancels the pending unmount when reopened during the linger', async () => {
await act(async () => testRoot.render(<WorktreeJumpPalette />))
await act(async () => useAppStore.getState().openModal('worktree-palette'))
await flushEffects()
await act(async () => useAppStore.getState().closeModal())
await act(async () => vi.advanceTimersByTimeAsync(299))
await act(async () => useAppStore.getState().openModal('worktree-palette'))
await act(async () => vi.advanceTimersByTimeAsync(1_000))
expect(testContainer.querySelector('[data-command-dialog="true"]')).not.toBeNull()
expect(activeContentSubscriptions()).toBe(1)
})
})
@@ -79,7 +79,8 @@ import {
createWorktreePaletteRequestGuard,
getNextWorktreePaletteSelection,
getWorktreePaletteSelectionItemIds,
getWorktreePaletteCreateActionState
getWorktreePaletteCreateActionState,
type WorktreePaletteRequestGuard
} from '@/lib/worktree-palette-create-action'
import { getWorkspacePortsByWorktreeId } from '@/lib/workspace-port-groups'
import {
@@ -262,8 +263,8 @@ type PaletteListEntry = PaletteItem | CreateWorktreePaletteItem | SectionHeader
const CREATE_WORKSPACE_QUICK_ACTION_ITEM_ID = `quick-action:${CREATE_WORKSPACE_QUICK_ACTION_ID}`
// Why: outlast the CommandDialog close animation (~150200ms) so gated status maps stay live until fading rows are gone.
const PALETTE_STATUS_INPUTS_LINGER_MS = 300
// Why: outlast the CommandDialog close animation so its rows do not disappear mid-fade.
const PALETTE_CLOSE_LINGER_MS = 300
type OpenTabPaletteItem = BrowserPaletteItem | SimulatorPaletteItem | WorkspaceTabPaletteItem
@@ -508,9 +509,42 @@ function getSettingsTargetFromSectionId(sectionId: string): {
}
export default function WorktreeJumpPalette(): React.JSX.Element | null {
const visible = useAppStore((s) => s.activeModal === 'worktree-palette')
const [lingering, setLingering] = useState(visible)
useEffect(() => {
if (visible) {
setLingering(true)
return
}
const timer = window.setTimeout(() => setLingering(false), PALETTE_CLOSE_LINGER_MS)
return () => window.clearTimeout(timer)
}, [visible])
// Why: reopening must invalidate a pending create lookup from the previous content mount.
const createLookupGuard = useMemo(() => createWorktreePaletteRequestGuard(), [])
if (!visible && !lingering) {
return null
}
return (
<WorktreeJumpPaletteContent
visible={visible}
lingering={lingering}
createLookupGuard={createLookupGuard}
/>
)
}
function WorktreeJumpPaletteContent({
visible,
lingering,
createLookupGuard
}: {
visible: boolean
lingering: boolean
createLookupGuard: WorktreePaletteRequestGuard
}): React.JSX.Element | null {
// Why: subscribe to language changes so translated memos recompute without a fake i18n.language dependency.
useTranslation()
const visible = useAppStore((s) => s.activeModal === 'worktree-palette')
const closeModal = useAppStore((s) => s.closeModal)
const openModal = useAppStore((s) => s.openModal)
const openSettingsPage = useAppStore((s) => s.openSettingsPage)
@@ -526,27 +560,14 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
const detectedWorktreesByRepo = useAppStore((s) => s.detectedWorktreesByRepo)
const pendingWorktreeCreations = useAppStore((s) => s.pendingWorktreeCreations)
const pluginCommands = usePluginCommands()
// Why: keep status maps subscribed through the close animation — dropping them while CommandDialog fades out would flash rows empty mid-animation.
const [statusInputsLingering, setStatusInputsLingering] = useState(false)
useEffect(() => {
if (visible) {
setStatusInputsLingering(true)
return
}
const timer = window.setTimeout(
() => setStatusInputsLingering(false),
PALETTE_STATUS_INPUTS_LINGER_MS
)
return () => window.clearTimeout(timer)
}, [visible])
const paletteStatusInputsActive = visible || statusInputsLingering
// Why: these hot status maps get a new identity on every app-wide write, so gate the subscription on active-or-closing to stop the always-mounted palette re-rendering on unrelated terminals.
const paletteStatusInputsActive = visible || lingering
// Why: keep hot status maps live through the shell's close-animation linger.
// Why: ptyIdsByTabId must be included — slept tabs keep a wake-hint sessionId in tab.ptyId, so without it the palette dot would lie green.
const { ptyIdsByTabId, terminalLayoutsByTabId, tabsByWorktree } = useAppStore(
useShallow((s) => selectPaletteStatusInputs(s, paletteStatusInputsActive))
)
const { prCache, issueCache, hostedReviewCache } = useAppStore(
useShallow((s) => selectWorktreePaletteCacheInputs(s, visible || statusInputsLingering))
useShallow((s) => selectWorktreePaletteCacheInputs(s, paletteStatusInputsActive))
)
const migrationUnsupportedByPtyId = useAppStore((s) => s.migrationUnsupportedByPtyId)
const activeView = useAppStore((s) => s.activeView)
@@ -634,7 +655,6 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
const inputRef = useRef<HTMLInputElement>(null)
const fallbackFocusOuterFrameRef = useRef<number | null>(null)
const fallbackFocusInnerFrameRef = useRef<number | null>(null)
const createLookupGuard = useMemo(() => createWorktreePaletteRequestGuard(), [])
const preserveCreateLookupOnCloseRef = useRef(false)
const repoMap = useMemo(() => new Map(repos.map((r) => [r.id, r])), [repos])