From 006ce9d1163ed37080d6ba6ef8b0dfe2cec1e461 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 2 Aug 2026 01:02:11 -0700 Subject: [PATCH] fix(dev): split the confirmation dialog so Fast Refresh can accept it (#11980) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(dev): split the confirmation dialog so Fast Refresh can accept it `confirmation-dialog.tsx` exported both `ConfirmationDialogProvider` and `useConfirmationDialog`, so React Fast Refresh could never treat it as a boundary and Vite applied every edit to it in two passes under two `?t=` stamps. When a second file in the same subtree changed in one watcher batch, `createContext` ran twice and the provider published one context object while the consumer read the other — `useContext` returned null and the hook threw. Two field crash reports hit this at `ChecksPanel`, both dev-server sessions. The context and hook move to a new component-free `confirmation-dialog-context.ts`; `confirmation-dialog.tsx` keeps the provider and now exports only a component, so the refresh runtime accepts it. Not one line of the provider body changes — the 16 hook importers just point at the new module, `vi.mock` targets included, and `App.tsx` is untouched. * test(dev): pin the confirmation dialog Fast Refresh boundary The split that fixed the context-identity crash had no test behind it: no test imported ConfirmationDialogProvider, and the six vi.mock call sites replace the hook module wholesale, so they pass just as well with the provider and hook back in one file. Assert the module shapes the refresh transform actually keys on -- the context module registers no component, so it never gets an HMR footer to invalidate through. Co-authored-by: Orca * test(dev): assert the refresh boundary on the module namespace The source-regex guard did not guard. Its patterns match only declaration forms, so `export { useConfirmationDialog } from './confirmation-dialog-context'` in the provider module -- which restores the crash, verified in a browser -- passed it 3/3. It also failed on a comment that merely contained the word createContext, and would fail on React 19's `` shorthand. Assert on the module namespace object instead, using react-refresh's own component criterion, so re-exports and default exports are visible. The third test renders the provider and resolves the hook through it, which is a real behavioural check rather than a shape one. Co-authored-by: Orca * test(dev): classify boundary exports with the refresh runtime's own predicate The hand-rolled `^[A-Z]` name check called `export class Foo {}` a component; the runtime rejects any class whose prototype carries extra members, so that shape restored the two-pass split undetected. Use react-refresh's exported `isLikelyComponentType` and mirror `isCompoundComponent` instead of a third approximation. react-refresh was already resolvable only via shamefully-hoist, so it is now an explicit devDependency. * test(dev): tighten confirmation dialog boundary guard --------- Co-authored-by: Orca --- package.json | 1 + pnpm-lock.yaml | 3 ++ .../src/components/GitHubItemDialog.tsx | 2 +- .../src/components/PullRequestPage.tsx | 2 +- src/renderer/src/components/TaskPage.tsx | 2 +- .../components/confirmation-dialog-context.ts | 25 ++++++++++++ ...nfirmation-dialog-refresh-boundary.test.ts | 38 +++++++++++++++++++ .../src/components/confirmation-dialog.tsx | 27 +++---------- .../components/right-sidebar/ChecksPanel.tsx | 2 +- ...SourceControl.open-file-highlight.test.tsx | 2 +- .../SourceControl.preview-open.test.tsx | 2 +- .../right-sidebar/SourceControl.tsx | 2 +- .../SourceControl.virtual-file-list.test.tsx | 2 +- .../use-hosted-review-actions.test.tsx | 2 +- .../use-hosted-review-actions.ts | 2 +- .../right-sidebar/useFileDeletion.ts | 2 +- .../components/settings/QuickCommandsPane.tsx | 2 +- .../src/components/settings/Settings.tsx | 2 +- .../tab-bar/TabBarQuickCommandsButton.tsx | 2 +- ...e-explorer-delete-owner-provenance.test.ts | 4 +- .../remote-host-file-delete-repro.test.ts | 2 +- 21 files changed, 91 insertions(+), 37 deletions(-) create mode 100644 src/renderer/src/components/confirmation-dialog-context.ts create mode 100644 src/renderer/src/components/confirmation-dialog-refresh-boundary.test.ts diff --git a/package.json b/package.json index 52da12d0278..b68ff2de99a 100644 --- a/package.json +++ b/package.json @@ -219,6 +219,7 @@ "react-dom": "^19.2.7", "react-grab": "^0.1.33", "react-markdown": "^10.1.0", + "react-refresh": "^0.18.0", "rehype-highlight": "^7.0.2", "rehype-katex": "^7.0.1", "rehype-raw": "^7.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 491c536773b..dbe19cf1e7a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -324,6 +324,9 @@ importers: react-markdown: specifier: ^10.1.0 version: 10.1.0(@types/react@19.2.17)(react@19.2.7) + react-refresh: + specifier: ^0.18.0 + version: 0.18.0 rehype-highlight: specifier: ^7.0.2 version: 7.0.2 diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index dfc2fe90afe..f17e104ddfa 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -57,7 +57,7 @@ import { Button } from '@/components/ui/button' import { ButtonGroup } from '@/components/ui/button-group' import { Input } from '@/components/ui/input' import { useMountedRef } from '@/hooks/useMountedRef' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { Accordion, AccordionContent, diff --git a/src/renderer/src/components/PullRequestPage.tsx b/src/renderer/src/components/PullRequestPage.tsx index c00abfad692..8e6063583fd 100644 --- a/src/renderer/src/components/PullRequestPage.tsx +++ b/src/renderer/src/components/PullRequestPage.tsx @@ -50,7 +50,7 @@ import { Button } from '@/components/ui/button' import { ButtonGroup } from '@/components/ui/button-group' import { Input } from '@/components/ui/input' import { useMountedRef } from '@/hooks/useMountedRef' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { Accordion, AccordionContent, diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 5c30fa545d0..3ff8a909b7e 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -103,7 +103,7 @@ import type { TaskSourceAvailabilityNotice, TaskSourceHostAvailability } from './task-source-context-summary' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { getGitHubPRPrimaryReviewer, getGitHubPRReviewerRows, diff --git a/src/renderer/src/components/confirmation-dialog-context.ts b/src/renderer/src/components/confirmation-dialog-context.ts new file mode 100644 index 00000000000..94a0ebc9984 --- /dev/null +++ b/src/renderer/src/components/confirmation-dialog-context.ts @@ -0,0 +1,25 @@ +import { createContext, useContext } from 'react' + +// Keep the context component-free so Fast Refresh preserves its identity. + +export type ConfirmationDialogOptions = { + title: string + description?: string + confirmLabel?: string + cancelLabel?: string + confirmVariant?: 'default' | 'destructive' +} + +export type ConfirmationDialogContextValue = ( + options: ConfirmationDialogOptions +) => Promise + +export const ConfirmationDialogContext = createContext(null) + +export function useConfirmationDialog(): ConfirmationDialogContextValue { + const confirm = useContext(ConfirmationDialogContext) + if (!confirm) { + throw new Error('useConfirmationDialog must be used inside ConfirmationDialogProvider') + } + return confirm +} diff --git a/src/renderer/src/components/confirmation-dialog-refresh-boundary.test.ts b/src/renderer/src/components/confirmation-dialog-refresh-boundary.test.ts new file mode 100644 index 00000000000..952cb21e23d --- /dev/null +++ b/src/renderer/src/components/confirmation-dialog-refresh-boundary.test.ts @@ -0,0 +1,38 @@ +// @vitest-environment happy-dom +import { createRequire } from 'node:module' +import { createElement, type ReactNode } from 'react' +import { renderHook } from '@testing-library/react' +import { describe, expect, it } from 'vitest' +import * as contextModule from '@/components/confirmation-dialog-context' +import * as providerModule from '@/components/confirmation-dialog' +import { ConfirmationDialogProvider } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' + +// react-refresh ships no types; take the one binding this needs. +const { isLikelyComponentType } = createRequire(import.meta.url)('react-refresh/runtime') as { + isLikelyComponentType: (value: unknown) => boolean +} + +describe('confirmation dialog Fast Refresh boundary', () => { + it('exports nothing from the provider module that invalidates the refresh boundary', () => { + expect(Object.keys(providerModule)).toEqual(['ConfirmationDialogProvider']) + expect(isLikelyComponentType(providerModule.ConfirmationDialogProvider)).toBe(true) + }) + + it('keeps the context and hook in a component-free module', () => { + const components = Object.entries(contextModule) + .filter(([, value]) => isLikelyComponentType(value)) + .map(([name]) => name) + + expect(components).toEqual([]) + expect(typeof contextModule.useConfirmationDialog).toBe('function') + }) + + it('resolves the hook against the context the provider publishes', () => { + const wrapper = ({ children }: { children: ReactNode }) => + createElement(ConfirmationDialogProvider, null, children) + const { result } = renderHook(() => useConfirmationDialog(), { wrapper }) + + expect(typeof result.current).toBe('function') + }) +}) diff --git a/src/renderer/src/components/confirmation-dialog.tsx b/src/renderer/src/components/confirmation-dialog.tsx index f13285c4907..3011078c33b 100644 --- a/src/renderer/src/components/confirmation-dialog.tsx +++ b/src/renderer/src/components/confirmation-dialog.tsx @@ -1,4 +1,4 @@ -import React, { createContext, useCallback, useContext, useEffect, useRef, useState } from 'react' +import React, { useCallback, useEffect, useRef, useState } from 'react' import { Button } from '@/components/ui/button' import { @@ -9,27 +9,20 @@ import { DialogHeader, DialogTitle } from '@/components/ui/dialog' +import { + ConfirmationDialogContext, + type ConfirmationDialogContextValue, + type ConfirmationDialogOptions +} from '@/components/confirmation-dialog-context' import { useAppStore } from '@/store' import { translate } from '@/i18n/i18n' -type ConfirmationDialogOptions = { - title: string - description?: string - confirmLabel?: string - cancelLabel?: string - confirmVariant?: 'default' | 'destructive' -} - type ConfirmationDialogRequest = { id: number options: ConfirmationDialogOptions resolve: (confirmed: boolean) => void } -type ConfirmationDialogContextValue = (options: ConfirmationDialogOptions) => Promise - -const ConfirmationDialogContext = createContext(null) - export function ConfirmationDialogProvider({ children }: { @@ -116,11 +109,3 @@ export function ConfirmationDialogProvider({ ) } - -export function useConfirmationDialog(): ConfirmationDialogContextValue { - const confirm = useContext(ConfirmationDialogContext) - if (!confirm) { - throw new Error('useConfirmationDialog must be used inside ConfirmationDialogProvider') - } - return confirm -} diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 30fade22496..07c72d5eaa1 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -89,7 +89,7 @@ import { normalizeGlobalWindowsRuntimeDefault } from '../../../../shared/project import { normalizeHostedReviewHeadRef } from '../../../../shared/hosted-review-refs' import { getHostedReviewCacheKey, refreshHostedReviewCard } from '@/store/slices/hosted-review' import { toast } from 'sonner' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { type ChecksPanelReview, selectChecksPanelReview } from './checks-panel-review' import { selectReviewCacheEntry } from './review-cache-entry-selection' import { diff --git a/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx index bddef9ef29c..8624d25fae2 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.open-file-highlight.test.tsx @@ -61,7 +61,7 @@ vi.mock('@/store/selectors', () => ({ useWorktreeMap: () => new Map([[mocks.activeWorktree.id, mocks.activeWorktree]]) })) -vi.mock('@/components/confirmation-dialog', () => ({ +vi.mock('@/components/confirmation-dialog-context', () => ({ useConfirmationDialog: () => vi.fn().mockResolvedValue(true) })) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx index 6d0121157bb..22bc4954b20 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.preview-open.test.tsx @@ -78,7 +78,7 @@ vi.mock('@/store/selectors', () => ({ useWorktreeMap: () => new Map([[mocks.activeWorktree.id, mocks.activeWorktree]]) })) -vi.mock('@/components/confirmation-dialog', () => ({ +vi.mock('@/components/confirmation-dialog-context', () => ({ useConfirmationDialog: () => vi.fn().mockResolvedValue(true) })) diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index a579b4ff846..3322ef5c71b 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -149,7 +149,7 @@ import { DialogTitle } from '@/components/ui/dialog' import { BaseRefPicker } from '@/components/settings/BaseRefPicker' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { formatDiffComment, formatDiffComments } from '@/lib/diff-comments-format' import { getDiffCommentLineLabel, getDiffCommentSource } from '@/lib/diff-comment-compat' import { DiffNotesSendMenu } from '@/components/editor/DiffNotesSendMenu' diff --git a/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx b/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx index 2dc5f0bc474..7e1d4c7d2ec 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.virtual-file-list.test.tsx @@ -66,7 +66,7 @@ vi.mock('@/store/selectors', () => ({ useWorktreeMap: () => new Map([[mocks.activeWorktree.id, mocks.activeWorktree]]) })) -vi.mock('@/components/confirmation-dialog', () => ({ +vi.mock('@/components/confirmation-dialog-context', () => ({ useConfirmationDialog: () => vi.fn().mockResolvedValue(true) })) diff --git a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx index 82527dc8ed5..1eae2a150bc 100644 --- a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx +++ b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.test.tsx @@ -14,7 +14,7 @@ const runtimeRpcMocks = vi.hoisted(() => ({ callRuntimeRpc: vi.fn() })) -vi.mock('@/components/confirmation-dialog', () => ({ +vi.mock('@/components/confirmation-dialog-context', () => ({ useConfirmationDialog: () => confirmationMocks.confirm })) diff --git a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts index 331ada85d34..eec79060650 100644 --- a/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts +++ b/src/renderer/src/components/right-sidebar/use-hosted-review-actions.ts @@ -1,6 +1,6 @@ import { useCallback, useState } from 'react' import { toast } from 'sonner' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import type { GitHubPRAutoMergeAction } from '@/components/github-pr-merge-state' import type { HostedReviewInfo } from '../../../../shared/hosted-review' import type { PRInfo, Repo, GitHubPRMergeMethod } from '../../../../shared/types' diff --git a/src/renderer/src/components/right-sidebar/useFileDeletion.ts b/src/renderer/src/components/right-sidebar/useFileDeletion.ts index 4c5a0c9ec3f..6e5dcc5af54 100644 --- a/src/renderer/src/components/right-sidebar/useFileDeletion.ts +++ b/src/renderer/src/components/right-sidebar/useFileDeletion.ts @@ -1,7 +1,7 @@ import { useCallback, useMemo, useRef } from 'react' import { toast } from 'sonner' import { useAppStore } from '@/store' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { dirname } from '@/lib/path' import { useShortcutLabel } from '@/hooks/useShortcutLabel' import { isPathEqualOrDescendant } from './file-explorer-paths' diff --git a/src/renderer/src/components/settings/QuickCommandsPane.tsx b/src/renderer/src/components/settings/QuickCommandsPane.tsx index 4b70c72c900..526255160b6 100644 --- a/src/renderer/src/components/settings/QuickCommandsPane.tsx +++ b/src/renderer/src/components/settings/QuickCommandsPane.tsx @@ -9,7 +9,7 @@ import { import { useAppStore } from '../../store' import { Button } from '../ui/button' import { Label } from '../ui/label' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { getSettingOwnershipSummary } from './setting-ownership' import { translate } from '@/i18n/i18n' import { QuickCommandsList } from './QuickCommandsList' diff --git a/src/renderer/src/components/settings/Settings.tsx b/src/renderer/src/components/settings/Settings.tsx index 17a251bea15..fe894fa07e6 100644 --- a/src/renderer/src/components/settings/Settings.tsx +++ b/src/renderer/src/components/settings/Settings.tsx @@ -22,7 +22,7 @@ import { useAppStore } from '../../store' import { useSystemPrefersDark } from '@/components/terminal-pane/use-system-prefers-dark' import { isMacUserAgent, isWindowsUserAgent } from '@/components/terminal-pane/pane-helpers' import { applyDocumentTheme } from '@/lib/document-theme' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { SCROLLBACK_PRESETS_ROWS, getFallbackTerminalFonts, diff --git a/src/renderer/src/components/tab-bar/TabBarQuickCommandsButton.tsx b/src/renderer/src/components/tab-bar/TabBarQuickCommandsButton.tsx index 0810ea9c2b2..d972edda0d0 100644 --- a/src/renderer/src/components/tab-bar/TabBarQuickCommandsButton.tsx +++ b/src/renderer/src/components/tab-bar/TabBarQuickCommandsButton.tsx @@ -14,7 +14,7 @@ import { getRepoIdFromWorktreeId } from '../../../../shared/worktree-id' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { runQuickCommandInNewTab } from '@/lib/run-quick-command-in-new-tab' import type { TerminalQuickCommand } from '../../../../shared/types' -import { useConfirmationDialog } from '@/components/confirmation-dialog' +import { useConfirmationDialog } from '@/components/confirmation-dialog-context' import { translate } from '@/i18n/i18n' import { TabBarQuickCommandsMenu } from './TabBarQuickCommandsMenu' diff --git a/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts b/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts index ce426695e6f..20a325a5d7d 100644 --- a/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts +++ b/src/renderer/src/runtime/file-explorer-delete-owner-provenance.test.ts @@ -23,7 +23,9 @@ const fsDeletePath = vi.fn() const fsRenamePath = vi.fn() const runtimeEnvironmentCall = vi.fn() -vi.mock('@/components/confirmation-dialog', () => ({ useConfirmationDialog: () => confirm })) +vi.mock('@/components/confirmation-dialog-context', () => ({ + useConfirmationDialog: () => confirm +})) vi.mock('@/hooks/useShortcutLabel', () => ({ useShortcutLabel: () => 'Delete' })) vi.mock('@/components/editor/editor-autosave', () => ({ requestEditorFileSave: vi.fn().mockResolvedValue(undefined), diff --git a/src/renderer/src/runtime/remote-host-file-delete-repro.test.ts b/src/renderer/src/runtime/remote-host-file-delete-repro.test.ts index a3aa3592a97..a9c144499c9 100644 --- a/src/renderer/src/runtime/remote-host-file-delete-repro.test.ts +++ b/src/renderer/src/runtime/remote-host-file-delete-repro.test.ts @@ -18,7 +18,7 @@ const fsReadFile = vi.fn() const fsDeletePath = vi.fn() const runtimeEnvironmentCall = vi.fn() -vi.mock('@/components/confirmation-dialog', () => ({ +vi.mock('@/components/confirmation-dialog-context', () => ({ useConfirmationDialog: () => confirm })) vi.mock('@/hooks/useShortcutLabel', () => ({ useShortcutLabel: () => 'Delete' }))