From c498d763cd345deda64cb00d7fcbdd3a2ae3b133 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 10 Aug 2026 00:18:54 -0700 Subject: [PATCH] perf(renderer): unmount closed jump palette content (#13525) --- .../WorktreeJumpPalette.mount-gating.test.tsx | 198 ++++++++++++++++++ .../src/components/WorktreeJumpPalette.tsx | 62 ++++-- 2 files changed, 239 insertions(+), 21 deletions(-) create mode 100644 src/renderer/src/components/WorktreeJumpPalette.mount-gating.test.tsx diff --git a/src/renderer/src/components/WorktreeJumpPalette.mount-gating.test.tsx b/src/renderer/src/components/WorktreeJumpPalette.mount-gating.test.tsx new file mode 100644 index 00000000000..85cb356fbc1 --- /dev/null +++ b/src/renderer/src/components/WorktreeJumpPalette.mount-gating.test.tsx @@ -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() + 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: () => +})) + +vi.mock('@/components/repo/RepoBadgeLabel', () => ({ + RepoBadgeMark: () => +})) + +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 }) =>
{children}
, + CommandGroup: ({ children }: { children: React.ReactNode }) =>
{children}
, + CommandDialog: ({ children, open }: { children: React.ReactNode; open?: boolean }) => + open ?
{children}
: null, + CommandInput: React.forwardRef(function CommandInput( + _props: Record, + ref: React.ForwardedRef + ) { + return + }), + CommandList: React.forwardRef(function CommandList( + { children }: { children: React.ReactNode }, + ref: React.ForwardedRef + ) { + return
{children}
+ }), + CommandEmpty: ({ children }: { children: React.ReactNode }) =>
{children}
, + CommandItem: ({ children }: { children: React.ReactNode }) => ( + + ) + } +}) + +const initialAppState = useAppStore.getInitialState() +let testContainer: HTMLDivElement +let testRoot: Root + +async function flushEffects(): Promise { + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) +} + +async function churnClosedStore(count: number): Promise { + 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()) + 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()) + 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) + }) +}) diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index a8adbec77a9..b03d7026950 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -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 (~150–200ms) 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 ( + + ) +} + +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(null) const fallbackFocusOuterFrameRef = useRef(null) const fallbackFocusInnerFrameRef = useRef(null) - const createLookupGuard = useMemo(() => createWorktreePaletteRequestGuard(), []) const preserveCreateLookupOnCloseRef = useRef(false) const repoMap = useMemo(() => new Map(repos.map((r) => [r.id, r])), [repos])