fix(dev): split the confirmation dialog so Fast Refresh can accept it (#11980)

* 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 <help@stably.ai>

* 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 `<Ctx value={...}>` 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 <help@stably.ai>

* 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 <help@stably.ai>
This commit is contained in:
Neil
2026-08-02 01:02:11 -07:00
committed by GitHub
co-authored by Orca
parent e58c051d16
commit 006ce9d116
21 changed files with 91 additions and 37 deletions
+1
View File
@@ -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",
+3
View File
@@ -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
@@ -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,
@@ -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,
+1 -1
View File
@@ -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,
@@ -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<boolean>
export const ConfirmationDialogContext = createContext<ConfirmationDialogContextValue | null>(null)
export function useConfirmationDialog(): ConfirmationDialogContextValue {
const confirm = useContext(ConfirmationDialogContext)
if (!confirm) {
throw new Error('useConfirmationDialog must be used inside ConfirmationDialogProvider')
}
return confirm
}
@@ -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')
})
})
@@ -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<boolean>
const ConfirmationDialogContext = createContext<ConfirmationDialogContextValue | null>(null)
export function ConfirmationDialogProvider({
children
}: {
@@ -116,11 +109,3 @@ export function ConfirmationDialogProvider({
</ConfirmationDialogContext.Provider>
)
}
export function useConfirmationDialog(): ConfirmationDialogContextValue {
const confirm = useContext(ConfirmationDialogContext)
if (!confirm) {
throw new Error('useConfirmationDialog must be used inside ConfirmationDialogProvider')
}
return confirm
}
@@ -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 {
@@ -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)
}))
@@ -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)
}))
@@ -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'
@@ -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)
}))
@@ -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
}))
@@ -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'
@@ -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'
@@ -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'
@@ -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,
@@ -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'
@@ -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),
@@ -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' }))