Remove completed task TASK-ABORT-03
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,42 +1,3 @@
|
||||
### TASK-ABORT-03 — Stale `abortRef` Read in `.finally()` in Three Hooks
|
||||
|
||||
**Where found**
|
||||
`frontend/src/hooks/useGlobalConfig.ts`, `frontend/src/hooks/useServerSettings.ts`, and `frontend/src/hooks/useJailConfigDetail.ts`. In each hook the `.finally()` block reads:
|
||||
```ts
|
||||
.finally(() => {
|
||||
if (!abortRef.current?.signal.aborted) {
|
||||
setLoading(false);
|
||||
}
|
||||
});
|
||||
```
|
||||
`abortRef.current` is a mutable ref. If `load()` is called a second time before the first fetch completes, `abortRef.current` is replaced with a new controller. The first fetch's `.finally()` then reads the *new* controller's signal (which is not aborted) and calls `setLoading(false)` while the second fetch is still in flight.
|
||||
|
||||
**Goal**
|
||||
Capture the local controller reference before the async operation and use that in `.finally()`:
|
||||
```ts
|
||||
const ctrl = new AbortController();
|
||||
abortRef.current = ctrl;
|
||||
// ...
|
||||
.finally(() => {
|
||||
if (!ctrl.signal.aborted) { // use the captured local variable
|
||||
setLoading(false);
|
||||
}
|
||||
});
|
||||
```
|
||||
This is the pattern already used correctly in `useListData`, `useBanTrend`, and most other hooks.
|
||||
|
||||
**Possible traps and issues**
|
||||
- This is a low-frequency race condition (only observable if `load()` is called in rapid succession), but it produces a spinner that disappears prematurely while a fetch is still in flight.
|
||||
- After this fix verify that the existing checks in `.then()` and `.catch()` also use the locally-captured `ctrl` (not `abortRef.current`) — they typically already do, but confirm.
|
||||
|
||||
**Docs changes needed**
|
||||
Add a code convention note in `Docs/Web-Development.md`: "Always capture the `AbortController` in a local `const` and use that local variable in all callbacks — never read `abortRef.current` from inside an async callback."
|
||||
|
||||
**Why this is needed**
|
||||
The loading spinner is the primary feedback mechanism for data fetches. Prematurely clearing it while a request is still in flight misleads the user and can cause partial renders.
|
||||
|
||||
---
|
||||
|
||||
### TASK-ABORT-04 — `useIpLookup` Has No AbortController or Unmount Guard
|
||||
|
||||
**Where found**
|
||||
|
||||
Reference in New Issue
Block a user