shoc-frontend-new/ARCHITECTURE_AND_CODE_QUALITY.md
Alexandre Brandizzi 4337cc662b
Some checks are pending
CI / ci (push) Waiting to run
CI / governance (push) Waiting to run
Deploy / deploy (push) Waiting to run
chore(governance): enforce frontend quality system (#53)
* chore(governance): make React/TS conventions mandatory via executable gates

Add AGENTS.md, QUALITY_GATES.md, ARCHITECTURE_AND_CODE_QUALITY.md, and
REVIEW_AND_PR_FRAMEWORK.md as the binding conventions and PR review
contract for humans and all coding/review agents.

Add a single 'npm run verify' command (format + lint + build + test +
governance) and 'npm run governance', which runs a dependency-free godfile
ratchet (whole-repo, baseline in scripts/governance-baseline.json) and a
changed-file maintainability gate (complexity<=20, function<=150, params<=4,
depth<=4) via ESLint. Legacy is handled by ratchets, not relaxation: 5
godfiles over 500 lines are grandfathered debt; maintainability thresholds
apply to changed TS/TSX (72 legacy violations across ~51 files otherwise).

Add a repo-owned 'governance' CI job that runs 'npm run verify' so every
gate is guaranteed from this repository, independent of the org reusable
workflow.

* fix(governance): make frontend ratchets fail closed
2026-07-24 16:47:34 -03:00

7.1 KiB
Raw Permalink Blame History

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 the Text when prop for one-sided conditions; never cond ? <Element/> : null. (ESLint no-restricted-syntax.)
  • MUST make the left operand of && in JSX entirely boolean. Coerce presence with Boolean(value) (or Boolean(a || b)); preserve numeric/empty-string semantics and TypeScript narrowing with count > 0, value != null. {count && <X/>} renders 0 and is rejected by the type-aware seahaven/no-non-boolean-jsx-and rule. (See FRONTEND_MAINTAINABILITY.md for the approved guard forms.)
  • MUST NOT widen typed element-slot props (icon, action, actions) back to ReactNode; coerce with Boolean(prop) before &&.

Typography and feedback

  • MUST use Text from @/components/ui/text for paragraphs, headings, descriptions, labels, captions, code, and async feedback. Raw <p> and <h1>–<h6> are rejected. (ESLint no-restricted-syntax.)
  • MUST use Text variant="error" for the vp-error styling; the vp-error class on any non-Text element is rejected (seahaven/no-vp-error-outside-text). Do not compose vp-error dynamically to bypass the static check.
  • MUST NOT pass component, role, or aria-live to Text — they are omitted from TextProps so the variant contract (element, tone, live region) cannot be overridden. Use as and tone.
  • SHOULD keep the feedback (polite status) region mounted and toggle text with when; mount error (assertive alert) 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 a VITE_ variable.

Hooks, performance, and Big-O

  • MUST satisfy react-hooks recommended rules (including exhaustive-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/useCallback only 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.