Skip to content

Spec 010 — Frontend conventions ​

Status: implemented Branch: 010-frontend-conventions

Status is set by the human, never by the agent. It moves draft → approved → implemented.

Problem ​

apps/api has docs/architecture/nestjs.md: a new feature module follows a documented shape rather than re-deciding one. apps/web has no such document, and — unlike the api, whose conventions were established across specs 003–008 and then written down — it has almost no code to derive them from. Three hand-written files exist (App.tsx, routes/health-page.tsx, api/index.ts), and they already disagree with themselves: App.tsx is PascalCase, health-page.tsx is kebab-case. panda.config.ts carries no tokens, so the first feature to render an item invents its own rarity colours. useHealth hand-rolls a pending/error/success union that every consuming component must branch on — one component today, one copy of three branches per endpoint thereafter.

The legendary planner is the next feature, and it is the largest UI in the product. Every convention left unstated now gets decided implicitly by whoever writes that first, and re-litigated in review. This spec decides them while the cost of deciding is three files.

User stories ​

Ordered by priority. Each story must be independently testable and shippable — if only P1 ships, there is still something usable.

P1 — A web feature has one documented shape ​

As the developer, I want one written answer to "where does this file go and what is it called" for apps/web, so that a feature is laid out the same way whoever — or whatever — writes it, and review argues about the feature rather than its filing.

Independent test: with only this story implemented, docs/architecture/react.md exists and the existing health feature has been moved to match it; a reader can place every file of a hypothetical new feature from the document alone.

Acceptance scenarios

  1. Given docs/architecture/react.md, when a developer adds a feature, then the document states where its components, hooks, route table, constants and tests live, and every existing file in apps/web/src already obeys it.
  2. Given a feature folder under src/features/<feature>/, when it needs a route, then the route table lives in that folder (routes.tsx) and main.tsx only assembles the feature route tables it is given — no route is declared outside a feature.
  3. Given any file in apps/web/src, when it is named, then its filename equals the symbol it exports (HealthPage.tsx exports HealthPage, usePlanner.ts exports usePlanner).
  4. Given a component rendered directly by a route, when it is named, then it carries the Page suffix; a component not rendered directly by a route does not.
  5. Given two features, when one needs something the other has, then the shared thing moves to src/shared/ (or src/api for server data) — neither feature imports the other by any path.
  6. Given state that is not server data, when it is stored, then filters and what the user is looking at live in URL search params, and only ephemeral UI state (open, hover, focus) lives in useState.

P2 — Server data reaches components exactly one way ​

As the developer, I want every component to receive server data already fetched and already validated, so that no component contains a loading branch, an error branch, or a fetch call, and the endpoint count stops multiplying that boilerplate.

Independent test: with only this story implemented, useHealth returns Health rather than a union, HealthPage renders it with no status branching, and the app still shows the round-trip.

Acceptance scenarios

  1. Given a facade hook in src/api, when a component calls it, then it receives validated data directly — no status field to branch on, no undefined to guard.
  2. Given a pending request, when the component renders, then the fallback comes from the route's <Suspense> boundary, not from a branch inside the component.
  3. Given a request that fails, or a response that fails Zod validation, when the component renders, then both surface through the same error boundary, and the boundary is declared once per route rather than per component.
  4. Given any file outside src/api, when it needs server data, then it calls a custom hook; it does not import @tanstack/react-query and does not import src/api/generated.

P3 — The conventions are enforced by tests, not vigilance ​

As the developer, I want each convention that a machine can check to be checked by a test that fails when it is broken, so that the document describes a state the repo is actually in rather than one it was in when the document was written.

Independent test: with only this story implemented, deliberately violating each enforced rule in a scratch file makes pnpm test fail, and reverting makes it pass.

Acceptance scenarios

  1. Given a file in one feature importing from another feature, when pnpm test runs, then a guard test fails and names the offending file.
  2. Given a file outside src/api importing @tanstack/react-query or src/api/generated, whenpnpm test runs, then a guard test fails and names it.
  3. Given a component containing useMemo, useCallback or React.memo, when pnpm test runs, then a guard test fails and names it.
  4. Given a style declaration containing a literal colour, when pnpm test runs, then a guard test fails and names it.

P4 — The visual language has its own vocabulary ​

As the developer, I want the design system written down separately from the code conventions, with the domain's own values in it, so that rarity colours and surfaces are looked up rather than invented per feature, and a visual change is not buried in a document about file layout.

Independent test: with only this story implemented, docs/architecture/design-system.md exists, panda.config.ts defines the token layer it describes, and the existing components reference tokens.

