mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf(settings): commit free-text account settings on a debounce, not per keystroke (#18651)
* perf(settings): commit free-text account settings on a debounce, not per keystroke Four raw text inputs bound value straight to the store and called updateSettings in onChange, so every character was an IPC round trip that replaced the settings object identity in every other window, re-rendering everything subscribed to it. Route them through a DebouncedSettingsTextInput that keeps a local draft and commits after 700ms, on blur, and on unmount — matching the repository-hook script draft. The draft lives in the input component because the account sections are render functions the settings search calls conditionally, so hooks cannot legally live in them. * fix(settings): update the draft's latest-commit ref in an effect, not during render * fix(settings): flush a pending text-setting draft on beforeunload and drop the dirty flag A window close or app quit never unmounts the React tree, so the unmount flush could not run and a value typed within the last 700ms was lost. The close coordinator already dispatches a synthetic beforeunload while the tree is mounted, so listen for it the same way the session checkpoint does. The pending timer is now the single source of truth for "uncommitted edits"; the separate dirty flag it duplicated is gone.
This commit is contained in:
@@ -0,0 +1,39 @@
|
||||
import type React from 'react'
|
||||
import { Input } from '../ui/input'
|
||||
import { useDebouncedSettingsTextDraft } from './use-debounced-settings-text-draft'
|
||||
|
||||
type DebouncedSettingsTextInputProps = Omit<
|
||||
React.ComponentProps<typeof Input>,
|
||||
'value' | 'onChange' | 'onBlur'
|
||||
> & {
|
||||
value: string
|
||||
commit: (next: string) => void
|
||||
onEdit?: () => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Text input for a free-text setting, committed on a debounce instead of per keystroke.
|
||||
*
|
||||
* Why a component and not a hook at the call site: the account sections are render functions the
|
||||
* settings search calls conditionally, so hooks cannot live in them. Rendering this as JSX gives
|
||||
* the draft its own component to mount and unmount with.
|
||||
*/
|
||||
export function DebouncedSettingsTextInput({
|
||||
value,
|
||||
commit,
|
||||
onEdit,
|
||||
...inputProps
|
||||
}: DebouncedSettingsTextInputProps): React.JSX.Element {
|
||||
const draft = useDebouncedSettingsTextDraft({ value, commit })
|
||||
return (
|
||||
<Input
|
||||
{...inputProps}
|
||||
value={draft.value}
|
||||
onChange={(event) => {
|
||||
onEdit?.()
|
||||
draft.onChange(event.target.value)
|
||||
}}
|
||||
onBlur={draft.onBlur}
|
||||
/>
|
||||
)
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import { Popover, PopoverContent, PopoverTrigger } from '../ui/popover'
|
||||
import { MiniMaxIcon } from '../status-bar/icons'
|
||||
import { SearchableSetting } from './SearchableSetting'
|
||||
import type { AccountsPaneSectionModel } from './accounts-pane-types'
|
||||
import { DebouncedSettingsTextInput } from './DebouncedSettingsTextInput'
|
||||
|
||||
const MINIMAX_CONSOLE_URL = 'https://platform.minimax.io/console/usage'
|
||||
|
||||
@@ -265,10 +266,10 @@ export function renderMiniMaxAccountsSection(model: AccountsPaneSectionModel): R
|
||||
<Label>
|
||||
{translate('auto.components.settings.AccountsPane.bf160bb6c0', 'Group ID override')}
|
||||
</Label>
|
||||
<Input
|
||||
<DebouncedSettingsTextInput
|
||||
type="text"
|
||||
value={settings.minimaxGroupId}
|
||||
onChange={(e) => updateSettings({ minimaxGroupId: e.target.value })}
|
||||
commit={(minimaxGroupId) => updateSettings({ minimaxGroupId })}
|
||||
placeholder={translate(
|
||||
'auto.components.settings.AccountsPane.0747d6391a',
|
||||
'Use group ID from cookie'
|
||||
@@ -290,10 +291,10 @@ export function renderMiniMaxAccountsSection(model: AccountsPaneSectionModel): R
|
||||
<Label>
|
||||
{translate('auto.components.settings.AccountsPane.4ff2af7524', 'Usage model names')}
|
||||
</Label>
|
||||
<Input
|
||||
<DebouncedSettingsTextInput
|
||||
type="text"
|
||||
value={settings.minimaxUsageModels}
|
||||
onChange={(e) => updateSettings({ minimaxUsageModels: e.target.value })}
|
||||
commit={(minimaxUsageModels) => updateSettings({ minimaxUsageModels })}
|
||||
placeholder={translate('auto.components.settings.AccountsPane.3c92b0d31c', 'general')}
|
||||
spellCheck={false}
|
||||
className="text-xs"
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { Button } from '../ui/button'
|
||||
import { Input } from '../ui/input'
|
||||
import { Label } from '../ui/label'
|
||||
import { Switch } from '../ui/switch'
|
||||
import { GeminiIcon, OpenCodeGoIcon } from '../status-bar/icons'
|
||||
import { SearchableSetting } from './SearchableSetting'
|
||||
import type { AccountsPaneSectionModel } from './accounts-pane-types'
|
||||
import { DebouncedSettingsTextInput } from './DebouncedSettingsTextInput'
|
||||
|
||||
export function renderGeminiAccountsSection(model: AccountsPaneSectionModel): React.JSX.Element {
|
||||
const { localAccountRuntimeSentenceLabel, recordFeatureInteraction, settings, updateSettings } =
|
||||
@@ -114,13 +114,11 @@ export function renderOpenCodeAccountsSection(model: AccountsPaneSectionModel):
|
||||
)}
|
||||
</Label>
|
||||
<div className="flex gap-2">
|
||||
<Input
|
||||
<DebouncedSettingsTextInput
|
||||
type="password"
|
||||
value={settings.opencodeSessionCookie}
|
||||
onChange={(e) => {
|
||||
recordOpenCodeSettingEdit('cookie')
|
||||
updateSettings({ opencodeSessionCookie: e.target.value })
|
||||
}}
|
||||
onEdit={() => recordOpenCodeSettingEdit('cookie')}
|
||||
commit={(opencodeSessionCookie) => updateSettings({ opencodeSessionCookie })}
|
||||
placeholder={translate(
|
||||
'auto.components.settings.AccountsPane.a7e38affcd',
|
||||
'Fe26.2**… token or auth=Fe26.2**… header'
|
||||
@@ -180,13 +178,11 @@ export function renderOpenCodeAccountsSection(model: AccountsPaneSectionModel):
|
||||
{translate('auto.components.settings.AccountsPane.dbdb0b0bd8', 'Workspace ID override')}
|
||||
</Label>
|
||||
<div className="flex gap-2">
|
||||
<Input
|
||||
<DebouncedSettingsTextInput
|
||||
type="text"
|
||||
value={settings.opencodeWorkspaceId}
|
||||
onChange={(e) => {
|
||||
recordOpenCodeSettingEdit('workspaceId')
|
||||
updateSettings({ opencodeWorkspaceId: e.target.value })
|
||||
}}
|
||||
onEdit={() => recordOpenCodeSettingEdit('workspaceId')}
|
||||
commit={(opencodeWorkspaceId) => updateSettings({ opencodeWorkspaceId })}
|
||||
placeholder={translate(
|
||||
'auto.components.settings.AccountsPane.a122332371',
|
||||
'wrk_… (leave blank for automatic lookup)'
|
||||
|
||||
@@ -0,0 +1,222 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { StrictMode } from 'react'
|
||||
import { act, cleanup, renderHook } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { useDebouncedSettingsTextDraft } from './use-debounced-settings-text-draft'
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
describe('useDebouncedSettingsTextDraft', () => {
|
||||
it('shows every keystroke immediately but commits once', () => {
|
||||
const commit = vi.fn()
|
||||
const { result } = renderHook(() => useDebouncedSettingsTextDraft({ value: '', commit }))
|
||||
|
||||
for (const next of ['w', 'wr', 'wrk']) {
|
||||
act(() => result.current.onChange(next))
|
||||
}
|
||||
|
||||
expect(result.current.value).toBe('wrk')
|
||||
expect(commit).not.toHaveBeenCalled()
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
|
||||
expect(commit).toHaveBeenCalledTimes(1)
|
||||
expect(commit).toHaveBeenCalledWith('wrk')
|
||||
})
|
||||
|
||||
it('commits immediately on blur without waiting for the debounce', () => {
|
||||
const commit = vi.fn()
|
||||
const { result } = renderHook(() => useDebouncedSettingsTextDraft({ value: '', commit }))
|
||||
|
||||
act(() => result.current.onChange('abc'))
|
||||
act(() => result.current.onBlur())
|
||||
|
||||
expect(commit).toHaveBeenCalledExactlyOnceWith('abc')
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
|
||||
// The pending timer must not fire a second, duplicate commit.
|
||||
expect(commit).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('commits a pending edit when the field unmounts', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, unmount } = renderHook(() =>
|
||||
useDebouncedSettingsTextDraft({ value: '', commit })
|
||||
)
|
||||
|
||||
act(() => result.current.onChange('half-typed'))
|
||||
unmount()
|
||||
|
||||
expect(commit).toHaveBeenCalledExactlyOnceWith('half-typed')
|
||||
})
|
||||
|
||||
it('adopts an external value while the field is untouched', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, rerender } = renderHook(
|
||||
({ value }) => useDebouncedSettingsTextDraft({ value, commit }),
|
||||
{ initialProps: { value: 'first' } }
|
||||
)
|
||||
|
||||
rerender({ value: 'from-another-window' })
|
||||
|
||||
expect(result.current.value).toBe('from-another-window')
|
||||
expect(commit).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not let an external value overwrite an in-progress edit', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, rerender } = renderHook(
|
||||
({ value }) => useDebouncedSettingsTextDraft({ value, commit }),
|
||||
{ initialProps: { value: 'first' } }
|
||||
)
|
||||
|
||||
act(() => result.current.onChange('typing'))
|
||||
rerender({ value: 'from-another-window' })
|
||||
|
||||
expect(result.current.value).toBe('typing')
|
||||
})
|
||||
|
||||
it('does not commit when nothing was edited', () => {
|
||||
const commit = vi.fn()
|
||||
const { result } = renderHook(() => useDebouncedSettingsTextDraft({ value: 'x', commit }))
|
||||
|
||||
act(() => result.current.onBlur())
|
||||
|
||||
expect(commit).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('useDebouncedSettingsTextDraft flush paths', () => {
|
||||
it('commits a pending edit on beforeunload, since a window close never unmounts the tree', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, unmount } = renderHook(() =>
|
||||
useDebouncedSettingsTextDraft({ value: '', commit })
|
||||
)
|
||||
|
||||
act(() => result.current.onChange('quit-mid-word'))
|
||||
act(() => {
|
||||
window.dispatchEvent(new Event('beforeunload', { cancelable: true }))
|
||||
})
|
||||
|
||||
expect(commit).toHaveBeenCalledExactlyOnceWith('quit-mid-word')
|
||||
|
||||
// The later unmount and timer must not commit the same value again.
|
||||
unmount()
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
expect(commit).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not commit on beforeunload when nothing is pending', () => {
|
||||
const commit = vi.fn()
|
||||
renderHook(() => useDebouncedSettingsTextDraft({ value: 'x', commit }))
|
||||
|
||||
act(() => {
|
||||
window.dispatchEvent(new Event('beforeunload', { cancelable: true }))
|
||||
})
|
||||
|
||||
expect(commit).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('stops listening for beforeunload after unmount', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, unmount } = renderHook(() =>
|
||||
useDebouncedSettingsTextDraft({ value: '', commit })
|
||||
)
|
||||
|
||||
act(() => result.current.onChange('abc'))
|
||||
unmount()
|
||||
act(() => {
|
||||
window.dispatchEvent(new Event('beforeunload', { cancelable: true }))
|
||||
})
|
||||
|
||||
expect(commit).toHaveBeenCalledExactlyOnceWith('abc')
|
||||
})
|
||||
|
||||
it('commits every edit burst, not only the first', () => {
|
||||
const commit = vi.fn()
|
||||
const { result } = renderHook(() => useDebouncedSettingsTextDraft({ value: '', commit }))
|
||||
|
||||
act(() => result.current.onChange('one'))
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
act(() => result.current.onChange('one two'))
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
|
||||
expect(commit).toHaveBeenNthCalledWith(1, 'one')
|
||||
expect(commit).toHaveBeenNthCalledWith(2, 'one two')
|
||||
})
|
||||
|
||||
it('adopts external values again once a pending edit has been committed', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, rerender } = renderHook(
|
||||
({ value }) => useDebouncedSettingsTextDraft({ value, commit }),
|
||||
{ initialProps: { value: '' } }
|
||||
)
|
||||
|
||||
act(() => result.current.onChange('typed'))
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
expect(commit).toHaveBeenCalledExactlyOnceWith('typed')
|
||||
|
||||
// The store echoes the commit, then another window writes a different value.
|
||||
rerender({ value: 'typed' })
|
||||
rerender({ value: 'from-another-window' })
|
||||
|
||||
expect(result.current.value).toBe('from-another-window')
|
||||
})
|
||||
|
||||
it('keeps a keystroke typed while the previous commit is still in flight', () => {
|
||||
const commit = vi.fn()
|
||||
const { result, rerender } = renderHook(
|
||||
({ value }) => useDebouncedSettingsTextDraft({ value, commit }),
|
||||
{ initialProps: { value: '' } }
|
||||
)
|
||||
|
||||
act(() => result.current.onChange('abc'))
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(700)
|
||||
})
|
||||
act(() => result.current.onChange('abcd'))
|
||||
// The store echoes the first commit after the user has already typed more.
|
||||
rerender({ value: 'abc' })
|
||||
|
||||
expect(result.current.value).toBe('abcd')
|
||||
|
||||
act(() => result.current.onBlur())
|
||||
expect(commit).toHaveBeenLastCalledWith('abcd')
|
||||
expect(commit).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not spuriously commit under StrictMode effect replay', () => {
|
||||
const commit = vi.fn()
|
||||
const { result } = renderHook(() => useDebouncedSettingsTextDraft({ value: 'x', commit }), {
|
||||
wrapper: StrictMode
|
||||
})
|
||||
|
||||
expect(commit).not.toHaveBeenCalled()
|
||||
|
||||
act(() => result.current.onChange('xy'))
|
||||
act(() => result.current.onBlur())
|
||||
|
||||
expect(commit).toHaveBeenCalledExactlyOnceWith('xy')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,86 @@
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
|
||||
// Matches the repository-hook script draft, the established debounce for settings text in this pane.
|
||||
const SETTINGS_TEXT_COMMIT_DEBOUNCE_MS = 700
|
||||
|
||||
export type DebouncedSettingsTextDraft = {
|
||||
value: string
|
||||
onChange: (next: string) => void
|
||||
onBlur: () => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Local draft for a free-text setting, committed on a debounce and flushed on blur, unmount, and
|
||||
* window unload.
|
||||
*
|
||||
* Why: binding an `<Input>` straight to `updateSettings` sends one IPC round trip per keystroke,
|
||||
* and each one replaces the `settings` object identity in every other window, re-rendering every
|
||||
* component subscribed to it. The committed value is unchanged — only the number of commits is.
|
||||
*
|
||||
* A pending timer is the single source of truth for "the draft has uncommitted edits": `onChange`
|
||||
* is the only place that arms it and `flush` the only place that clears it, so there is no separate
|
||||
* dirty flag to fall out of sync.
|
||||
*/
|
||||
export function useDebouncedSettingsTextDraft(args: {
|
||||
value: string
|
||||
commit: (next: string) => void
|
||||
}): DebouncedSettingsTextDraft {
|
||||
const { value, commit } = args
|
||||
const [draft, setDraft] = useState(value)
|
||||
const draftRef = useRef(draft)
|
||||
const commitRef = useRef(commit)
|
||||
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
|
||||
|
||||
// Why an effect, not a render-time write: render must stay pure, and React can replay it.
|
||||
useEffect(() => {
|
||||
commitRef.current = commit
|
||||
}, [commit])
|
||||
|
||||
// Why gated on a pending commit: an external write (another window, a reset) should land in the
|
||||
// field, but must not yank characters out from under someone mid-edit.
|
||||
useEffect(() => {
|
||||
if (timerRef.current !== null) {
|
||||
return
|
||||
}
|
||||
draftRef.current = value
|
||||
setDraft(value)
|
||||
}, [value])
|
||||
|
||||
const flush = useCallback(() => {
|
||||
if (timerRef.current === null) {
|
||||
return
|
||||
}
|
||||
clearTimeout(timerRef.current)
|
||||
timerRef.current = null
|
||||
commitRef.current(draftRef.current)
|
||||
}, [])
|
||||
|
||||
const onChange = useCallback(
|
||||
(next: string) => {
|
||||
draftRef.current = next
|
||||
setDraft(next)
|
||||
if (timerRef.current !== null) {
|
||||
clearTimeout(timerRef.current)
|
||||
}
|
||||
timerRef.current = setTimeout(flush, SETTINGS_TEXT_COMMIT_DEBOUNCE_MS)
|
||||
},
|
||||
[flush]
|
||||
)
|
||||
|
||||
// Why unmount: closing the pane (or the settings search hiding the section) mid-word must persist
|
||||
// the same value typing it would have. `flush` has no dependencies, so this cleanup only ever runs
|
||||
// on unmount.
|
||||
// Why beforeunload: a window close or app quit never unmounts the tree, so the cleanup cannot run.
|
||||
// The close coordinator dispatches a synthetic beforeunload while the tree is still mounted so
|
||||
// listeners like this one can flush; `updateSettings` issues its IPC synchronously, ahead of the
|
||||
// close confirmation, so main persists the value before it flushes the store on quit.
|
||||
useEffect(() => {
|
||||
window.addEventListener('beforeunload', flush)
|
||||
return () => {
|
||||
window.removeEventListener('beforeunload', flush)
|
||||
flush()
|
||||
}
|
||||
}, [flush])
|
||||
|
||||
return { value: draft, onChange, onBlur: flush }
|
||||
}
|
||||
Reference in New Issue
Block a user