mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
Reduce renderer listener churn (#5831)
Co-authored-by: Orca <help@stably.ai> Co-authored-by: brennanb2025 <brennankbenson@gmail.com>
This commit is contained in:
co-authored by
Orca
brennanb2025
parent
6f6664e744
commit
357d293bb0
+44
-18
@@ -1386,10 +1386,50 @@ function App(): React.JSX.Element {
|
||||
)
|
||||
}
|
||||
|
||||
const globalShortcutStateRef = useRef({
|
||||
activeView,
|
||||
activeWorktreeId,
|
||||
actions,
|
||||
floatingTerminalOpen,
|
||||
floatingVisibleTabCount,
|
||||
keybindings,
|
||||
terminalShortcutPolicy: settings?.terminalShortcutPolicy,
|
||||
setFloatingTerminalOpenWithFocus,
|
||||
workspaceChromeActive,
|
||||
creationLayoutActive
|
||||
})
|
||||
// Why: window key listeners are global and long-lived; keep one registration
|
||||
// while letting the handler read current shortcut state on each key event.
|
||||
globalShortcutStateRef.current = {
|
||||
activeView,
|
||||
activeWorktreeId,
|
||||
actions,
|
||||
floatingTerminalOpen,
|
||||
floatingVisibleTabCount,
|
||||
keybindings,
|
||||
terminalShortcutPolicy: settings?.terminalShortcutPolicy,
|
||||
setFloatingTerminalOpenWithFocus,
|
||||
workspaceChromeActive,
|
||||
creationLayoutActive
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
const doubleTapDetector = new ModifierDoubleTapDetector()
|
||||
|
||||
const dispatchShortcutInput = (input: ShortcutDispatchInput): void => {
|
||||
const {
|
||||
activeView,
|
||||
activeWorktreeId,
|
||||
actions,
|
||||
floatingTerminalOpen,
|
||||
floatingVisibleTabCount,
|
||||
keybindings,
|
||||
terminalShortcutPolicy,
|
||||
setFloatingTerminalOpenWithFocus,
|
||||
workspaceChromeActive,
|
||||
creationLayoutActive
|
||||
} = globalShortcutStateRef.current
|
||||
|
||||
// Why: child-component handlers (e.g. terminal search Cmd+G / Cmd+Shift+G)
|
||||
// register on the same window capture phase and fire first. If they already
|
||||
// called preventDefault, this handler must not also act on the event —
|
||||
@@ -1415,13 +1455,10 @@ function App(): React.JSX.Element {
|
||||
const matchShortcut = (actionId: KeybindingActionId): boolean =>
|
||||
keybindingMatchesAction(actionId, input, shortcutPlatform, keybindings, {
|
||||
context,
|
||||
terminalShortcutPolicy: settings?.terminalShortcutPolicy
|
||||
terminalShortcutPolicy
|
||||
})
|
||||
const notifyTerminalCapture = (actionId: KeybindingActionId): void => {
|
||||
if (
|
||||
context !== 'terminal' ||
|
||||
(settings?.terminalShortcutPolicy ?? 'orca-first') !== 'orca-first'
|
||||
) {
|
||||
if (context !== 'terminal' || (terminalShortcutPolicy ?? 'orca-first') !== 'orca-first') {
|
||||
return
|
||||
}
|
||||
showTerminalShortcutCaptureNotification({
|
||||
@@ -1525,7 +1562,7 @@ function App(): React.JSX.Element {
|
||||
if (
|
||||
isFloatingWorkspacePanelShortcut(input, shortcutPlatform, null, keybindings, {
|
||||
context,
|
||||
terminalShortcutPolicy: settings?.terminalShortcutPolicy
|
||||
terminalShortcutPolicy
|
||||
})
|
||||
) {
|
||||
return
|
||||
@@ -1727,18 +1764,7 @@ function App(): React.JSX.Element {
|
||||
window.removeEventListener('keyup', onKeyUp, { capture: true })
|
||||
window.removeEventListener('blur', onBlur)
|
||||
}
|
||||
}, [
|
||||
activeView,
|
||||
activeWorktreeId,
|
||||
actions,
|
||||
floatingTerminalOpen,
|
||||
floatingVisibleTabCount,
|
||||
keybindings,
|
||||
settings?.terminalShortcutPolicy,
|
||||
setFloatingTerminalOpenWithFocus,
|
||||
workspaceChromeActive,
|
||||
creationLayoutActive
|
||||
])
|
||||
}, [])
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const controls = titlebarLeftControlsRef.current
|
||||
|
||||
@@ -117,4 +117,43 @@ describe('createRuntimeClientEventsSync', () => {
|
||||
await flush()
|
||||
expect(h.recordsFor('C')[0].unsubscribe).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('retries failed desired subscriptions without another store-driven sync', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
let desired = ['A']
|
||||
let attempt = 0
|
||||
const unsubscribe = vi.fn()
|
||||
const subscribe = vi.fn((): Promise<RuntimeClientEventSubscriptionHandle> => {
|
||||
attempt += 1
|
||||
if (attempt === 1) {
|
||||
return Promise.reject(new Error('temporary subscribe failure'))
|
||||
}
|
||||
return Promise.resolve({ unsubscribe })
|
||||
})
|
||||
const sync = createRuntimeClientEventsSync({
|
||||
getDesiredEnvironmentIds: () => desired,
|
||||
subscribe,
|
||||
onEvent: vi.fn(),
|
||||
retryDelayMs: 10
|
||||
})
|
||||
|
||||
sync.sync()
|
||||
await Promise.resolve()
|
||||
expect(subscribe).toHaveBeenCalledTimes(1)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(9)
|
||||
expect(subscribe).toHaveBeenCalledTimes(1)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
await Promise.resolve()
|
||||
expect(subscribe).toHaveBeenCalledTimes(2)
|
||||
|
||||
desired = []
|
||||
sync.sync()
|
||||
expect(unsubscribe).toHaveBeenCalledTimes(1)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -14,6 +14,7 @@ export type RuntimeClientEventsSyncDeps = {
|
||||
onError: (error: unknown) => void
|
||||
) => Promise<RuntimeClientEventSubscriptionHandle>
|
||||
onEvent: (environmentId: string, event: RuntimeClientEvent) => void
|
||||
retryDelayMs?: number
|
||||
}
|
||||
|
||||
export type RuntimeClientEventsSync = {
|
||||
@@ -44,8 +45,38 @@ export function createRuntimeClientEventsSync(
|
||||
): RuntimeClientEventsSync {
|
||||
const subscriptions = new Map<string, () => void>()
|
||||
const pending = new Set<string>()
|
||||
const retryTimers = new Map<string, ReturnType<typeof setTimeout>>()
|
||||
const retryDelayMs = deps.retryDelayMs ?? 1_000
|
||||
let generation = 0
|
||||
|
||||
const clearRetryTimer = (environmentId: string): void => {
|
||||
const retryTimer = retryTimers.get(environmentId)
|
||||
if (!retryTimer) {
|
||||
return
|
||||
}
|
||||
clearTimeout(retryTimer)
|
||||
retryTimers.delete(environmentId)
|
||||
}
|
||||
|
||||
const scheduleRetry = (environmentId: string, subscribeGeneration: number): void => {
|
||||
if (retryTimers.has(environmentId)) {
|
||||
return
|
||||
}
|
||||
// Why: useIpcEvents no longer retries on every store mutation; transient
|
||||
// subscribe failures still need a bounded retry while the env remains desired.
|
||||
const retryTimer = setTimeout(() => {
|
||||
retryTimers.delete(environmentId)
|
||||
if (
|
||||
subscribeGeneration !== generation ||
|
||||
!deps.getDesiredEnvironmentIds().includes(environmentId)
|
||||
) {
|
||||
return
|
||||
}
|
||||
sync()
|
||||
}, retryDelayMs)
|
||||
retryTimers.set(environmentId, retryTimer)
|
||||
}
|
||||
|
||||
const stop = (): void => {
|
||||
generation += 1
|
||||
for (const unsubscribe of subscriptions.values()) {
|
||||
@@ -53,10 +84,20 @@ export function createRuntimeClientEventsSync(
|
||||
}
|
||||
subscriptions.clear()
|
||||
pending.clear()
|
||||
for (const retryTimer of retryTimers.values()) {
|
||||
clearTimeout(retryTimer)
|
||||
}
|
||||
retryTimers.clear()
|
||||
}
|
||||
|
||||
const sync = (): void => {
|
||||
const desiredIds = new Set(deps.getDesiredEnvironmentIds())
|
||||
for (const environmentId of retryTimers.keys()) {
|
||||
if (desiredIds.has(environmentId)) {
|
||||
continue
|
||||
}
|
||||
clearRetryTimer(environmentId)
|
||||
}
|
||||
|
||||
for (const [environmentId, unsubscribe] of subscriptions) {
|
||||
if (desiredIds.has(environmentId)) {
|
||||
@@ -70,6 +111,7 @@ export function createRuntimeClientEventsSync(
|
||||
if (subscriptions.has(environmentId) || pending.has(environmentId)) {
|
||||
continue
|
||||
}
|
||||
clearRetryTimer(environmentId)
|
||||
pending.add(environmentId)
|
||||
const subscribeGeneration = generation
|
||||
void deps
|
||||
@@ -103,6 +145,9 @@ export function createRuntimeClientEventsSync(
|
||||
pending.delete(environmentId)
|
||||
if (subscribeGeneration === generation) {
|
||||
console.warn('[runtime-client-events] failed to subscribe:', error)
|
||||
if (deps.getDesiredEnvironmentIds().includes(environmentId)) {
|
||||
scheduleRetry(environmentId, subscribeGeneration)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
import type * as ReactModule from 'react'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
buildRuntimeClientEventEnvironmentKey,
|
||||
buildNewWorkspaceShortcutModalData,
|
||||
openNewWorkspaceFromShortcut,
|
||||
resolveBrowserSessionTabTarget,
|
||||
@@ -26,6 +27,14 @@ const STALE_PANE_KEY = makePaneKey('tab-future', STALE_LEAF_ID)
|
||||
const ORPHAN_PANE_KEY = makePaneKey('tab-orphan', ORPHAN_LEAF_ID)
|
||||
const TAB_1_PANE_KEY = makePaneKey('tab-1', TAB_1_LEAF_ID)
|
||||
|
||||
describe('buildRuntimeClientEventEnvironmentKey', () => {
|
||||
it('treats runtime environment ids as a stable set', () => {
|
||||
expect(buildRuntimeClientEventEnvironmentKey(['env-b', 'env-a', 'env-b'])).toBe(
|
||||
buildRuntimeClientEventEnvironmentKey(['env-a', 'env-b'])
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
function expectWorktreeRouting(worktreeId: string): unknown {
|
||||
return expect.objectContaining({ worktreeId })
|
||||
}
|
||||
|
||||
@@ -712,6 +712,14 @@ function getRuntimeClientEventEnvironmentIds(): string[] {
|
||||
return [...ids]
|
||||
}
|
||||
|
||||
export function buildRuntimeClientEventEnvironmentKey(environmentIds: string[]): string {
|
||||
return [...new Set(environmentIds)].sort().join('\u0000')
|
||||
}
|
||||
|
||||
function getRuntimeClientEventEnvironmentKey(): string {
|
||||
return buildRuntimeClientEventEnvironmentKey(getRuntimeClientEventEnvironmentIds())
|
||||
}
|
||||
|
||||
function getWorktreeRuntimeEnvironmentId(worktreeId: string | null | undefined): string | null {
|
||||
return getRuntimeEnvironmentIdForWorktree(useAppStore.getState(), worktreeId)
|
||||
}
|
||||
@@ -886,7 +894,17 @@ export function useIpcEvents(): void {
|
||||
})
|
||||
|
||||
runtimeClientEventsSync.sync()
|
||||
unsubs.push(useAppStore.subscribe(runtimeClientEventsSync.sync))
|
||||
let runtimeClientEventEnvironmentKey = getRuntimeClientEventEnvironmentKey()
|
||||
unsubs.push(
|
||||
useAppStore.subscribe(() => {
|
||||
const nextKey = getRuntimeClientEventEnvironmentKey()
|
||||
if (nextKey === runtimeClientEventEnvironmentKey) {
|
||||
return
|
||||
}
|
||||
runtimeClientEventEnvironmentKey = nextKey
|
||||
runtimeClientEventsSync.sync()
|
||||
})
|
||||
)
|
||||
unsubs.push(runtimeClientEventsSync.stop)
|
||||
|
||||
unsubs.push(
|
||||
|
||||
Reference in New Issue
Block a user