gridpilot.gg/plans/website-architecture-audit.md

Website architecture audit: Template vs Client, Server Actions, Mutations, React Query

Contract baseline: docs/architecture/website/WEBSITE_CONTRACT.md

0) Executive summary

The codebase is not uniformly wrong; it is inconsistent.

Two competing architectures coexist:

1. Contract-first RSC (good):

   • app/**/page.tsx calls PageQuery → builds ViewData → renders Template (examples below).
   • Writes go through Server Actions → Mutations (examples below).

2. React Query client app (problematic when used for writes):

   • Client modules execute writes via React Query useMutation() directly against services (or placeholders).
   • This violates the strict write boundary in docs/architecture/website/FORM_SUBMISSION.md.

Additionally, some Templates violate determinism rules (notably toLocaleString() usage) which can cause SSR/hydration mismatches.

My recommendation to get to a clean codebase:

• Keep React Query only for client-side reads (and optional cache invalidation after server actions), but ban React Query mutations for writes.
• Enforce a single write path everywhere: Client intent → Server Action → Mutation → Service → API Client.
• Align Templates with determinism and responsibility rules: Templates render; they do not format using runtime-locale APIs.

---

1) What the code is doing today (evidence)

1.1 A good reference read flow (RSC → PageQuery → Template)

• Server page calls PageQuery and renders Template: DashboardPage()
• Template is a pure renderer receiving ViewData: DashboardTemplate()

This matches the contract read flow in docs/architecture/website/WEBSITE_CONTRACT.md.

1.2 A good reference write flow (Server Action → Mutation)

• Server action calls a mutation and revalidates: updateUserStatus()
• Server action calls a mutation and revalidates: updateProfileAction()
• Server action calls a mutation and revalidates: completeOnboardingAction()

This matches the contract write flow in docs/architecture/website/WEBSITE_CONTRACT.md and the mutation guidance in docs/architecture/website/MUTATIONS.md.

1.3 React Query is definitely used, and it is used for writes

React Query is a dependency: apps/website/package.json

React Query provider exists: QueryClientProvider()

There is a standardized abstraction that promotes React Query usage for both reads and writes:

• usePageData()
• usePageMutation()

Concrete contract violations:

• Client-side writes via React Query mutation calling a service:

  • useMutation() calling leagueService.createAdminScheduleRace() and similar write methods.
  • This is a direct breach of the strict rule: “All writes MUST enter through Next.js Server Actions” in docs/architecture/website/FORM_SUBMISSION.md.

• Duplicate write paths exist for onboarding:

  • Server action exists: completeOnboardingAction()
  • But a React Query mutation hook also calls a service directly: useCompleteOnboarding()

• Some “mutation hooks” are placeholders (dangerous because they normalize the wrong path):

  • useJoinTeam()
  • useLeaveTeam()
  • These currently do no server action and no API call, but they establish the pattern that client-side “writes” live in React Query.

1.4 Template vs Client responsibilities are inconsistent

A clean example of the intended split exists:

• Server page acts as data orchestrator: Page()
• Client entry-point owns state and navigation: DriversPageClient()
• Template is basically composition/rendering: DriversTemplate()

But the actual boundaries are blurry in at least two ways:

1. Templates contain non-deterministic formatting:

   • toLocaleString()
   • This violates strict determinism constraints in docs/architecture/website/VIEW_DATA.md and docs/architecture/website/DISPLAY_OBJECTS.md.

2. app/**/*PageClient.tsx sometimes becomes “everything”: state, rendering, presentation model transformation, and workaround ViewModel creation.

   • Example: “TODO wtf we have builders for this” comment while doing ViewData → ViewModel ad-hoc mapping in LeaguesPageClient() and specifically the conversion at LeagueSummaryViewModel.

1.5 There are plain bugs/anti-patterns that amplify the mess

• useState() used as an effect runner (should be useEffect()):
  • useState()
  • This is not an architecture issue per se, but it is “noise” that makes architecture discussions harder.

---

2) The root causes (why this became a mess)

1. Write boundary not enforced strongly enough

   • There are good server actions + mutation examples, but nothing stops new client code from doing useMutation() → service.write().

2. Templates and ViewData rules are not consistently applied

   • Determinism rules (no Intl.*, no toLocale*) are strict in docs, but not enforced in Templates.

