refactor(frontend): extract shared fetch lifecycle into useFetchData base hook
Eliminates ~100 lines of duplicated code across useListData and usePolledData by creating a composable base hook that handles: - Abort controller lifecycle and cancellation - Loading/error state management - Fetch error handling - Unmount cleanup Changes: - Create hooks/useFetchData.ts with base fetch lifecycle (no effects on consumers) - Refactor useListData to compose useFetchData, returns items array by default - Refactor usePolledData to compose useFetchData, adds polling and focus-refetch - Add comprehensive tests for useFetchData base hook - Document hook architecture and composition pattern in Web-Development.md Result: Both hooks now use shared primitives, reducing maintenance burden and ensuring consistent cancellation/error handling across all data fetches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,43 +1,4 @@
|
||||
## 17) Weak typed error contracts in generic hooks
|
||||
- Where found:
|
||||
- [frontend/src/hooks/useListData.ts](frontend/src/hooks/useListData.ts)
|
||||
- [frontend/src/hooks/usePolledData.ts](frontend/src/hooks/usePolledData.ts)
|
||||
- Why this is needed:
|
||||
- Unknown-only error handling weakens actionable UX and diagnostics.
|
||||
- Goal:
|
||||
- Standardize API error typing and hook-level error model.
|
||||
- What to do:
|
||||
- Introduce discriminated error payloads.
|
||||
- Map ApiError and network errors consistently.
|
||||
- Possible traps and issues:
|
||||
- Type expansion can touch many call sites.
|
||||
- Docs changes needed:
|
||||
- Add typed error model examples.
|
||||
- Doc references:
|
||||
- [frontend/src/api/client.ts](frontend/src/api/client.ts)
|
||||
|
||||
---
|
||||
|
||||
## 18) Duplicate polling/list loading behavior across hooks
|
||||
- Where found:
|
||||
- [frontend/src/hooks/useListData.ts](frontend/src/hooks/useListData.ts)
|
||||
- [frontend/src/hooks/usePolledData.ts](frontend/src/hooks/usePolledData.ts)
|
||||
- Why this is needed:
|
||||
- Duplication multiplies maintenance bugs.
|
||||
- Goal:
|
||||
- Share a composable base for fetch lifecycle, cancellation, and polling.
|
||||
- What to do:
|
||||
- Extract shared primitives and keep hook-specific selectors minimal.
|
||||
- Possible traps and issues:
|
||||
- Generic abstractions can become too complex.
|
||||
- Docs changes needed:
|
||||
- Add hook architecture overview.
|
||||
- Doc references:
|
||||
- [Docs/Web-Development.md](Docs/Web-Development.md)
|
||||
|
||||
---
|
||||
|
||||
## 19) Provider dependency chain is implicit
|
||||
## 18) Provider dependency chain is implicit
|
||||
- Where found:
|
||||
- [frontend/src/App.tsx](frontend/src/App.tsx)
|
||||
- Why this is needed:
|
||||
@@ -56,7 +17,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 20) Loading UX lacks progressive/skeleton states
|
||||
## 19) Loading UX lacks progressive/skeleton states
|
||||
- Where found:
|
||||
- [frontend/src/pages](frontend/src/pages)
|
||||
- Why this is needed:
|
||||
@@ -74,7 +35,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 21) Silent auth error swallow in fetch error utility
|
||||
## 20) Silent auth error swallow in fetch error utility
|
||||
- Where found:
|
||||
- [frontend/src/utils/fetchError.ts](frontend/src/utils/fetchError.ts)
|
||||
- Why this is needed:
|
||||
@@ -92,7 +53,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 22) Magic strings are scattered in frontend storage keys
|
||||
## 21) Magic strings are scattered in frontend storage keys
|
||||
- Where found:
|
||||
- [frontend/src/providers/AuthProvider.tsx](frontend/src/providers/AuthProvider.tsx)
|
||||
- [frontend/src/layouts/MainLayout.tsx](frontend/src/layouts/MainLayout.tsx)
|
||||
@@ -112,7 +73,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 23) No global cancellation policy on route transitions
|
||||
## 22) No global cancellation policy on route transitions
|
||||
- Where found:
|
||||
- [frontend/src/hooks](frontend/src/hooks)
|
||||
- Why this is needed:
|
||||
@@ -130,7 +91,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 24) API response wrapper shape is inconsistent
|
||||
## 23) API response wrapper shape is inconsistent
|
||||
- Where found:
|
||||
- [backend/app/routers/dashboard.py](backend/app/routers/dashboard.py)
|
||||
- [backend/app/routers/jails.py](backend/app/routers/jails.py)
|
||||
@@ -151,7 +112,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 25) No canonical snake_case/camelCase serialization policy
|
||||
## 24) No canonical snake_case/camelCase serialization policy
|
||||
- Where found:
|
||||
- [backend/app/models/server.py](backend/app/models/server.py)
|
||||
- [frontend/src/types/server.ts](frontend/src/types/server.ts)
|
||||
@@ -171,7 +132,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 26) Pagination contract is not standardized across endpoints
|
||||
## 25) Pagination contract is not standardized across endpoints
|
||||
- Where found:
|
||||
- [backend/app/routers/dashboard.py](backend/app/routers/dashboard.py)
|
||||
- [backend/app/routers/history.py](backend/app/routers/history.py)
|
||||
@@ -191,7 +152,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 27) Error response body shape is inconsistent
|
||||
## 26) Error response body shape is inconsistent
|
||||
- Where found:
|
||||
- [backend/app/main.py](backend/app/main.py)
|
||||
- [backend/app/routers](backend/app/routers)
|
||||
@@ -211,7 +172,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 28) Login failure delay can enable app-layer DoS
|
||||
## 27) Login failure delay can enable app-layer DoS
|
||||
- Where found:
|
||||
- [backend/app/routers/auth.py](backend/app/routers/auth.py#L110)
|
||||
- Why this is needed:
|
||||
@@ -229,7 +190,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 29) Blocklist URL validation has DNS-rebinding window
|
||||
## 28) Blocklist URL validation has DNS-rebinding window
|
||||
- Where found:
|
||||
- [backend/app/utils/ip_utils.py](backend/app/utils/ip_utils.py#L145)
|
||||
- [backend/app/services/blocklist_service.py](backend/app/services/blocklist_service.py#L81)
|
||||
@@ -249,7 +210,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 30) Setup persistence is non-atomic across DB contexts
|
||||
## 29) Setup persistence is non-atomic across DB contexts
|
||||
- Where found:
|
||||
- [backend/app/services/setup_service.py](backend/app/services/setup_service.py)
|
||||
- [backend/app/repositories/settings_repo.py](backend/app/repositories/settings_repo.py)
|
||||
@@ -268,7 +229,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 31) Fire-and-forget reschedule may fail silently
|
||||
## 30) Fire-and-forget reschedule may fail silently
|
||||
- Where found:
|
||||
- [backend/app/tasks/blocklist_import.py](backend/app/tasks/blocklist_import.py#L108)
|
||||
- Why this is needed:
|
||||
@@ -286,7 +247,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 32) RateLimiter cleanup function is not scheduled/used
|
||||
## 31) RateLimiter cleanup function is not scheduled/used
|
||||
- Where found:
|
||||
- [backend/app/utils/rate_limiter.py](backend/app/utils/rate_limiter.py#L84)
|
||||
- [backend/app/startup.py](backend/app/startup.py)
|
||||
@@ -305,7 +266,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 33) Trusted proxy configuration is hardcoded in auth router
|
||||
## 32) Trusted proxy configuration is hardcoded in auth router
|
||||
- Where found:
|
||||
- [backend/app/routers/auth.py](backend/app/routers/auth.py#L46)
|
||||
- [backend/app/utils/client_ip.py](backend/app/utils/client_ip.py)
|
||||
@@ -325,7 +286,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 34) Setup redirect allowlist uses broad prefix matching
|
||||
## 33) Setup redirect allowlist uses broad prefix matching
|
||||
- Where found:
|
||||
- [backend/app/main.py](backend/app/main.py#L434)
|
||||
- Why this is needed:
|
||||
@@ -343,7 +304,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 35) API client sends JSON and CSRF header for every request method
|
||||
## 34) API client sends JSON and CSRF header for every request method
|
||||
- Where found:
|
||||
- [frontend/src/api/client.ts](frontend/src/api/client.ts)
|
||||
- Why this is needed:
|
||||
@@ -362,7 +323,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 36) Polling continues when tab is not visible
|
||||
## 35) Polling continues when tab is not visible
|
||||
- Where found:
|
||||
- [frontend/src/hooks/usePolledData.ts](frontend/src/hooks/usePolledData.ts#L90)
|
||||
- [frontend/src/hooks/useBlocklistStatus.ts](frontend/src/hooks/useBlocklistStatus.ts)
|
||||
@@ -381,7 +342,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 37) Multi-worker safety check depends on one environment variable
|
||||
## 36) Multi-worker safety check depends on one environment variable
|
||||
- Where found:
|
||||
- [backend/app/startup.py](backend/app/startup.py#L61)
|
||||
- Why this is needed:
|
||||
@@ -399,7 +360,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 38) History archive query paths may need explicit indexing plan
|
||||
## 37) History archive query paths may need explicit indexing plan
|
||||
- Where found:
|
||||
- [backend/app/db.py](backend/app/db.py)
|
||||
- [backend/app/repositories/history_archive_repo.py](backend/app/repositories/history_archive_repo.py)
|
||||
@@ -420,7 +381,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 39) No explicit DI container strategy for backend service graph
|
||||
## 38) No explicit DI container strategy for backend service graph
|
||||
- Where found:
|
||||
- [backend/app/dependencies.py](backend/app/dependencies.py)
|
||||
- [backend/app/services](backend/app/services)
|
||||
@@ -439,7 +400,7 @@
|
||||
|
||||
---
|
||||
|
||||
## 40) Frontend and backend observability are not aligned
|
||||
## 39) Frontend and backend observability are not aligned
|
||||
- Where found:
|
||||
- [backend/app/main.py](backend/app/main.py)
|
||||
- [frontend/src](frontend/src)
|
||||
|
||||
@@ -246,6 +246,51 @@ const doBan = useCallback(
|
||||
- ❌ Passing API functions directly to components — couples components to API contract
|
||||
- ❌ Multiple domain hooks for the same data without deduplication — causes wasted requests and state desync
|
||||
|
||||
### Hook Architecture & Reusable Primitives
|
||||
|
||||
BanGUI's hooks are built on composable primitives to eliminate duplication and enforce consistent patterns.
|
||||
|
||||
**Base Hook: `useFetchData`** (`hooks/useFetchData.ts`)
|
||||
- The foundation of all data-fetching hooks
|
||||
- Encapsulates fetch lifecycle: abort controller management, loading/error state, cancellation safety
|
||||
- Signature: `useFetchData(fetcher, selector, errorMessage, onSuccess?, initialData?) → { data, loading, error, refresh }`
|
||||
- Not used directly by consumers; only composed by higher-level hooks
|
||||
- Handles automatic cleanup on unmount (abort signal cancellation)
|
||||
|
||||
**Tier 2 Hooks Built on `useFetchData`:**
|
||||
- `useListData`: Wraps `useFetchData` with `initialData` defaulting to `[]` and returns `{ items, ... }`
|
||||
- `usePolledData`: Wraps `useFetchData` and adds polling (interval) + window-focus refetch on top
|
||||
- Additional specialized hooks can be added by composing `useFetchData` with domain-specific effects
|
||||
|
||||
**Composition Pattern for New Hooks:**
|
||||
When building a new Tier 2 hook with custom behavior, follow this pattern:
|
||||
```ts
|
||||
export function useMyCustomData<TResponse, TData>(options: MyOptions): MyResult {
|
||||
// 1. Use useFetchData for the base fetch lifecycle
|
||||
const { data, loading, error, refresh } = useFetchData({
|
||||
fetcher: options.fetcher,
|
||||
selector: options.selector,
|
||||
errorMessage: options.errorMessage,
|
||||
onSuccess: options.onSuccess,
|
||||
initialData: options.initialData,
|
||||
});
|
||||
|
||||
// 2. Add custom effects for additional behavior (e.g., polling, focus handling, custom cleanup)
|
||||
useEffect(() => {
|
||||
// Your custom logic here
|
||||
}, [...dependencies]);
|
||||
|
||||
// 3. Return a domain-specific result shape
|
||||
return { data, loading, error, refresh, customField: /* derived */ };
|
||||
}
|
||||
```
|
||||
|
||||
**Why this architecture?**
|
||||
- **DRY**: Eliminates duplicate fetch logic across multiple hooks
|
||||
- **Consistency**: All hooks share the same cancellation and error handling semantics
|
||||
- **Testability**: Base hook can be tested in isolation; custom effects are minimal and easy to test
|
||||
- **Maintainability**: Bug fixes to abort or error handling only need to happen once
|
||||
|
||||
---
|
||||
|
||||
## 4. Code Organization
|
||||
|
||||
Reference in New Issue
Block a user