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
- 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 inapps/web/srcalready obeys it. - Given a feature folder under
src/features/<feature>/, when it needs a route, then the route table lives in that folder (routes.tsx) andmain.tsxonly assembles the feature route tables it is given — no route is declared outside a feature. - Given any file in
apps/web/src, when it is named, then its filename equals the symbol it exports (HealthPage.tsxexportsHealthPage,usePlanner.tsexportsusePlanner). - Given a component rendered directly by a route, when it is named, then it carries the
Pagesuffix; a component not rendered directly by a route does not. - Given two features, when one needs something the other has, then the shared thing moves to
src/shared/(orsrc/apifor server data) — neither feature imports the other by any path. - 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
- Given a facade hook in
src/api, when a component calls it, then it receives validated data directly — nostatusfield to branch on, noundefinedto guard. - 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. - 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.
- Given any file outside
src/api, when it needs server data, then it calls a custom hook; it does not import@tanstack/react-queryand does not importsrc/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
- Given a file in one feature importing from another feature, when
pnpm testruns, then a guard test fails and names the offending file. - Given a file outside
src/apiimporting@tanstack/react-queryorsrc/api/generated, whenpnpm testruns, then a guard test fails and names it. - Given a component containing
useMemo,useCallbackorReact.memo, whenpnpm testruns, then a guard test fails and names it. - Given a style declaration containing a literal colour, when
pnpm testruns, 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
- 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. - Given a component's styles, when they are written, then they reference tokens; a literal hex or px value is rejected (P3 #4).
- Given a repeated style variant, when it is first written, then it is a colocated
cvain the component's own file; it becomes a config recipe only when the component moves tosrc/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
- Given the Vite build, when it runs, then the React Compiler transforms
apps/webcomponents and the production build succeeds. - 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/srcis laid out as:api/(contract boundary),features/<feature>/(leaf-only),shared/(shared-only, containingui/andlib/), plus theApp.tsxshell andmain.tsxmanifest.src/apidoes not move undershared/— it is the Orval output target named bystack.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 insrc/api/__tests__/, so the existingsrc/api/boundary.test.tsmoves 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.tsxassembles feature route tables and contains no route definitions of its own — the roleapp.module.tsplays 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) orsrc/api(server data). - R7 — Algorithmic code lives in plain
.tsmodules with no React import, so it is tested as a function rather than through a render.
Data flow
- R8 — Facade hooks in
src/apiwrapuseSuspenseQuery, 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;useSuspenseQueryrejects 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.mdstates 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-queryis imported only withinsrc/api;src/api/generatedis imported only withinsrc/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;useStatefor ephemeral UI. No global store, no context used as a data store.
Styling
- R13 —
panda.config.tsdefines 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 onstyles.css. - R14 — A style variant starts as a colocated
cvain the component's file and becomes a config recipe only when the component is promoted tosrc/shared/ui. - R15 — The design system is documented in
docs/architecture/design-system.md, separate from the code conventions indocs/architecture/react.md.
Memoization
- R16 — The React Compiler is enabled for
apps/web.useMemo,useCallbackandReact.memoare 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 ababeloption 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 inreact.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.tsdoes not wire it, and the compiler only sees files in the module graph — CI must runpnpm build, not the test suite alone, for R17 to hold. Both limits are stated inreact.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.tsalready establishes: cross-feature imports (R6), fetching outsidesrc/api(R11), hand-memoization (R16), literal colours (R13). - R19 — Conventions that cannot be machine-checked are stated as prose in
react.mdand are not pretended to be enforced. - R19a — R3 is enforced by Biome, not by a guard test:
useFilenamingConventionwithfilenameCases: ["export", "camelCase"], raised toerrorand scoped toapps/web/**throughoverrides— unscoped it would fail every kebab-case file inapps/api.src/test-setup.tsfails this rule and is renamed totestSetup.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 alongsidenestjs.md.
Divergences to record
- R21 —
react.mdcloses with a section naming whereapps/webdeliberately diverges from generic React advice and fromapps/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 —
mswalso requires an explicitallowBuildsentry inpnpm-workspace.yaml, set tofalse— its postinstall only copies a browser service worker the jsdom tier never uses. Without an entry,pnpmwrites an unresolved placeholder that blocks every subsequentpnpmcommand,pnpm exec vitestincluded. research.md F3; same gatestack.mddocuments for@swc/core. - R22b — No
react-error-boundarydependency. The boundary is a class component insrc/shared/ui; reset-after-error uses TanStack's ownQueryErrorResetBoundary. Confirmed — research.md V3.
The facade adapter
- R25 —
src/apiowns a named, testedsuspenseOptionsadapter: Orval types the generatedqueryFnasQueryFunction | typeof skipToken, whichuseSuspenseQueryforbids, 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 noany: plain narrowing typechecks clean underexactOptionalPropertyTypes. 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/healthunder 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'ssrc/**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/webcontains 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 buildandpnpm verify:contractare all green, and the app runs with the health round-trip still visible. - SC8 — Every convention stated in
react.mdanddesign-system.mdis 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/apiundershared/, 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
reactdomain rules are already active forapps/web—useHookAtTopLevel,useExhaustiveDependenciesandnoArrayIndexKeyfire today underpreset: recommended, confirmed by probe in this session. This spec adds to that baseline rather than establishing it. docs/architecture/nestjs.mdis the sibling document this one mirrors. It exists in the working tree but is not yet onorigin/main;react.mddoes 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.
| Criterion | Test |
|---|---|
| P1 #1 | No 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 #2 | apps/web/src/features/health/__tests__/routes.test.tsx — "R5/P1 #2: the feature owns its own route table" |
| P1 #3 | Not 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 #4 | apps/web/src/__tests__/conventions.test.ts — "P1 #4: every component a route renders is named <Name>Page" |
| P1 #5 | apps/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 #6 | Prose 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 #1 | apps/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 #2 | Not 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 #3 | apps/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 #4 | Amended 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 #1 | apps/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 #2 | Amended 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 #3 | apps/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 #4 | apps/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 #1 | apps/web/src/__tests__/tokens.test.ts — "P4 #1: every GW2 item rarity has a colour token" |
| P4 #2 | apps/web/src/__tests__/conventions.test.ts — "P4 #2/SC4: no literal colour values" |
| P4 #3 | Prose 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 #1 | Not 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 #2 | Not 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. |
| SC1 | Not 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. |
| SC2 | apps/web/src/features/health/__tests__/HealthPage.test.tsx — "SC2/P2 #1: renders the status with no branch of its own" |
| SC3 | apps/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. |
| SC4 | apps/web/src/__tests__/conventions.test.ts — "P4 #2/SC4: no literal colour values" |
| SC5 | apps/web/src/__tests__/conventions.test.ts — "R16/SC5: nothing is memoized by hand" |
| SC6 | apps/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). |
| SC7 | The 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. |
| SC8 | Human 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. |