3. Two read strategies are both used

   • RSC PageQueries exist.
   • React Query hooks exist.
   • Without a clear “when to use which,” teams will mix-and-match.

---

3) Recommended target architecture (single coherent system)

3.1 Writes: one true path

Hard rule (enforced):

• Client code MUST NOT perform write HTTP requests.
• Client code MUST NOT call write methods on services that hit the API.
• Client code MUST NOT use React Query useMutation() for writes.

Canonical write flow (matches docs):

flowchart TD
  UI[Client component intent] --> SA[Server Action]
  SA --> M[Mutation]
  M --> S[Service]
  S --> AC[API Client]
  AC --> API[Backend API]
  SA --> RV[Revalidate Path]

Docs reference: docs/architecture/website/FORM_SUBMISSION.md

3.2 Reads: choose a default, allow an exception

Default:

• If the route can be rendered on the server: page.tsx → PageQuery → ViewDataBuilder → Template.

Exception:

• Use React Query for client-only interactive islands that truly need client-driven data refresh.
• React Query should be treated as a client state/cache of authoritative API truth, not the truth itself.

Canonical read flow option A (RSC):

flowchart TD
  Page[page.tsx] --> PQ[PageQuery]
  PQ --> S[Service]
  S --> AC[API Client]
  PQ --> B[ViewDataBuilder]
  B --> T[Template]

Docs reference: docs/architecture/website/WEBSITE_CONTRACT.md

Canonical read flow option B (client-only island):

flowchart TD
  C[Client component] --> Q[React Query useQuery]
  Q --> S[Service or API Client read]
  C --> T[Template]

Important: This is only acceptable for reads. Writes still go through server actions.

---

4) Concrete refactor plan (incremental, low-risk)

Step 1: Establish explicit conventions

1. app/**/page.tsx MUST be server by default and must only orchestrate:

   • call PageQuery
   • map Result → notFound()/redirect()/error UI
   • build ViewData using Builder
   • render Template

2. app/**/*PageClient.tsx is the ONLY place allowed to:

   • use useRouter()
   • hold UI state
   • call server actions

3. templates/**/*Template.tsx MUST:

   • accept ViewData (or client-only “presentation props” like handler functions)
   • contain no Intl.* or toLocale* formatting
   • contain no data fetching

Step 2: Fix strict correctness violations first (determinism)

• Remove runtime-locale formatting from templates:
  • Replace toLocaleString() and toLocaleString() with deterministic formatting.
  • Preferred: ViewData already contains display-ready strings (built via ViewDataBuilder), or deterministic Display Objects.
  • Docs reference: docs/architecture/website/VIEW_DATA.md.

Step 3: Move client-side writes behind server actions

Primary offender to migrate:

• Replace React Query write mutations in LeagueAdminSchedulePageClient()
  • Replace useMutation() with server actions (publish/save/delete) that call Mutations.
  • Then the client component calls those server actions and refreshes view (either router.refresh() or route revalidation).

Secondary offenders:

• Consolidate onboarding writes to one path:
  • Keep server actions: completeOnboardingAction() and generateAvatarsAction()
  • Delete or rewrite client hook useCompleteOnboarding() to call server action instead of a service.

Step 4: Clean up presentation model transforms

• Eliminate ad-hoc “ViewData → ViewModel” conversions in client code:
  • Example to refactor: the conversion at LeagueSummaryViewModel.
  • Replace with either:
    • a proper ViewModelBuilder (if the component truly needs ViewModel semantics), or
    • adjust the component (LeagueCard) to accept ViewData shape directly.

Step 5: Reduce React Query surface area (optional, but recommended)

• Deprecate the abstraction that encourages client-side writes:
  • Keep usePageData() for reads if desired.
  • Remove or forbid usePageMutation().

Step 6: Add guardrails to prevent regression

Minimal guardrails that pay off:

1. ESLint rule or convention: forbid useMutation imports except in __tests__ or an allowlist.
2. ESLint rule: forbid service write methods from being imported into client modules (harder, but possible with path rules).
3. ESLint rule: forbid toLocaleString() and Intl.* in Templates.

---

5) Answers to your specific concerns

5.1 *Template.tsx vs *Client.tsx

