Skip to content

Commit 633f641

Browse files
fylornclaude
andauthored
Load the web console's server state through TanStack Query (#23)
* Add TanStack Query to the web console Pull in @tanstack/react-query 5.102 and mount a shared client in main.tsx. The client's defaults live in src/lib/query-client.ts with their reasons: no retries, since nearly every failure here is a 4xx a retry cannot fix, and no refetch on window focus, since admin pages mount several queries at once. api() now announces successful writes (onSuccessfulWrite), and main.tsx has the cache drop the queries no screen is using. One write can change what several cached screens hold; this way none of them reopens onto data from before it, without each call site knowing what it touched. @tanstack/eslint-plugin-query joins the lint config: its exhaustive-deps keeps every value a queryFn reads in its key. Tests get renderWithQueryClient, which renders inside a fresh cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Load server state through TanStack Query Every load that carried a react-hooks/set-state-in-effect suppression goes through useQuery now, and writes invalidate what their screen shows instead of calling a loader again. The loading flags, the abort controllers, the fetch-token race guard and the 13 loader useCallback wrappers they needed are gone; query keys follow the endpoint. Behaviour that changes on purpose: - Logout, and the cross-tab logout broadcast, clear the cache along with the user. The signed-in user is one shared query, so signing out from Profile now signs out the whole UI. - A reload after a write refreshes in place instead of flashing the skeleton, and a load error clears on the next successful load. - RouteEditorDialog maps remote-models entries to ids: the endpoint returns objects, and the dialog handed them to React as children. - OutboxBacklogDialog drops onChanged: invalidating the webhook-outbox prefix also refreshes the parent's backlog counts. - Polling uses refetchInterval, which pauses while the tab is hidden. Where cached data can already be there on the first render, local state follows it through trackers that start empty: BatchImportDialog can mount open, the Models page's ?import= deeplink opens it during render, and the pricing form seeds once, from a fetch made after mount. BatchImportDialog also offers nothing until both lists are fresh, so a cached copy cannot offer models imported since. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Derive the setup wizard's validation errors during render adminErrors was state kept in step with the form by an effect that ran validateAdmin after every change. It is a pure function of the form values, so it is a useMemo now: an error clears on the keystroke that fixes it, and the last set-state-in-effect suppression goes with the effect. It was labelled a hand-rolled load; it never fetched anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Document the query layer in the web README The "Data fetching" section described hand-rolled loads and the two reasons a suppression of set-state-in-effect could stand. Both are gone, so the section now describes the query layer — keys, writes, signals, deriving local state — and keeps the rule against new suppressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent dd4d6b0 commit 633f641

42 files changed

Lines changed: 1550 additions & 1421 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎web/README.md‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ The management console for ThinkWatch, built with React 19, TypeScript, and Vite
66

