mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
ConnorYoh
2cf355c5cd
# Description of Changes
## The problem
`useAdminSettings` backs all 18 admin config sections. Each section
fetched its own copy of its settings block, held it in hand-rolled
loading/saving state, and refetched manually after every save.
Three consequences:
- **Duplicate fetching.** Four AI tabs all read the `aiEngine` block.
Nothing was shared, so each open refetched it.
- **Duplicated wiring.** All 18 sections carried the same effect to
trigger the fetch, each one depending on a `fetchSettings` callback that
would have refetched on every render had it ever become unstable.
- **Console noise.** The hook made 11 `console.*` calls, four of them
`JSON.stringify(settings, null, 2)` on **every fetch and every save** —
admin configuration serialised into the console of every admin session.
Every save also ended with a hand-written `await fetchSettings()`.
Forget it in a new section and its pending badges silently go stale.
## The fix
The hook uses TanStack Query, keyed on `sectionName`, so sections
reading the same block share one fetch and one cache entry.
The fetch gate moved into the hook. Sections used to write:
```ts
const { settings, fetchSettings } = useAdminSettings({ sectionName: "legal" });
useEffect(() => {
if (loginEnabled) fetchSettings();
}, [loginEnabled, fetchSettings]);
```
and now write:
```ts
const { settings } = useAdminSettings({
sectionName: "legal",
enabled: loginEnabled,
});
```
Saving is a mutation that invalidates the section on success, so the
refetch is structural rather than something each section remembers.
The delta computation and the save transformer are unchanged — that is
domain logic, not fetching. `settings` is still an editable draft seeded
from the server response, so forms behave exactly as before.
## Why it is better
Measured against the previous implementation across identical scenarios.
`commits` counts committed renders.
| Scenario | Before | After |
|---|---|---|
| Open one section | 2 commits, 1 request | 2 commits, 1 request |
| Browse the four AI tabs | 8 commits, 4 requests | **5 commits, 1
request** |
| Edit and save | 4 commits, 2 requests | 4 commits, 2 requests |
Committed renders are equal or better everywhere; browsing the AI tabs
costs a quarter of the requests.
The diff reads +449 / −303, but that includes a test file for a hook
that had no tests:
| | Added | Removed | Net |
|---|---|---|---|
| Production code (21 files) | 154 | 303 | **−149** |
| Tests (1 file) | 295 | 0 | +295 |
The 18 section files account for −133 of that: each drops an effect, a
destructure and usually an import, and gains one `enabled:` line. The
hook itself goes from 234 to 180 lines. `console.*` calls go from 11 to
0.
## Caching
Settings inherit the client's 30s stale window rather than refetching on
every mount, which is where the request saving comes from.
Nothing inside a cached block is server-observed — the only live reads
in these sections, `/api/v1/ai/health` and the tessdata language list,
are separate calls outside this query. A block therefore only changes
when another admin writes it.
Two things bound the staleness:
- Sections already held a single snapshot for as long as the modal
stayed open, with no refetch on focus. 30s is shorter than that window,
not longer.
- `computeDelta` only emits fields whose draft differs from the baseline
it was seeded from, so a stale baseline cannot produce a collateral
write. The only race is two admins editing the same field, which is
unchanged. Saving invalidates, so acting refreshes to current values.
The blocks where a stale read would matter most — `security`, `premium`,
`database` — are set once at deployment and effectively never edited
concurrently. The block with the most cache reuse, `aiEngine`, is the
least consequential.
**Convention:** config blocks cache; observed state does not. A section
that displays live server state inside its settings block should
override `staleTime` locally.
## Testing
14 tests, covering the shared fetch, cache reuse across tab reopens, key
separation between blocks, the `enabled` gate, delta-only saves, the
empty-delta short circuit, post-save invalidation, pending-value
display, and draft reseeding.
Each was checked by breaking the implementation and confirming the suite
fails: per-consumer query keys, sending the whole draft instead of the
delta, dropping the post-save invalidate, reporting loaded while
disabled, skipping the empty-delta short circuit, and reverting the
stale window to zero.
`task frontend:check` green. Two unrelated tests fail on this branch —
`workbenchSession.test.ts` and `notificationActions.test.tsx` — and fail
identically on `main`.
## Follow-ups
The sections that fetch through services rather than this hook — Teams,
TeamDetails, People, roughly 2,600 lines — are unchanged. Between them
they share two reads (`getTeams` and `getUsers`, both used by all three)
and carry ten distinct write operations, with no test coverage today.
---
## Primer: mutations
`useQuery` is for reads. It caches, dedupes, and re-renders when data
arrives. `useMutation` is for writes, where none of that applies — a
write happens once, when the user asks.
```ts
const save = useMutation({
mutationFn: (body) => putAdminSection("legal", body),
onSuccess: () => queryClient.invalidateQueries({ queryKey }),
});
save.mutate(body); // fire and forget
await save.mutateAsync(body); // or await it
save.isPending; // disable the button
save.error; // show the failure
```
`isPending` and `error` replace the `useState` flag and
`try/catch/finally` you would otherwise write around every save.
After a write the cache holds stale data. Two ways to fix it:
| | What it does | Use when |
|---|---|---|
| `invalidateQueries` | Marks the data stale so it refetches | The
server may transform, queue or reject part of what you sent |
| `setQueryData` | Writes your value into the cache, no request | The
response tells you exactly what the server now holds |
**Invalidate by default. Use `setQueryData` only when the response is
authoritative.**
This hook has to invalidate: the server can queue a settings change
rather than applying it, returning it in a `_pending` block that the
form renders as a badge. Writing the local draft into the cache would
show a queued change as applied.
Most mutations are not like that. A "rename a team" write, where the
response is the new team, is a `setQueryData` case.
One gotcha: `mutate` does not throw, `mutateAsync` does. An awaited
`mutateAsync` without a `try/catch` is an unhandled rejection.