diff --git a/web/VaultPanel.tsx b/web/VaultPanel.tsx index dc40a53..f464eed 100644 --- a/web/VaultPanel.tsx +++ b/web/VaultPanel.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useEffect, useState } from 'react'; import { KeyRound, Loader2, RefreshCw } from 'lucide-react'; import { useClient } from 'hooks/useClient'; import { toneFill, toneText, type Tone } from '@/components/Data'; @@ -54,41 +54,59 @@ export const VaultPanel = () => { const [health, setHealth] = useState(null); const [probing, setProbing] = useState(false); - const loadConfig = useCallback(async () => { - try { - const next = await get('/vault/config'); - setConfig(next); - setUrl(next.url ?? ''); - setError(null); - } catch (err) { - setError(err instanceof Error ? err.message : 'could not read the vault configuration'); - } finally { - setLoading(false); - } - }, [get]); + /* + Plain functions, NOT `useCallback`, and the effect below depends on NEITHER of them. - // Never throws: an unreachable vault is the thing this panel exists to SHOW, not an error state for the - // panel itself. A rejected probe becomes a result with `ok: false`, same as a 502 body would. - const probe = useCallback(async () => { + `useClient()` calls `createClient()` on every render with no memoisation, so `get`/`put` are fresh + function identities each time. A `useCallback(..., [get])` therefore also changes identity every + render, and an effect depending on THAT re-runs every render. The result was a panel that wiped its + own input: + + keystroke -> setUrl -> re-render -> new `get` -> new callback -> effect fires + -> GET /config -> setUrl(saved value) -> the character just typed disappears + + Typing was impossible, pasting flashed and reverted. Nothing about it looked like a dependency-array + problem from the outside, which is what made it worth writing down rather than just fixing. + + The rule this settles for anything in this plugin: never put a value derived from `useClient()` in a + dependency array. The rest of the codebase already follows it (see `CommandHqDetail`, which keys its + fetch effect on `sessionId` alone). + */ + const probe = async () => { setProbing(true); try { + // Never throws: an unreachable vault is the thing this panel exists to SHOW, not an error state for + // the panel itself. A rejected probe becomes a result with `ok: false`, same as a 502 body would. setHealth(await get('/vault/config/health')); } catch (err) { setHealth({ ok: false, error: err instanceof Error ? err.message : 'probe failed' }); } finally { setProbing(false); } - }, [get]); + }; + const loadConfig = async () => { + try { + const next = await get('/vault/config'); + setConfig(next); + setUrl(next.url ?? ''); + setError(null); + // Probed here rather than from an effect watching `configured`, for the same reason: an effect is + // one more thing that can re-fire. Only when there is somewhere to probe — asking about an + // unconfigured vault returns a 503 that reads as a fault when the honest answer is "not set up yet". + if (next.configured) void probe(); + } catch (err) { + setError(err instanceof Error ? err.message : 'could not read the vault configuration'); + } finally { + setLoading(false); + } + }; + + // Mount only. Deliberately empty deps — see the note above. useEffect(() => { void loadConfig(); - }, [loadConfig]); - - // Only probe once there is somewhere to probe. Asking about an unconfigured vault returns a 503 that - // reads on screen as a fault, when the honest answer is "you have not set this up yet". - useEffect(() => { - if (config?.configured) void probe(); - }, [config?.configured, probe]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); const save = async () => { setSaving(true);