mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(renderer): keep render-phase guards in state so discarded renders revert them (#34414)
QuickCommandsPane, useAddRepoCloneFlow, UntitledFileRenameDialog and the workspace board column resize hook guarded a render-phase setState with a ref written in the same render. When a sibling suspends, React discards that render and its queued updates but keeps the ref write, so the retry skipped the adjustment (no Add dialog, empty clone destination, stale save-as name, stale column width). Track each guard in state instead, and sync the column-resize handler refs in a layout effect. The other two sites from the issue (WorktreeMetaDialog prevIsOpenRef and use-native-chat-draft lastScopeKey) no longer exist on main: the dialog remounts a keyed draft and the chat draft reads via useSyncExternalStore. Fixes #25527 Co-authored-by: Matheus Breguêz <matbrgz@gmail.com>
This commit is contained in:
@@ -39,7 +39,7 @@ export function UntitledFileRenameDialog({
|
||||
const [error, setError] = useState<string | null>(null)
|
||||
const nameInputRef = useRef<HTMLInputElement>(null)
|
||||
const focusFrameRef = useRef<number | null>(null)
|
||||
const seededOpenStateRef = useRef({ open: false, baseName, worktreePath })
|
||||
const [seededOpenState, setSeededOpenState] = useState({ open: false, baseName, worktreePath })
|
||||
const mountedRef = useMountedRef()
|
||||
|
||||
const displayError = externalError ?? error
|
||||
@@ -64,15 +64,18 @@ export function UntitledFileRenameDialog({
|
||||
|
||||
// Seed before painting; Radix’s open lifecycle owns focus without a post-render effect.
|
||||
if (open) {
|
||||
const seeded = seededOpenStateRef.current
|
||||
if (!seeded.open || seeded.baseName !== baseName || seeded.worktreePath !== worktreePath) {
|
||||
seededOpenStateRef.current = { open: true, baseName, worktreePath }
|
||||
if (
|
||||
!seededOpenState.open ||
|
||||
seededOpenState.baseName !== baseName ||
|
||||
seededOpenState.worktreePath !== worktreePath
|
||||
) {
|
||||
setSeededOpenState({ open: true, baseName, worktreePath })
|
||||
setName(baseName)
|
||||
setDir(worktreePath)
|
||||
setError(null)
|
||||
}
|
||||
} else if (seededOpenStateRef.current.open) {
|
||||
seededOpenStateRef.current = { open: false, baseName, worktreePath }
|
||||
} else if (seededOpenState.open) {
|
||||
setSeededOpenState({ open: false, baseName, worktreePath })
|
||||
}
|
||||
|
||||
const handleBrowse = useCallback(async () => {
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { Suspense, use, type ReactNode } from 'react'
|
||||
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
|
||||
import { getDefaultSettings } from '../../../shared/constants'
|
||||
import { ConfirmationDialogContext } from './confirmation-dialog-context'
|
||||
import { UntitledFileRenameDialog } from './editor/UntitledFileRenameDialog'
|
||||
import { QuickCommandsPane } from './settings/QuickCommandsPane'
|
||||
import { useWorkspaceKanbanColumnResize } from './sidebar/use-workspace-kanban-column-resize'
|
||||
import { useAddRepoCloneFlow } from './sidebar/useAddRepoCloneFlow'
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
function SuspendUntil({ promise }: { promise: Promise<void> }): null {
|
||||
use(promise)
|
||||
return null
|
||||
}
|
||||
|
||||
// A sibling suspends after the adjusting render, so React discards that render and retries it.
|
||||
async function renderThenDiscardUpdate(
|
||||
initial: ReactNode,
|
||||
update: ReactNode
|
||||
): Promise<ReturnType<typeof render>> {
|
||||
const view = render(<Suspense fallback={null}>{initial}</Suspense>)
|
||||
let release = (): void => {}
|
||||
const suspended = new Promise<void>((resolve) => (release = resolve))
|
||||
await act(async () =>
|
||||
view.rerender(
|
||||
<Suspense fallback={null}>
|
||||
{update}
|
||||
<SuspendUntil promise={suspended} />
|
||||
</Suspense>
|
||||
)
|
||||
)
|
||||
await act(async () => {
|
||||
release()
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
})
|
||||
return view
|
||||
}
|
||||
|
||||
function KanbanColumn({ width, onCommit }: { width: number; onCommit: (w: number) => void }) {
|
||||
const resize = useWorkspaceKanbanColumnResize(width, onCommit)
|
||||
return (
|
||||
<div data-testid="column" onKeyDown={resize.onColumnResizeKeyDown}>
|
||||
{resize.columnWidth}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
it('applies a committed board column width', async () => {
|
||||
const onCommit = vi.fn()
|
||||
await renderThenDiscardUpdate(
|
||||
<KanbanColumn width={320} onCommit={onCommit} />,
|
||||
<KanbanColumn width={420} onCommit={onCommit} />
|
||||
)
|
||||
expect(screen.getByTestId('column').textContent).toBe('420')
|
||||
fireEvent.keyDown(screen.getByTestId('column'), { key: 'ArrowRight' })
|
||||
expect(onCommit).toHaveBeenLastCalledWith(440)
|
||||
})
|
||||
|
||||
function CloneDestination({ workspaceDir }: { workspaceDir?: string }) {
|
||||
const flow = useAddRepoCloneFlow({
|
||||
step: 'clone',
|
||||
activeRuntimeEnvironmentId: null,
|
||||
workspaceDir,
|
||||
fetchWorktrees: vi.fn(),
|
||||
onGitRepoReady: vi.fn()
|
||||
})
|
||||
return <div data-testid="destination">{flow.cloneDestination}</div>
|
||||
}
|
||||
|
||||
it('fills the clone destination once settings hydrate', async () => {
|
||||
await renderThenDiscardUpdate(
|
||||
<CloneDestination />,
|
||||
<CloneDestination workspaceDir="/home/dev/orca" />
|
||||
)
|
||||
expect(screen.getByTestId('destination').textContent).toBe('/home/dev/orca')
|
||||
})
|
||||
|
||||
it('seeds the save-as dialog with the opened file name', async () => {
|
||||
const props = { worktreePath: '/repo', onClose: vi.fn(), onConfirm: vi.fn() }
|
||||
await renderThenDiscardUpdate(
|
||||
<UntitledFileRenameDialog {...props} open={false} currentName="Untitled-1.md" />,
|
||||
<UntitledFileRenameDialog {...props} open currentName="Untitled-2.md" />
|
||||
)
|
||||
expect(screen.queryByDisplayValue('Untitled-2')).not.toBeNull()
|
||||
})
|
||||
|
||||
it('opens the Add dialog for a quick-command deep link', async () => {
|
||||
const settings = getDefaultSettings('/home/dev')
|
||||
const confirm = async (): Promise<boolean> => true
|
||||
await renderThenDiscardUpdate(
|
||||
<ConfirmationDialogContext value={confirm}>
|
||||
<QuickCommandsPane settings={settings} />
|
||||
</ConfirmationDialogContext>,
|
||||
<ConfirmationDialogContext value={confirm}>
|
||||
<QuickCommandsPane settings={settings} addCommandIntentSignal={1} />
|
||||
</ConfirmationDialogContext>
|
||||
)
|
||||
expect(screen.queryByText('Add Quick Command')).not.toBeNull()
|
||||
})
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react'
|
||||
import type { GlobalSettings } from '../../../../shared/global-settings-types'
|
||||
import type { TerminalQuickCommand } from '../../../../shared/terminal-quick-command-types'
|
||||
import { getTerminalQuickCommandScope } from '../../../../shared/terminal-quick-commands'
|
||||
@@ -125,7 +125,7 @@ export function QuickCommandsPane({
|
||||
const hostOptions = getTerminalQuickCommandHostOptions(settings, runtimeEnvironments)
|
||||
|
||||
const [editor, setEditor] = useState<EditorState>(null)
|
||||
const consumedAddIntentSignalRef = useRef(0)
|
||||
const [consumedAddIntentSignal, setConsumedAddIntentSignal] = useState(0)
|
||||
// Null means sticky All so newly added repos appear automatically, matching Tasks filtering.
|
||||
const [scopeSelection, setScopeSelection] = useState<ReadonlySet<string> | null>(null)
|
||||
const [scopePopoverOpen, setScopePopoverOpen] = useState(false)
|
||||
@@ -196,10 +196,10 @@ export function QuickCommandsPane({
|
||||
const intentSignal = addCommandIntentSignal
|
||||
if (
|
||||
typeof intentSignal === 'number' &&
|
||||
shouldOpenQuickCommandAddIntent(intentSignal, consumedAddIntentSignalRef.current)
|
||||
shouldOpenQuickCommandAddIntent(intentSignal, consumedAddIntentSignal)
|
||||
) {
|
||||
// Consume deep-link add intent before paint so the editor opens without a pane-only flash.
|
||||
consumedAddIntentSignalRef.current = intentSignal
|
||||
setConsumedAddIntentSignal(intentSignal)
|
||||
setEditor({
|
||||
mode: 'add',
|
||||
command: createDraftForCurrentFilter(),
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'
|
||||
import type React from 'react'
|
||||
import {
|
||||
WORKSPACE_BOARD_COLUMN_WIDTH_STEP,
|
||||
@@ -21,6 +21,7 @@ export function useWorkspaceKanbanColumnResize(
|
||||
)
|
||||
const [isResizingColumn, setIsResizingColumn] = useState(false)
|
||||
const nextCommittedWidth = clampWorkspaceBoardColumnWidth(committedWidth)
|
||||
const [renderedCommittedWidth, setRenderedCommittedWidth] = useState(nextCommittedWidth)
|
||||
const committedWidthRef = useRef(nextCommittedWidth)
|
||||
const commitWidthRef = useRef(onCommitWidth)
|
||||
const resizingRef = useRef(false)
|
||||
@@ -30,16 +31,20 @@ export function useWorkspaceKanbanColumnResize(
|
||||
const frameRef = useRef<number | null>(null)
|
||||
|
||||
commitWidthRef.current = onCommitWidth
|
||||
if (committedWidthRef.current !== nextCommittedWidth) {
|
||||
if (renderedCommittedWidth !== nextCommittedWidth) {
|
||||
setRenderedCommittedWidth(nextCommittedWidth)
|
||||
if (!resizingRef.current && columnWidth !== nextCommittedWidth) {
|
||||
// Apply external widths before rendering children; retain the local draft during active resize.
|
||||
setColumnWidth(nextCommittedWidth)
|
||||
}
|
||||
}
|
||||
// Sync handler refs after commit so a discarded render cannot leave them on an abandoned width.
|
||||
useLayoutEffect(() => {
|
||||
committedWidthRef.current = nextCommittedWidth
|
||||
if (!resizingRef.current) {
|
||||
draftWidthRef.current = nextCommittedWidth
|
||||
if (columnWidth !== nextCommittedWidth) {
|
||||
// Apply external widths before rendering children; retain the local draft during active resize.
|
||||
setColumnWidth(nextCommittedWidth)
|
||||
}
|
||||
}
|
||||
}
|
||||
}, [nextCommittedWidth])
|
||||
|
||||
const resetDocumentStyles = useCallback(() => {
|
||||
document.body.style.cursor = ''
|
||||
|
||||
@@ -63,7 +63,7 @@ export function useAddRepoCloneFlow({
|
||||
// Reject stale callbacks after a clone flow is reset or superseded.
|
||||
const cloneGenRef = useRef(0)
|
||||
// Allow late settings hydration to fill the default once per clone-step entry.
|
||||
const cloneStepAutoFilledRef = useRef(false)
|
||||
const [cloneStepAutoFilled, setCloneStepAutoFilled] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
if (!isCloning) {
|
||||
@@ -77,13 +77,15 @@ export function useAddRepoCloneFlow({
|
||||
cloneDestination,
|
||||
activeRuntimeEnvironmentId,
|
||||
workspaceDir,
|
||||
cloneStepAutoFilled: cloneStepAutoFilledRef.current
|
||||
cloneStepAutoFilled
|
||||
})
|
||||
if (step !== 'clone') {
|
||||
cloneStepAutoFilledRef.current = false
|
||||
if (cloneStepAutoFilled) {
|
||||
setCloneStepAutoFilled(false)
|
||||
}
|
||||
} else if (cloneDestinationAutoFill) {
|
||||
// Late defaults may seed local destinations; remote clone paths remain user-entered.
|
||||
cloneStepAutoFilledRef.current = true
|
||||
setCloneStepAutoFilled(true)
|
||||
setCloneDestination(cloneDestinationAutoFill.destination)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user