Update documentation and refactor useAutoSave hook
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,27 +1,3 @@
|
|||||||
### TASK-QUALITY-03 — `useHistory` Object Identity Dependency Footgun
|
|
||||||
|
|
||||||
**Where found**
|
|
||||||
`frontend/src/hooks/useHistory.ts`. The hook accepts a `query` object as a parameter and lists it directly in the `useCallback` dependency array for the internal `load` function. If a caller passes an inline object literal on every render (e.g. `useHistory({ page: 1, jail: selectedJail })`), `query` is a new reference every render, causing a new `load` callback, which causes `useEffect([load])` to fire, triggering an infinite re-fetch.
|
|
||||||
|
|
||||||
**Goal**
|
|
||||||
Document this constraint prominently in the hook's JSDoc and in `Docs/Web-Development.md`. Alternatively, change the hook to accept individual primitive parameters instead of an object, eliminating the reference-stability requirement:
|
|
||||||
```ts
|
|
||||||
export function useHistory(page: number, pageSize: number, jail?: string, ...): UseHistoryResult
|
|
||||||
```
|
|
||||||
This is the safest fix because it makes incorrect usage a compile-time error.
|
|
||||||
|
|
||||||
**Possible traps and issues**
|
|
||||||
- Changing the signature is a breaking change for all callers. Audit all call sites before changing the signature.
|
|
||||||
- The interim documentation fix (a clear JSDoc warning) is a lower-risk option if refactoring callers is out of scope.
|
|
||||||
|
|
||||||
**Docs changes needed**
|
|
||||||
Add a note to `Docs/Web-Development.md`: "Hooks that accept objects as parameters must either destructure to primitives internally or require the caller to provide a stable reference (e.g. via `useMemo`)."
|
|
||||||
|
|
||||||
**Why this is needed**
|
|
||||||
This is a silent footgun. The hook works correctly in all current call sites only because callers happen to use `useMemo` or stable state references. A future caller passing an inline literal will introduce an infinite re-fetch with no obvious diagnostic.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### TASK-QUALITY-04 — `pendingSaveRef as boolean` Redundant Cast in `useAutoSave`
|
### TASK-QUALITY-04 — `pendingSaveRef as boolean` Redundant Cast in `useAutoSave`
|
||||||
|
|
||||||
**Where found**
|
**Where found**
|
||||||
|
|||||||
@@ -74,7 +74,7 @@ export function useAutoSave<T>(
|
|||||||
isSavingRef.current = false;
|
isSavingRef.current = false;
|
||||||
|
|
||||||
// If a pending save was queued while we were saving, run it now.
|
// If a pending save was queued while we were saving, run it now.
|
||||||
if (pendingSaveRef.current as boolean) {
|
if (pendingSaveRef.current) {
|
||||||
pendingSaveRef.current = false;
|
pendingSaveRef.current = false;
|
||||||
await performSave(latestValueRef.current);
|
await performSave(latestValueRef.current);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user