Acceptance scenarios

  1. Given panda.config.ts, when a component needs an item rarity colour, then a token for that rarity exists and the component references it by name.
  2. Given a component's styles, when they are written, then they reference tokens; a literal hex or px value is rejected (P3 #4).
  3. Given a repeated style variant, when it is first written, then it is a colocated cva in the component's own file; it becomes a config recipe only when the component moves to src/shared/ui.

P5 — Memoization stops being a judgement call ​

As the developer, I want the React Compiler to handle memoization, so that the rule is "never hand-write it" rather than "hand-write it when you have profiled", which is the rule people get wrong in both directions.

Independent test: with only this story implemented, the compiler runs in the build, the existing components compile without bail-out, and no hand-memoization exists in apps/web.

Acceptance scenarios

  1. Given the Vite build, when it runs, then the React Compiler transforms apps/web components and the production build succeeds.
  2. Given a component the compiler cannot safely transform, when the codebase is checked, then that bail-out is detectable rather than silent.

Requirements ​

Layout and naming

  • R1 — apps/web/src is laid out as: api/ (contract boundary), features/<feature>/ (leaf-only), shared/ (shared-only, containing ui/ and lib/), plus the App.tsx shell and main.tsx manifest. src/api does not move under shared/ — it is the Orval output target named by stack.md, and relocating it would change generated artefacts for a cosmetic gain.
  • R2 — A feature folder owns its components, hooks, route table (routes.tsx), constants and tests. Tests live in <feature>/__tests__/; src/api's own tests live in src/api/__tests__/, so the existing src/api/boundary.test.ts moves there.
  • R3 — A filename equals the symbol it exports. Components are PascalCase (HealthPage.tsx), hooks and plain modules camelCase (usePlanner.ts, planMath.ts). The exception is a module named for its contents rather than a single export — routes.tsx, constants.ts, index.ts — which is the only case where a file may export several symbols.
  • R4 — A component rendered directly by a route is named <Name>Page. Nested components carry no suffix.
  • R5 — main.tsx assembles feature route tables and contains no route definitions of its own — the role app.module.ts plays for the api.
  • R6 — A feature never imports another feature, by any path. On a second consumer, the shared thing is promoted to src/shared/ui (presentational), src/shared/lib (pure, no React) or src/api (server data).
  • R7 — Algorithmic code lives in plain .ts modules with no React import, so it is tested as a function rather than through a render.

Data flow

  • R8 — Facade hooks in src/api wrap useSuspenseQuery, validate the response with its generated Zod schema, and return the validated data directly. No hook returns a status union. Confirmed — research.md F2: this holds without changing the Orval configuration, but only through the adapter R25 introduces; useSuspenseQuery rejects the generated options factory as-is.
  • R9 — A failed request and a failed validation surface the same way: as a throw, caught by an error boundary. Qualified — research.md V3: TanStack throws to the boundary only when there is no data to show, so a failed background refetch over a warm cache keeps rendering the stale value. react.md states this exception explicitly rather than leaving it to be mistaken for a bug.
  • R10 — Each route declares one <Suspense> fallback and one error boundary. Feature components contain no loading or error branch for server data.
  • R11 — Data fetching happens only in a custom hook. @tanstack/react-query is imported only within src/api; src/api/generated is imported only within src/api (already true, spec 004 SC6).
  • R12 — Non-server state has three homes and no others: URL search params for filters and what the user is looking at; src/api (TanStack Query) for everything the server knows, the player's owned materials included; useState for ephemeral UI. No global store, no context used as a data store.

Styling

  • R13 — panda.config.ts defines a token layer: item rarity colours from the domain, plus semantic tokens for surface and text. Components reference tokens; literal colour values are rejected. Confirmed — research.md F6: Panda 1.11.5 emits both the rarity tokens and a _dark-conditioned semantic token. One consequence for R18's guard: a semantic token emits no CSS variable until something references it, so the assertion belongs on the generated types, not on styles.css.
  • R14 — A style variant starts as a colocated cva in the component's file and becomes a config recipe only when the component is promoted to src/shared/ui.
  • R15 — The design system is documented in docs/architecture/design-system.md, separate from the code conventions in docs/architecture/react.md.

