panel: stop wiping the URL input on every keystroke
Typing was impossible — each character vanished, and pasting flashed and
reverted to the saved value.
`useClient()` calls `createClient()` on every render with no memoisation, so
`get`/`put` are new function identities each time. The panel wrapped its
fetches in `useCallback(..., [get])`, which therefore also changed identity
every render, and the effects depended on THOSE:
keystroke -> setUrl -> re-render -> new `get` -> new callback -> effect fires
-> GET /config -> setUrl(saved value) -> character gone
Fixed by making them plain functions and loading once on mount, with the health
probe fired from the load rather than from a second effect watching
`configured` — an effect is one more thing that can re-fire.
The rule, written into the file since nothing about the symptom pointed at a
dependency array: never put a value derived from `useClient()` in a dependency
array. The rest of the codebase already follows it (CommandHqDetail keys its
fetch effect on `sessionId` alone).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+42
-24
@@ -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<HealthResult | null>(null);
|
||||
const [probing, setProbing] = useState(false);
|
||||
|
||||
const loadConfig = useCallback(async () => {
|
||||
try {
|
||||
const next = await get<VaultConfig>('/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<HealthResult>('/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<VaultConfig>('/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);
|
||||
|
||||
Reference in New Issue
Block a user