From 3cc85a3daeeeb9d65d5165170e55da7733ac6e1f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 31 May 2026 11:13:14 -0700 Subject: [PATCH] perf: reset linear api key dialog state during render (#4290) --- .../linear-api-key-dialog-state.test.ts | 34 +++++++++++ .../components/linear-api-key-dialog-state.ts | 30 ++++++++++ .../src/components/linear-api-key-dialog.tsx | 57 ++++++++++--------- 3 files changed, 95 insertions(+), 26 deletions(-) create mode 100644 src/renderer/src/components/linear-api-key-dialog-state.test.ts create mode 100644 src/renderer/src/components/linear-api-key-dialog-state.ts diff --git a/src/renderer/src/components/linear-api-key-dialog-state.test.ts b/src/renderer/src/components/linear-api-key-dialog-state.test.ts new file mode 100644 index 00000000000..7339fdba5b6 --- /dev/null +++ b/src/renderer/src/components/linear-api-key-dialog-state.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest' +import { + CLOSED_LINEAR_API_KEY_DIALOG_STATE, + resolveLinearApiKeyDialogState, + type LinearApiKeyDialogState +} from './linear-api-key-dialog-state' + +describe('resolveLinearApiKeyDialogState', () => { + it('preserves draft and error state while the dialog is open', () => { + const state: LinearApiKeyDialogState = { + apiKeyDraft: 'lin_api_key', + connectState: 'error', + connectError: 'Nope' + } + + expect(resolveLinearApiKeyDialogState(state, true)).toBe(state) + }) + + it('preserves identity for an already-reset closed dialog', () => { + expect(resolveLinearApiKeyDialogState(CLOSED_LINEAR_API_KEY_DIALOG_STATE, false)).toBe( + CLOSED_LINEAR_API_KEY_DIALOG_STATE + ) + }) + + it('resets draft and connection state when the dialog is closed', () => { + const state: LinearApiKeyDialogState = { + apiKeyDraft: 'lin_api_key', + connectState: 'connecting', + connectError: 'Previous error' + } + + expect(resolveLinearApiKeyDialogState(state, false)).toEqual(CLOSED_LINEAR_API_KEY_DIALOG_STATE) + }) +}) diff --git a/src/renderer/src/components/linear-api-key-dialog-state.ts b/src/renderer/src/components/linear-api-key-dialog-state.ts new file mode 100644 index 00000000000..979c68ae232 --- /dev/null +++ b/src/renderer/src/components/linear-api-key-dialog-state.ts @@ -0,0 +1,30 @@ +export type LinearApiKeyDialogConnectState = 'idle' | 'connecting' | 'error' + +export type LinearApiKeyDialogState = { + apiKeyDraft: string + connectState: LinearApiKeyDialogConnectState + connectError: string | null +} + +export const CLOSED_LINEAR_API_KEY_DIALOG_STATE: LinearApiKeyDialogState = Object.freeze({ + apiKeyDraft: '', + connectState: 'idle', + connectError: null +}) + +export function createLinearApiKeyDialogState(): LinearApiKeyDialogState { + return CLOSED_LINEAR_API_KEY_DIALOG_STATE +} + +export function resolveLinearApiKeyDialogState( + state: LinearApiKeyDialogState, + open: boolean +): LinearApiKeyDialogState { + if (open) { + return state + } + if (state.apiKeyDraft === '' && state.connectState === 'idle' && state.connectError === null) { + return state + } + return CLOSED_LINEAR_API_KEY_DIALOG_STATE +} diff --git a/src/renderer/src/components/linear-api-key-dialog.tsx b/src/renderer/src/components/linear-api-key-dialog.tsx index f53cb20e037..38e18a977f1 100644 --- a/src/renderer/src/components/linear-api-key-dialog.tsx +++ b/src/renderer/src/components/linear-api-key-dialog.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId, useMemo, useState } from 'react' +import { useId, useMemo, useState } from 'react' import { ExternalLink, LoaderCircle, Lock } from 'lucide-react' import type { LinearWorkspace } from '../../../shared/types' import { @@ -20,6 +20,10 @@ import { import { Input } from '@/components/ui/input' import { Label } from '@/components/ui/label' import { cn } from '@/lib/utils' +import { + createLinearApiKeyDialogState, + resolveLinearApiKeyDialogState +} from './linear-api-key-dialog-state' type LinearApiKeyDialogProps = { open: boolean @@ -49,23 +53,19 @@ export function LinearApiKeyDialog({ const mountedRef = useMountedRef() const apiKeyInputId = useId() const apiKeyErrorId = useId() - const [apiKeyDraft, setApiKeyDraft] = useState('') - const [connectState, setConnectState] = useState<'idle' | 'connecting' | 'error'>('idle') - const [connectError, setConnectError] = useState(null) + const [dialogState, setDialogState] = useState(createLinearApiKeyDialogState) const runtimeTarget = useMemo(() => getActiveRuntimeTarget(settings), [settings]) const personalKeyUrl = buildLinearPersonalApiKeySettingsUrl(workspace?.organizationUrlKey) const workspaceApiUrl = buildLinearWorkspaceApiSettingsUrl(workspace?.organizationUrlKey) const submitLabel = connectLabel ?? (workspace ? 'Update access' : 'Connect') - - useEffect(() => { - if (open) { - return - } - setApiKeyDraft('') - setConnectState('idle') - setConnectError(null) - }, [open]) + const resolvedDialogState = resolveLinearApiKeyDialogState(dialogState, open) + if (resolvedDialogState !== dialogState) { + // Why: parent-controlled close can race an in-flight connect request; keep + // hidden draft/error state reset before the next open paints. + setDialogState(resolvedDialogState) + } + const { apiKeyDraft, connectState, connectError } = resolvedDialogState const handleOpenChange = (nextOpen: boolean): void => { if (connectState !== 'connecting') { @@ -78,26 +78,30 @@ export function LinearApiKeyDialog({ if (!apiKey || connectState === 'connecting') { return } - setConnectState('connecting') - setConnectError(null) + setDialogState((current) => ({ ...current, connectState: 'connecting', connectError: null })) try { const result = await connectLinear(apiKey) if (!mountedRef.current) { return } if (result.ok) { - setApiKeyDraft('') - setConnectState('idle') + setDialogState(createLinearApiKeyDialogState()) onOpenChange(false) onConnected?.() return } - setConnectState('error') - setConnectError(result.error) + setDialogState((current) => ({ + ...current, + connectState: 'error', + connectError: result.error + })) } catch (error) { if (mountedRef.current) { - setConnectState('error') - setConnectError(error instanceof Error ? error.message : 'Connection failed') + setDialogState((current) => ({ + ...current, + connectState: 'error', + connectError: error instanceof Error ? error.message : 'Connection failed' + })) } } } @@ -143,11 +147,12 @@ export function LinearApiKeyDialog({ placeholder="lin_api_..." value={apiKeyDraft} onChange={(event) => { - setApiKeyDraft(event.target.value) - if (connectState === 'error') { - setConnectState('idle') - setConnectError(null) - } + const nextDraft = event.target.value + setDialogState((current) => ({ + apiKeyDraft: nextDraft, + connectState: current.connectState === 'error' ? 'idle' : current.connectState, + connectError: current.connectState === 'error' ? null : current.connectError + })) }} disabled={connectState === 'connecting'} aria-invalid={connectState === 'error'}