From 020963855200f46c69d306ad4bbb7a6a1b181b1d Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Mon, 8 Jun 2026 09:21:16 +0200 Subject: [PATCH] docs: consolidate frontend guardrails + test-harness gotchas Adds web/GUARDRAILS.md capturing the recurring CI-guardrail and test-harness lessons in one place: the check:size (250 KB-gz largest chunk) and check:colors (design-token) guards, the jsdom/storybook vitest split, MSW onUnhandledRequest:"error" overrides, RTL accessible-name collisions, Storybook nested-router/portal handling, and the components/ui code-style split. Wires a pointer from CLAUDE.md. All claims verified against the live scripts, ci.yaml, and src/test/. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 1 + web/GUARDRAILS.md | 146 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 web/GUARDRAILS.md diff --git a/CLAUDE.md b/CLAUDE.md index 502cd6d..a6469b1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,3 +28,4 @@ cargo clippy --workspace --all-targets -- -D warnings # lint before committing - **Code navigation:** prefer the insikt LSP server over grep/glob — it resolves macro-generated symbols that text search misses. (insikt runs standalone, not via the gateway MCP.) - **Dependencies:** manage via the `cargo-mcp` server rather than editing `Cargo.toml` by hand. - **Formatting:** `cargo +nightly fmt` (nightly toolchain required). +- **Frontend guardrails:** before touching `web/`, read **[web/GUARDRAILS.md](web/GUARDRAILS.md)** — it covers the CI gate (`check:size` 250 KB-gz budget, `check:colors` design-token enforcement) and the test-harness quirks (MSW `onUnhandledRequest: "error"`, the jsdom/storybook vitest split, RTL accessible-name collisions, Storybook nested-router and portal handling, and the `components/ui/` code-style split). diff --git a/web/GUARDRAILS.md b/web/GUARDRAILS.md new file mode 100644 index 0000000..3f7f644 --- /dev/null +++ b/web/GUARDRAILS.md @@ -0,0 +1,146 @@ +# Frontend Guardrails & Test Harness + +The web frontend has a handful of CI guardrails and test-harness quirks that trip +new work in the same predictable ways. This file consolidates them so the lessons +are enforced up front instead of rediscovered one failing run at a time. + +The canonical gate is `.gitea/workflows/ci.yaml`, which runs (in order) from `web/`: + +``` +pnpm typecheck # tsc -b --noEmit +pnpm lint # eslint . +pnpm test # vitest run (jsdom + storybook projects) +pnpm build # tsc -b && vite build +pnpm check:size # scripts/check-bundle-size.mjs +pnpm check:colors # scripts/check-no-raw-colors.mjs +``` + +Run the whole gate locally before pushing. The two `check:*` scripts only have +meaningful output **after** `pnpm build` has produced `dist/`. + +--- + +## Guardrail: bundle size (`check:size`) + +`scripts/check-bundle-size.mjs` fails if the **largest single JS chunk** exceeds +**250 KB gzipped**. It measures every file in `dist/assets/*.js` and reports the +biggest. + +- The budget is on the *largest chunk*, not the total. Code-split chunks are + measured independently — a heavy primitive that lands in its own lazy chunk + costs that chunk's size, not the entry bundle's. Past additions (Base UI + Select ≈12.6 KB gz, the Drawer ≈12.7 KB gz) paid their cost in a separate + `*.js` chunk and left the main `index` chunk untouched. +- **When a UI primitive pushes the main chunk over budget**, extract the + component that uses it into its own module and `React.lazy()` it (the codebase + already lazy-loads `ObjectEditForm` / `FieldsPage`). This is the standard fix, + especially for anything rendered conditionally (e.g. only on a narrow + viewport) — the wide path then never pays for it. +- Adding one more Base UI primitive to the always-loaded app shell costs + single-digit KB gz; there is comfortable headroom under 250. + +## Guardrail: design tokens / no raw colors (`check:colors`) + +`scripts/check-no-raw-colors.mjs` fails if any raw Tailwind palette utility +appears **outside `src/components/ui/`**. The regex matches +`{text|bg|border|ring|fill|stroke|from|to|via|decoration|outline|divide|placeholder}-{palette}-{50..950}` +(palette = `neutral|gray|slate|…|rose`). + +- In app / shell code use **token classes only**: `bg-accent`, `text-foreground`, + `text-muted-foreground`, `bg-primary`, etc. Numerics like `gap-2` and token + names are never flagged — only `palette-shade` pairs are. +- `src/components/ui/` (the vendored kit) is **exempt** and may use raw colors. +- New status/semantic colors must be exposed as tokens, not raw palette classes. + In Tailwind v4 a token utility (`bg-success`, `text-warning`) only *exists* if + the token is declared in `@theme inline` as `--color-: var(--)` — + defining the raw CSS var in `:root` alone is not enough. Opacity modifiers on + tokens (`bg-primary/10`) do resolve. +- For a sweeping token migration, add/enable the `check:colors` guard **last** + (after the file-by-file swap) — it can only pass once the whole migration is + complete. + +> **Contrast caveat:** do not trust a WCAG contrast ratio quoted in a plan or a +> prior review — recompute it (OKLCH → linear sRGB → relative luminance) before +> changing a color token. A near-black foreground on a *lighter* colored +> background has *higher* contrast; a single wrong measurement has propagated +> through spec + plan more than once here. + +--- + +## Test harness: two vitest projects + +`vite.config.ts` defines **two** vitest projects, and a test file is claimed by +exactly one: + +| Project | Environment | Claims | +|-------------|----------------------|-------------------------| +| (default) | jsdom + `src/test/setup.ts` | `*.test.ts(x)` | +| `storybook` | chromium (Playwright, via `storybookTest`) | `*.stories.tsx` | + +- Component tests render through **`renderApp`** (`src/test/render.tsx`), which + wraps `QueryClientProvider` + a `createMemoryRouter` and **side-imports + `../i18n`** — so i18n is auto-initialized and you do **not** add extra + providers. Pass an initial route via `renderApp(ui, { route: "/x" })`. +- Story-as-test runs the story's `play()` fn in a real browser: + `pnpm vitest run `. A canonical pass reports + "1 Test File, 1 Test passed". A first-cold-cache storybook run can emit a + transient `Cannot read properties of null (reading useEffect)` from + `QueryClientProvider` — it does not reproduce once the cache warms; re-run to + confirm green. + +## Test harness: MSW handlers (`onUnhandledRequest: "error"`) + +`src/test/setup.ts` starts the MSW server with **`onUnhandledRequest: "error"`** +and `resetHandlers()` after each test. Shared handlers live in +`src/test/handlers.ts`. + +- Any request with **no matching handler fails the test** with + `[MSW] Cannot bypass a request when using the error strategy`. If a hook hits + an endpoint the shared handlers don't cover (e.g. `PATCH /…/terms/:id` for an + update-mutation success test), **add a per-test override** — + `server.use(http.patch(url, () => new HttpResponse(null, { status: 204 })))` — + rather than editing the shared `handlers.ts`. The `afterEach` reset keeps the + override test-local. + +## Test harness: React Testing Library accessible-name collisions + +`getByRole('textbox', { name: /key/i })` throws on **multiple matches** when two +inputs resolve to the same accessible name — e.g. a create-form input +(`