Memoization

  • R16 — The React Compiler is enabled for apps/web. useMemo, useCallback and React.memo are not written by hand. Confirmed — research.md V1: the existing components compile without bail-out, at a measured cost of +110 ms on the Vite build (259 → 367 ms, warm).
  • R16a — The compiler is wired as react(), babel({ presets: [reactCompilerPreset()] }), not through a babel option on @vitejs/plugin-react — that option does not exist in v6, and passing it is ignored in silence. This adds four devDependencies (babel-plugin-react-compiler, @rolldown/plugin-babel, @babel/core, @types/babel__core) and reintroduces Babel into a Vite 8 build that is otherwise Babel-free. That trade is accepted deliberately and recorded in react.md. Corrected — research.md F1; the first measurement showed no build-time change precisely because the spec's assumed wiring did nothing.
  • R17 — A compiler bail-out is detectable rather than silent, through reactCompilerPreset({ panicThreshold: 'all_errors' }), which turns a bail-out into a build failure naming the file and the rule. No linter, no healthcheck CLI, no ESLint. Confirmed — research.md V2: Biome 2.5.5 carries no react-compiler rules (0 of 523), so the compiler itself is the detector.
  • R17a — Because the compiler runs only in the build — apps/web/vitest.config.ts does not wire it, and the compiler only sees files in the module graph — CI must run pnpm build, not the test suite alone, for R17 to hold. Both limits are stated in react.md. research.md V2 caveat.