77
- **React 19** with TypeScript
88
- **TanStack Router** for file-based routing
9+
- **TanStack Query** for server state — see [Data fetching](#data-fetching)
910
- **shadcn/ui** (Radix UI + Tailwind CSS 4) for components
1011
- **react-i18next** for internationalization (English + Chinese)
1112
- **Vitest** + React Testing Library for testing
@@ -23,28 +24,36 @@ pnpm exec tsc --noEmit # Type check
2324

2425
## Data fetching
2526

26-
Loads are hand-rolled: a `useCallback` that fetches and sets state, plus a
27-
`useEffect` that calls it. There is no data-fetching layer in this app.
28-
29-
That shape trips `react-hooks/set-state-in-effect`, and the sites that it
30-
flags carry a one-line suppression saying which of two things is going on:
31-
32-
- **The rule is wrong.** `useEffect(() => { load(); }, [load])` where `load`
33-
is async and its first statement is the `await`. Every setState inside
34-
runs in the continuation — never synchronously with the effect, never a
35-
cascading render. The rule's cross-function analysis does not model
36-
`await`.
37-
- **The rule is right and the fix is architectural.** The loader's first
38-
statement flips a spinner. Hoisting that flag out to render-time silences
39-
the rule, but it splits "start a load" across two places and leaves every
40-
other caller of the loader responsible for remembering half of it.
41-
42-
**Both go away with a data-fetching layer** (TanStack Query or equivalent),
43-
which owns the loading flag and the cache and removes the effect entirely.
44-
That is a deliberate piece of work, not something to fold into a lint pass.
45-
Until then, do not add new suppressions of this rule without one of the two
46-
reasons above — every other finding it reports is a real one, and the rest
47-
of the codebase is clean of them.
27+
Server state goes through [TanStack Query](https://tanstack.com/query). A
28+
component asks for what it renders with `useQuery`, and the query layer owns
29+
the rest: the loading flag, the cache, cancelling a request nobody is waiting
30+
for, and keeping a late response from landing under filters that have since
31+
changed. No load runs in an effect.
32+
33+
- **Keys follow the endpoint.** `GET /api/admin/teams/{id}/members` is keyed
34+
`['admin', 'teams', id, 'members']`, with query-string parameters in a
35+
trailing object. A key names everything its `queryFn` reads —
36+
`@tanstack/query/exhaustive-deps` enforces that — and invalidating a
37+
prefix such as `['admin', 'teams']` reaches every query under it. Pass
38+
`exact: true` when a prefix would reach further than the write did.
39+
- **Writes invalidate.** After a mutation, `await
40+
queryClient.invalidateQueries({ queryKey })` for what the screen shows.
41+
Awaiting keeps orderings like "close the dialog once the list is fresh".
42+
Screens that aren't open need nothing: every successful write through
43+
`api()` drops the cached queries no screen is using (`onSuccessfulWrite`,
44+
wired up in `main.tsx`), so none of them reopens onto pre-write data.
45+
- **Pass the signal.** `queryFn: ({ signal }) => api(path, { signal })` lets
46+
the layer cancel a request once no component needs its answer.
47+
- **Derive, don't copy.** Read `query.data` where it is rendered. When a
48+
screen needs local state that follows what was loaded — a form draft, a
49+
selection — adjust it during render, the way `useResetOnChange` does.
50+
51+
`src/lib/query-client.ts` builds the client and says why its defaults differ
52+
from the library's. Tests render through `renderWithQueryClient` from
53+
`src/test/render.tsx`, which gives every call an empty cache.
54+
55+
Do not add new suppressions of `react-hooks/set-state-in-effect` — every
56+
finding it reports is a real one, and the codebase is clean of them.
4857

4958
## Project Structure
5059

@@ -58,6 +67,7 @@ src/
5867
│ └── use-mobile.ts # Responsive breakpoint detection
5968
├── lib/
6069
│ ├── api.ts # HTTP client with HMAC signing & auto token refresh
70+
│ ├── query-client.ts # TanStack Query client & its defaults
6171
│ └── utils.ts # Utility functions
6272
├── i18n/
6373
│ ├── en.json # English translations
@@ -88,6 +98,7 @@ src/
8898
│ ├── settings.tsx # Dynamic system settings (7 tabs)
8999
│ └── log-forwarders.tsx # Log forwarding configuration
90100
├── test/
101+
│ ├── render.tsx # render() inside a fresh query cache
91102
│ └── setup.ts # Test setup (jest-dom + i18n)
92103
└── router.tsx # Route definitions & setup redirect logic
93104
```

‎web/eslint.config.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import globals from 'globals'
33
import reactHooks from 'eslint-plugin-react-hooks'
44
import reactRefresh from 'eslint-plugin-react-refresh'
55
import reactCompiler from 'eslint-plugin-react-compiler'
6+
import pluginQuery from '@tanstack/eslint-plugin-query'
67
import tseslint from 'typescript-eslint'
78
import { defineConfig, globalIgnores } from 'eslint/config'
89

@@ -22,6 +23,10 @@ export default defineConfig([
2223
tseslint.configs.recommended,
2324
reactHooks.configs.flat.recommended,
2425
reactRefresh.configs.vite,
26+
// The query layer's counterpart to exhaustive effect deps: a value a
27+
// `queryFn` reads but its `queryKey` omits makes two different
28+
// requests share one cache entry.
29+
pluginQuery.configs['flat/recommended'],
2530
],
2631
rules: {
2732
// `warn` rather than `error` while we land the underlying

‎web/package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"@codemirror/lang-json": "^6.0.2",
2121
"@fontsource-variable/geist": "^5.2.8",
2222
"@tailwindcss/vite": "^4.2.2",
23+
"@tanstack/react-query": "^5.102.8",
2324
"@tanstack/react-router": "^1.168.10",
2425
"@uiw/react-codemirror": "^4.25.9",
2526
"class-variance-authority": "^0.7.1",
@@ -44,6 +45,7 @@
4445
"devDependencies": {
4546
"@eslint/js": "^10.0.1",
4647
"@playwright/test": "^1.59.1",
48+
"@tanstack/eslint-plugin-query": "^5.102.8",
4749
"@testing-library/jest-dom": "^6.9.1",
4850
"@testing-library/react": "^16.3.2",
4951
"@testing-library/user-event": "^14.6.1",

‎web/pnpm-lock.yaml‎

Lines changed: 39 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎web/src/components/limits/user-limits-tab.test.tsx‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest'
2-
import { render, screen, waitFor } from '@testing-library/react'
2+
import { screen, waitFor } from '@testing-library/react'
3+
import { renderWithQueryClient } from '@/test/render'
34
import { UserLimitsTab } from './user-limits-tab'
45

56
vi.mock('@/lib/api', () => ({
@@ -55,7 +56,7 @@ describe('UserLimitsTab — remaining / exceeded labels', () => {
5556
dashboard({ ruleMax: 1000, ruleCurrent: 250, capLimit: 1_000_000, capCurrent: 600_000 }),
5657
)
5758

58-
render(<UserLimitsTab userId="user-1" />)
59+
renderWithQueryClient(<UserLimitsTab userId="user-1" />)
5960

6061
await waitFor(() => {
6162
expect(screen.getByText('25%')).toBeInTheDocument()
@@ -72,7 +73,7 @@ describe('UserLimitsTab — remaining / exceeded labels', () => {
7273
dashboard({ ruleMax: 100, ruleCurrent: 100, capLimit: 500, capCurrent: 600 }),
7374
)
7475

75-
render(<UserLimitsTab userId="user-2" />)
76+
renderWithQueryClient(<UserLimitsTab userId="user-2" />)
7677

7778
await waitFor(() => {
7879
// Rule pct caps at 100 because of Math.min, cap pct also caps at 100
@@ -103,7 +104,7 @@ describe('UserLimitsTab — remaining / exceeded labels', () => {
103104
recent_events: [],
104105
})
105106

106-
render(<UserLimitsTab userId="user-3" />)
107+
renderWithQueryClient(<UserLimitsTab userId="user-3" />)
107108

108109
await waitFor(() => {
109110
expect(screen.getByText('0%')).toBeInTheDocument()

‎web/src/components/limits/user-limits-tab.tsx‎

Lines changed: 32 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@
3131
// POST /api/admin/limits/bulk/{rules|budgets}/{disable|delete} — bulk
3232
// ============================================================================
3333

34-
import { useCallback, useEffect, useMemo, useState } from 'react';
34+
import { useMemo, useState } from 'react';
3535
import { useTranslation } from 'react-i18next';
36+
import { useQuery, useQueryClient } from '@tanstack/react-query';
3637
import { useNow } from '@/hooks/use-now';
3738
import { AlertCircle, Plus, Trash2, PowerOff, RotateCw, Pencil } from 'lucide-react';
3839
import { Button } from '@/components/ui/button';
@@ -174,10 +175,18 @@ interface DrawerInit {
174175

175176
export function UserLimitsTab({ userId }: UserLimitsTabProps) {
176177
const { t } = useTranslation();
177-
const [data, setData] = useState<LimitsDashboard | null>(null);
178-
const [loading, setLoading] = useState(true);
179-
const [error, setError] = useState('');
180-
const [selected, setSelected] = useState<Set<string>>(new Set());
178+
const queryClient = useQueryClient();
179+
const dashboardQuery = useQuery({
180+
queryKey: ['admin', 'users', userId, 'limits-dashboard'],
181+
queryFn: ({ signal }) =>
182+
api<LimitsDashboard>(`/api/admin/users/${userId}/limits-dashboard`, { signal }),
183+
});
184+
const data = dashboardQuery.data ?? null;
185+
const loading = dashboardQuery.isPending;
186+
const error = dashboardQuery.error?.message ?? '';
187+
const reload = () =>
188+
queryClient.invalidateQueries({ queryKey: ['admin', 'users', userId, 'limits-dashboard'] });
189+
const [selection, setSelection] = useState<Set<string>>(new Set());
181190
const [bulkAction, setBulkAction] = useState<'disable' | 'delete' | null>(null);
182191
const [drawer, setDrawer] = useState<DrawerInit | null>(null);
183192
const [resetTarget, setResetTarget] = useState<
@@ -186,39 +195,6 @@ export function UserLimitsTab({ userId }: UserLimitsTabProps) {
186195
| null
187196
>(null);
188197

189-
const reload = useCallback(async () => {
190-
setError('');
191-
try {
192-
const res = await api<LimitsDashboard>(`/api/admin/users/${userId}/limits-dashboard`);
193-
setData(res);
194-
// Drop stale selections.
195-
setSelected((prev) => {
196-
const live = new Set<string>();
197-
res.rules.forEach((r) => {
198-
const k = rowKey(r);
199-
if (k) live.add(k);
200-
});
201-
res.caps.forEach((c) => {
202-
const k = rowKey(c);
203-
if (k) live.add(k);
204-
});
205-
return new Set(Array.from(prev).filter((k) => live.has(k)));
206-
});
207-
} catch (e) {
208-
setError(e instanceof Error ? e.message : t('common.operationFailed'));
209-
} finally {
210-
setLoading(false);
211-
}
212-
}, [userId, t]);
213-
214-
useEffect(() => {
215-
// Hand-rolled load: the spinner flag is the first half of "start a
216-
// fetch" and belongs with it. See "Data fetching" in web/README.md —
217-
// this goes away with a data-fetching layer, not by moving the flag.
218-
// eslint-disable-next-line react-hooks/set-state-in-effect
219-
reload();
220-
}, [reload]);
221-
222198
const overrideKeys = useMemo(() => {
223199
if (!data) return { rules: new Set<string>(), caps: new Set<string>() };
224200
return {
@@ -229,10 +205,26 @@ export function UserLimitsTab({ userId }: UserLimitsTabProps) {
229205
};
230206
}, [data]);
231207

208+
// Drop stale selections: an override the latest load no longer lists
209+
// stops counting as selected.
210+
const selected = useMemo(() => {
211+
if (!data) return selection;
212+
const live = new Set<string>();
213+
data.rules.forEach((r) => {
214+
const k = rowKey(r);
215+
if (k) live.add(k);
216+
});
217+
data.caps.forEach((c) => {
218+
const k = rowKey(c);
219+
if (k) live.add(k);
220+
});
221+
return new Set(Array.from(selection).filter((k) => live.has(k)));
222+
}, [data, selection]);
223+
232224
const someSelected = selected.size > 0;
233225

234226
const toggleOne = (key: string) =>
235-
setSelected((prev) => {
227+
setSelection((prev) => {
236228
const next = new Set(prev);
237229
if (next.has(key)) next.delete(key);
238230
else next.add(key);
@@ -259,7 +251,7 @@ export function UserLimitsTab({ userId }: UserLimitsTabProps) {
259251
count: selected.size,
260252
}),
261253
);
262-
setSelected(new Set());
254+
setSelection(new Set());
263255
await reload();
264256
} catch (e) {
265257
toast.error(e instanceof Error ? e.message : t('common.operationFailed'));

0 commit comments

Comments
 (0)