• This split is correct and already present in multiple routes.
• The inconsistency is that some “Client” files are doing too much (transform, render, and mutate).
• The fix is to treat *PageClient.tsx as wiring + state + calling server actions, and keep templates deterministic.

5.2 Server actions vs mutations

• The codebase already shows the correct approach:

  • updateUserStatus()
  • updateProfileAction()

• The “mess” is client-side writes bypassing this.

5.4 Command Models: Are they useless?

No. Command Models (e.g., LoginCommandModel) remain valuable as optional UX helpers.

• Purpose: They manage transient form state and perform client-side UX validation.
• Relationship to Server Actions: They prepare the DTO that is passed to the Server Action.
• Boundary: They belong to the client-side "intent collection" phase. Once the Server Action is called, the Command Model's job is done.
• Redundancy: They are not redundant with ViewModels. ViewModels are for the read path (API -> UI), while Command Models are for the write path (UI -> API).

5.5 Identified places for Command Model usage

The following components currently use complex React state for forms and should be refactored to use Command Models:

1. CreateLeagueWizard.tsx: Needs a CreateLeagueCommandModel to manage multi-step wizard state and validation.
2. LeagueAdminSchedulePageClient.tsx: Should use a RaceScheduleCommandModel for the track/car/date fields.
3. ProtestDetailPageClient.tsx: Should fully leverage the existing ProtestDecisionCommandModel.
4. SponsorSignupPage: Needs a SponsorSignupCommandModel to clean up the formData and errors state.

---

6) Opinion: ViewData vs ViewModel (what actually makes sense)

Your original intuition is sound:

• ViewData = server output (JSON, deterministic, template-ready)
• ViewModel = client-only (state, derived UI helpers, never serialized)

The awkward part is: the repo currently contains conflicting written contracts.

• Strict statement: “Templates accept ViewData only” in docs/architecture/website/WEBSITE_CONTRACT.md.
• But docs/architecture/website/VIEW_DATA.md shows an example where a Template receives a ViewModel in client code.

My recommendation: keep Templates ViewData-only, keep ViewModels client-only

I would enforce this as the single, clean rule:

1. Templates accept ViewData only (one prop-shape; server and client use the same templates).
2. Client components may use ViewModels internally for state and derived values.
3. The boundary is explicit: client code converts ViewModel -> ViewData right before rendering a Template.

Why this is the least-bad option:

• It prevents a “two template APIs” world (one for ViewData, one for ViewModel) which tends to explode complexity.
• It aligns with RSC constraints (serializable data across the server/client boundary).
• It keeps ViewModels useful: they can own derived UI state and display logic, but the render surface is still a plain object.

Concrete example of why you feel “ViewModels are pushed away by ViewData”:

• In LeaguesPageClient() you’re holding ViewData and then doing a hand-rolled mapping to a “ViewModel-like” shape at LeagueSummaryViewModel. That’s a symptom of missing builders and an unclear boundary, not a reason to abandon ViewModels.

7) Implementation Status (Audit Results)

The following cleanups have been implemented:

1. Deterministic Formatting: Removed toLocaleString() and toLocaleDateString() from all templates. Introduced NumberDisplay and DateDisplay display objects for deterministic formatting.
2. Server Action Migration: Migrated the schedule administration logic in LeagueAdminSchedulePageClient.tsx from React Query useMutation to Next.js Server Actions.
3. Onboarding Consolidation: Updated onboarding hooks to use server actions instead of direct service calls.
4. ViewModel Builders: Introduced LeagueSummaryViewModelBuilder to eliminate ad-hoc ViewData -> ViewModel mapping in LeaguesPageClient.tsx.
5. React Query Deprecation: Deprecated usePageMutation in usePageData.ts.
6. Guardrails: Added gridpilot-rules/no-use-mutation-in-client ESLint rule to prevent future React Query write usage.
7. Command Models:
   • Created RaceScheduleCommandModel and integrated it into LeagueAdminSchedulePageClient.tsx.
   • Created SponsorSignupCommandModel and integrated it into SponsorSignupPage.
   • Ensured LeagueWizardCommandModel is ready for future refactoring of the complex wizard.

8) Next Steps for the Team

• Continue migrating remaining useMutation usages to Server Actions.
• Ensure all new templates follow the ViewData-only rule.
• Use NumberDisplay and DateDisplay for all UI formatting.