Enforcement

  • R18 — Every convention a machine can check has a test that fails on violation and names the offending file, extending the pattern apps/web/src/api/boundary.test.ts already establishes: cross-feature imports (R6), fetching outside src/api (R11), hand-memoization (R16), literal colours (R13).
  • R19 — Conventions that cannot be machine-checked are stated as prose in react.md and are not pretended to be enforced.
  • R19a — R3 is enforced by Biome, not by a guard test: useFilenamingConvention with filenameCases: ["export", "camelCase"], raised to error and scoped to apps/web/** through overrides — unscoped it would fail every kebab-case file in apps/api. src/test-setup.ts fails this rule and is renamed to testSetup.ts. Refuted then resolved — research.md V4: the spec assumed Biome could not express this; "export" is a supported filename case, so the hedged guard test is dropped.
  • R20 — The two documents are listed in CLAUDE.md's Reference section alongside nestjs.md.

Divergences to record

  • R21 — react.md closes with a section naming where apps/web deliberately diverges from generic React advice and from apps/api, so neither is "corrected" later: PascalCase filenames against the api's kebab-case; __tests__/ folders against the api's colocated .test.ts; no hand-memoization; no global state library; Suspense-only data access.

Dependencies

  • R22 — This spec adds five devDependencies and no runtime dependencies: msw (facade-tier tests) plus the four the compiler needs (R16a).
  • R22a — msw also requires an explicit allowBuilds entry in pnpm-workspace.yaml, set to false — its postinstall only copies a browser service worker the jsdom tier never uses. Without an entry, pnpm writes an unresolved placeholder that blocks every subsequent pnpm command, pnpm exec vitest included. research.md F3; same gate stack.md documents for @swc/core.
  • R22b — No react-error-boundary dependency. The boundary is a class component in src/shared/ui; reset-after-error uses TanStack's own QueryErrorResetBoundary. Confirmed — research.md V3.

The facade adapter

  • R25 — src/api owns a named, tested suspenseOptions adapter: Orval types the generated queryFn as QueryFunction | typeof skipToken, which useSuspenseQuery forbids, so every facade hook narrows through this one function rather than improvising. The narrowing lives in a plain function, never inline before the hook call — a guard there would be a conditional-hook violation and would fail R17's build check. No cast and no any: plain narrowing typechecks clean under exactOptionalPropertyTypes. research.md F2.

Testing

  • R23 — Feature component tests render against a mocked facade — fast, no network. The facade's own tests use MSW so that the generated hook, the Zod validator and the malformed-payload path are exercised for real, once, in the layer that owns them. Confirmed — research.md F4: MSW intercepts the generated client's relative /api/health under jsdom, and a malformed payload throws from the validator as SC6 requires.
  • R23a — Moving the tests needs no Vitest change: apps/web/vitest.config.ts's src/** globs are depth-agnostic and already match __tests__/. research.md F5.
  • R24 — Every acceptance scenario and success criterion here maps to a named test (project Definition of Done).

HTTP contract ​

No endpoint is added, removed or changed by this spec, and apps/api/openapi.json is untouched. The work is confined to apps/web and docs/. The contract pipeline described in stack.md is a constraint on this spec rather than an output of it: src/api/generated stays generated, never hand-edited, and pnpm verify:contract must remain green throughout (SC7).

Success criteria ​

Measurable and technology-agnostic — outcomes, not implementation.

  • SC1 — A new feature can be added touching no file outside its own folder, except the one line in the manifest that mounts its routes.
  • SC2 — No component in apps/web contains a loading branch, an error branch, or a fetch call for server data.
  • SC3 — Violating any machine-checked convention fails pnpm test, and the failure names the offending file.
  • SC4 — No literal colour value appears in apps/web/src; every colour resolves through a token.
  • SC5 — No hand-written memoization exists in apps/web/src.
  • SC6 — A malformed API response fails a test rather than reaching a component.
  • SC7 — pnpm typecheck, pnpm test, pnpm lint, pnpm build and pnpm verify:contract are all green, and the app runs with the health round-trip still visible.
  • SC8 — Every convention stated in react.md and design-system.md is true of the repository at merge — the documents describe the code, not an intention.

Out of scope ​

  • Dark mode and theming beyond the token layer's structure.
  • Accessibility tooling (axe, automated a11y assertions) and visual regression testing.
  • Internationalisation.
  • Any planner UI, or any feature beyond moving the existing health round-trip to the new shape.
  • Production networking, CORS and real origins (deferred by spec 004 R12, still deferred).
  • Moving src/api under shared/, and any change to the Orval configuration or generated output.
  • A global state library.

Assumptions ​

  • The stack is as pinned today: React 19.2, React Router 8, TanStack Query 5, Base UI 1.6, Panda 1.11, Vite 8, Biome 2.5.5, Vitest 4 with Testing Library.
  • Biome's react domain rules are already active for apps/web — useHookAtTopLevel, useExhaustiveDependencies and noArrayIndexKey fire today under preset: recommended, confirmed by probe in this session. This spec adds to that baseline rather than establishing it.
  • docs/architecture/nestjs.md is the sibling document this one mirrors. It exists in the working tree but is not yet on origin/main; react.md does not depend on it being merged first.
  • The player's owned materials will arrive from the GW2 API through apps/api, so they are server state. Nothing in this spec builds a client-side inventory store.
  • No feature beyond health exists to migrate, so the reference implementation is the health round-trip.

Traceability ​

Each acceptance scenario and success criterion must map to a named test. Fill this in during implementation.

CriterionTest
P1 #1No single test — docs/architecture/react.md states the shape (T8), and compliance is corroborated by the guard/naming suite passing on every moved file: apps/web/src/features/health/__tests__/routes.test.tsx ("R5/P1 #2: the feature owns its own route table"), Biome's noRestrictedImports (spec 012's replacement for the deleted boundary.test.ts scan of @tanstack/react-query imports), apps/web/src/__tests__/conventions.test.ts, and Biome's useFilenamingConvention. The "every existing file already obeys it" clause is a human-review claim, per T8's "re-read both documents against the repo" step.
P1 #2apps/web/src/features/health/__tests__/routes.test.tsx — "R5/P1 #2: the feature owns its own route table"
P1 #3Not a test — enforced by Biome: useFilenamingConvention (filenameCases: ["export", "camelCase"], error, scoped to apps/web/** in biome.json's overrides). Verified by pnpm lint (0 errors; see verification log).
P1 #4apps/web/src/__tests__/conventions.test.ts — "P1 #4: every component a route renders is named <Name>Page"
P1 #5apps/web/src/__tests__/conventions.test.ts — "R6/P1 #5/SC3: no feature imports another feature" (violation-catching proof: "R6/P3 #1: the rule catches a violation")
P1 #6Prose only — no enforcement layer. plan.md §Test strategy states R12 (the three state homes) has no guard because no client state exists in the app yet; docs/architecture/react.md states the rule as prose under R19.
P2 #1apps/web/src/api/__tests__/useHealth.test.tsx — "R8/P2 #1: returns validated data, not a status union"; apps/web/src/features/health/__tests__/HealthPage.test.tsx — "SC2/P2 #1: renders the status with no branch of its own"
P2 #2Not test-covered — plan.md's Test strategy and task-9-brief.md both state no test asserts the Suspense fallback timing. Code-level evidence: apps/web/src/App.tsx declares one <Suspense fallback={<p>Loading…</p>}> around <Outlet />. Manually verified this session: pnpm dev (api on :3000, web on :5173), curl http://localhost:5173/api/health round-tripped {"status":"ok"} through the dev proxy — confirming the app renders past the fallback to the real data. The fallback's own on-screen appearance was not visually observed in this session (browser automation was unavailable in this environment); this is an admitted gap, not a fabricated observation.
P2 #3apps/web/src/shared/ui/__tests__/ErrorBoundary.test.tsx — "R22b/P2 #3: renders the fallback when a child throws"; apps/web/src/api/__tests__/useHealth.test.tsx — "SC6: a malformed payload fails a test rather than reaching a component" (both a failed request and a failed validation surface through the same boundary)
P2 #4Amended by spec 012 — the conventions.test.ts scanner named here ("R11/P2 #4/SC3: …" and its violation-catching proof) was replaced by Biome's noRestrictedImports in biome.json, which fails pnpm lint, names the offending line, and additionally catches a feature-local api/ folder and dynamic import(). boundary.test.ts's two scanners, which checked the same rule a second time, were deleted with it — the convention now has exactly one owner, as this spec's own enforcement table requires.
P3 #1apps/web/src/__tests__/conventions.test.ts — "R6/P1 #5/SC3: no feature imports another feature" and its violation-catching proof "R6/P3 #1: the rule catches a violation"
P3 #2Amended by spec 012 — same replacement as P2 #4. The "catches a violation" proof is no longer a test: Biome's rule was verified against throwaway violating files before the scanner was deleted (docs/architecture/react.md, How the two import rules are scoped).
P3 #3apps/web/src/__tests__/conventions.test.ts — "R16/SC5: nothing is memoized by hand" and its violation-catching proof "R16/P3 #3: the rule catches a violation"
P3 #4apps/web/src/__tests__/conventions.test.ts — "P4 #2/SC4: no literal colour values" and its violation-catching proof "P3 #4: the rule catches a violation"
P4 #1apps/web/src/__tests__/tokens.test.ts — "P4 #1: every GW2 item rarity has a colour token"
P4 #2apps/web/src/__tests__/conventions.test.ts — "P4 #2/SC4: no literal colour values"
P4 #3Prose only — no enforcement layer. plan.md §Test strategy states R14 (colocated cva promoted to a config recipe on move to shared/ui) has no guard because nothing is shared yet; docs/architecture/react.md/design-system.md state the rule as prose under R19.
P5 #1Not a unit test — enforced by the React Compiler at build time: pnpm build succeeds with reactCompilerPreset({ panicThreshold: 'all_errors' }) wired in apps/web/vite.config.ts (confirmed green this session; see verification log). Wiring itself is checked by apps/web/src/__tests__/viteProxy.test.ts — "R16a: the compiler is wired through @rolldown/plugin-babel, not a \babel` option"and"R16a: the babel plugin is wired into the plugins array, not merely imported"`.
P5 #2Not a permanent test — apps/web/src/__tests__/viteProxy.test.ts — "R17: a bail-out fails the build rather than passing silently" checks the panicThreshold: 'all_errors' wiring only. The actual bail-out-fails-the-build behaviour was proven once during implementation by building with a deliberately violating component and observing the failure (plan.md §Test strategy), then discarding that fixture — a committed violating file would break every subsequent build.
SC1Not testable by design — no test can prove a counterfactual feature. Verified by human review of the diff: features/health/ is self-contained (its own components, hook consumption, route table, tests), and the guard suite (apps/web/src/__tests__/conventions.test.ts's cross-feature-import check) would fail if a future feature broke the "own folder" property.
SC2apps/web/src/features/health/__tests__/HealthPage.test.tsx — "SC2/P2 #1: renders the status with no branch of its own"
SC3apps/web/src/__tests__/conventions.test.ts — "R6/P1 #5/SC3: no feature imports another feature" plus its violation-catching proof, which assert the offending file is named in the failure. Amended by spec 012 — the react-query half moved to Biome's noRestrictedImports (see P2 #4), which names the offending file and line, so SC3 is better served than when this row was written.
SC4apps/web/src/__tests__/conventions.test.ts — "P4 #2/SC4: no literal colour values"
SC5apps/web/src/__tests__/conventions.test.ts — "R16/SC5: nothing is memoized by hand"
SC6apps/web/src/api/__tests__/useHealth.test.tsx — "SC6: a malformed payload fails a test rather than reaching a component". Amended by spec 012 — the boundary.test.ts half is now Biome's noRestrictedImports (see P2 #4).
SC7The full command set, run this session from the repo root: pnpm typecheck (clean), pnpm test (43 files / 236 passed, 1 expected fail — the pre-existing tests/workflow/settings.test.ts it.fails), pnpm lint (0 errors, 6 pre-existing warnings unrelated to this spec), pnpm build (both apps built), pnpm verify:contract (green, git diff --exit-code on openapi.json and src/api/generated clean). App run manually via pnpm dev: the health round-trip returned {"status":"ok"} through the dev proxy.
SC8Human review (T8: "re-read both documents against the repo") plus the enforcement table in docs/architecture/react.md, where every convention names its layer (Biome / guard test / React Compiler) and the layer is confirmed to exist by the rows above; the honest exceptions (P1 #6/R12, P4 #3/R14, and R7) are stated as prose rather than pretended to be enforced.