7.1 KiB
ARCHITECTURE_AND_CODE_QUALITY.md — MUST / MUST NOT conventions
Mandatory React/TypeScript conventions for this repository. Each rule lists how
it is enforced. Detailed rationale for rendering and typography lives in
docs/FRONTEND_MAINTAINABILITY.md. When a rule
says "MUST", it is enforced by lint, build, or npm run governance; "SHOULD"
means it is a review-enforced convention backed by the
PR review contract.
Conditional rendering
- MUST use
&&or theTextwhenprop for one-sided conditions; nevercond ? <Element/> : null. (ESLintno-restricted-syntax.) - MUST make the left operand of
&&in JSX entirely boolean. Coerce presence withBoolean(value)(orBoolean(a || b)); preserve numeric/empty-string semantics and TypeScript narrowing withcount > 0,value != null.{count && <X/>}renders0and is rejected by the type-awareseahaven/no-non-boolean-jsx-andrule. (See FRONTEND_MAINTAINABILITY.md for the approved guard forms.) - MUST NOT widen typed element-slot props (
icon,action,actions) back toReactNode; coerce withBoolean(prop)before&&.
Typography and feedback
- MUST use
Textfrom@/components/ui/textfor paragraphs, headings, descriptions, labels, captions, code, and async feedback. Raw<p>and<h1>–<h6>are rejected. (ESLintno-restricted-syntax.) - MUST use
Text variant="error"for thevp-errorstyling; thevp-errorclass on any non-Textelement is rejected (seahaven/no-vp-error-outside-text). Do not composevp-errordynamically to bypass the static check. - MUST NOT pass
component,role, oraria-livetoText— they are omitted fromTextPropsso the variant contract (element, tone, live region) cannot be overridden. Useasandtone. - SHOULD keep the
feedback(politestatus) region mounted and toggle text withwhen; mounterror(assertivealert) on demand.
Forms and mutations
- MUST use the libraries already established: React Hook Form for field registration/lifecycle, Zod for validation and inferred types, TanStack Query for server reads and mutations (pending/error state, invalidation, retries).
- MUST NOT add TanStack Form alongside React Hook Form for one form. A second form convention increases cognitive and dependency cost. Replacing React Hook Form requires an approved repo-wide migration ADR with measured benefits, a codemod/migration plan, and removal of the superseded dependency.
- SHOULD keep file upload selection/validation in a focused component and drive upload progress/errors/retry through a TanStack Query mutation, not in a route-sized page.
State ownership (bounded)
- MUST keep query loading/error/empty state adjacent to the query result; mutation pending/error state in the component that initiated the mutation.
- MUST compose focused state components instead of accumulating unrelated booleans in a page. Route pages coordinate sections and navigation; reusable sections own their interaction details.
- SHOULD render errors inline with accessible feedback (
Text variant="error"); reserve toasts for cross-page outcomes.
Data access
- MUST keep TanStack Query keys stable and descriptive (a consistent entity + identity tuple, co-located with the query). Unstable or ad-hoc keys break caching and invalidation. (Review-enforced; a future lint rule is tracked as a gap.)
- MUST NOT materialize large server collections into client state and then filter/sort them in the component when the server (or a memoized, virtualized layer) should own it. Prefer server-side filtering/pagination; if client-side is required, memoize and avoid re-filtering on every render. (Review-enforced.)
- MUST invalidate the right query keys after a mutation so caches do not show stale data.
Security
- MUST NOT surface raw server error payloads, stack traces, or internal
identifiers to end users. Map server errors to a safe user-facing message
(e.g.
Text variant="error"); log the full detail only to controlled channels. (Review-enforced.) - MUST NOT bake secrets, tokens, or account-specific values into the build.
Only
VITE_*build-time vars are allowed, and they are baked into the bundle — never put a secret in aVITE_variable.
Hooks, performance, and Big-O
- MUST satisfy
react-hooksrecommended rules (includingexhaustive-deps) under the zero-warnings gate. - SHOULD avoid
O(n²)or worse work inside render/hot paths; memoize derived data, key lists stably, and prefer server-side filtering for large sets (reviewers flag algorithmic complexity — see REVIEW_AND_PR_FRAMEWORK.md). - SHOULD keep renders pure; derive expensive values with
useMemo/useCallbackonly when measured to matter (no speculative memoization).
Maintainability (measured thresholds)
Enforced by npm run governance (QUALITY_GATES.md).
These are conservative code-shape proxies for review focus. File length is not
proof of a god object, cyclomatic complexity is not runtime Big-O, and neither
substitutes for behavior tests, profiling, query-plan evidence, or reviewer
judgment.
| Metric | Threshold | How enforced |
|---|---|---|
| File length (godfile) | ≤ 500 lines | Whole-repo ratchet: any file over the cap not in scripts/governance-baseline.json fails. |
| Cyclomatic complexity | ≤ 20 | Changed-file ESLint complexity. |
| Function length | ≤ 150 lines | Changed-file ESLint max-lines-per-function (skipComments). |
| Parameters | ≤ 4 | Changed-file ESLint max-params. |
| Nesting depth | ≤ 4 | Changed-file ESLint max-depth. |
Why these are changed-file ratchets, not whole-repo errors: measured against
the current codebase, the maintainability thresholds surface 72 violations
across ~51 legacy files (45 function-length, 24 complexity, 3 params), and
5 files exceed 500 lines. Applying them repo-wide at error would block
legacy without a migration. Instead:
- The 5 oversized files are explicit grandfathered debt with frozen
per-file caps in
scripts/governance-baseline.json. They may shrink but never grow; new entries and cap increases fail. - The complexity/function-length/params/depth rules apply to changed TS/TSX on a PR/push, so new and modified code must comply while untouched legacy is not blocked. Bring legacy into compliance when you next touch it.
These thresholds are starting ratchets: tighten them (lower caps, fewer baseline entries) as debt is paid down. Never raise them to make a change pass.