Files
ersatztv/docs/spa-conventions.md
T
timothyandClaude Fable 5 d9faa8be7b docs: spec-follows-runtime naming decision + casing wart (#198)
- api-conventions.md §5a: runtime Newtonsoft casing vs generated spec, the
  schema transformer that mirrors it, and the contract test guarding it.
- decisions.md: append the "wire format is source of truth; spec follows via the
  real contract resolver" decision.
- spa-conventions.md §4: trust the generated key casing; note the removed
  troubleshooting escape hatch and runtime-cased test mocks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 20:00:48 +02:00

8.6 KiB

SPA conventions — "Add a screen" playbook

Purpose: a precise playbook for adding a new screen (or sub-path editor) to the ChicoryTV React SPA (web/), for an agent with no prior context in this repo. Update this doc in the same PR that changes any convention below.

Companion to api-conventions.md (the API surface the SPA talks to) and docs/contributing.md (general repo conventions).

1. Stack & layout

Vite + React + TypeScript, builds to ErsatzTV/wwwroot/app (see web/vite.config.ts: base: '/app/', build.outDir: '../ErsatzTV/wwwroot/app'), served by the ASP.NET host at /app.

  • Routes + nav: web/src/App.tsx — one big route table of ScreenRoute objects (path, label, title, kicker, icon, etc.) plus an allowSubPaths?: boolean flag.
  • Screens: web/src/screens/*.tsx, one file per top-level screen, generally with a colocated *.test.tsx.
  • API clients: web/src/api/<domain>.ts (see §4).
  • Styling: web/src/shell.css (+ web/src/components/components.css) — utility classes with a ctv- prefix (~690 occurrences across those two files). Reuse an existing ctv-* class before inventing a new one.

2. CRITICAL: sub-path screens must own their own pathname state

If a route sets allowSubPaths: true (e.g. so /app/blocks/{id} works under the /app/blocks nav entry), the screen component itself must track window.location.pathname and listen for popstate — do not rely on App.tsx re-rendering ScreenContent when the sub-path changes.

Why: App.tsx's routeFromLocation() matches an allowSubPaths route by prefix (pathname.startsWith(\${route.path}/`)) and returns the **same ScreenRouteobject reference** for the base path and every sub-path under it.App's state update is setActiveRoute(routeFromLocation()); React's useStatesetter bails viaObject.iswhen the new value is reference-equal to the old one — so navigating from/app/blocksto/app/blocks/42(or between/app/blocks/42and/app/blocks/17) **never re-invokes ScreenContent** at the Applevel. See the comment block directly abovePlayoutsRouteScreeninApp.tsx (~line 3540) for the canonical explanation, and its implementation (useState(() => window.location.pathname)+useEffectwith apopstate` listener local to the wrapper component) for the fix.

Exemplars of screens that already do this correctly: BlocksScreen.tsx, TemplatesScreen.tsx, DecosScreen.tsx, DecoTemplatesScreen.tsx, and the PlayoutsRouteScreen wrapper in App.tsx (which owns two sibling sub-paths, /playouts/{id}/alternate-schedules and /playouts/{id}/templates, dispatching internally via parsePlayoutSubRoute).

3. Data loading pattern

Reference implementation: web/src/screens/LogsScreen.tsx. Structure to copy for any screen that fetches from the API:

  • A discriminated-union state type covering loading/success/error, e.g. type LogsState = { status: 'loading'; ... } | { status: 'success'; ... } | { status: 'error'; ... }.
  • A seqRef (monotonically incremented request counter) + activeRef (mount-tracking boolean, flipped in a mount/unmount useEffect) pair — guards against a stale, slower request overwriting a newer one's result, and against setting state after unmount.
  • The actual fetch lives in a useCallback (load), called from a separate useEffect(() => { load(); }, [load]).
  • Lint rule — react-hooks "no set-state-in-effect": never call setState synchronously in the body of a useEffect. State transitions happen only inside event handlers or promise .then()/.catch() callbacks (as in LogsScreen's load). This is enforced by eslint-plugin-react-hooks in web/eslint.config.js — a synchronous setState in an effect body will fail npm run lint.

4. API client modules

One file per domain in web/src/api/, e.g. logs.ts, blocks.ts, playouts.ts. Pattern (see web/src/api/logs.ts):

  • Re-export the generated response/DTO types from ./generated/v1: export type LogEntry = components['schemas']['LogEntryResponseModel'];
  • A typed params interface for the endpoint's query string (e.g. GetLogsParams).
  • The fetch function builds a URLSearchParams from only the params that are set, then calls the shared request<T>(url) helper from ./client.
  • An error-message helper (e.g. messageFromLogsError) that narrows unknownApiError (from ./client) → a human string, with a fallback message — screens use this instead of stringifying errors themselves.
  • web/src/api/index.ts re-exports everything so screens import from '../api', not from the individual domain file directly.

Trust the generated key casing — it mirrors the runtime. Since #198 the OpenAPI spec is generated to match the runtime Newtonsoft serializer exactly (a schema transformer runs the same contract resolver; see api-conventions.md §5a), so the generated types carry the real wire keys — including Newtonsoft's acronym quirks like ffmpegProfileId (channel FFmpeg-profile id) and ffmpegProfile (channel FFmpeg-profile display name). Do not hand-cast responses to "fix" a key or dual-read a spec-cased vs runtime-cased key (the old PlaybackTroubleshootingScreen #198 escape hatch that read data.channel.fFmpegProfileId has been removed — read ffmpegProfileId straight off the typed response). When you mock an API response in a test, use the generated (runtime) casing.

5. Artwork rendering

Render item.artwork / item.poster (or whatever the DTO field is named) directly as an <img src> — since PR #181, API responses already return rooted, directly-usable URLs (see api-conventions.md §4). Do not client-side-prefix artwork paths (no /artwork/posters/ string building in SPA code) — if you see that pattern, it's stale/wrong.

5b. HLS video preview

Screens that preview an ErsatzTV HLS stream use the reusable HlsPlayer component (web/src/media/HlsPlayer.tsx, introduced with #145). Pass it a src (the .m3u8 URL, or null for idle) and — when the manifest GET itself starts a server-side session (e.g. troubleshooting playback.m3u8) — a playToken you increment per play, so a repeat play with an identical URL still tears down and re-attaches (an unchanged src alone is a state no-op that never issues a new request); it attaches hls.js when Media Source Extensions are available and falls back to native HLS (video.canPlayType('application/vnd.apple.mpegurl'), i.e. Safari) otherwise, and tears down the hls.js instance on src change and unmount. Its config mirrors the legacy _Host.cshtml previewChannel (liveDurationInfinity: true + an unbounded manifest maxTimeToFirstByteMs) because the troubleshooting playback.m3u8 endpoint blocks until segments exist before it 302s to the live manifest. In tests, mock hls.js wholesale (vi.mock('hls.js', …) with a class exposing static isSupported(), static Events, and loadSource/attachMedia/on/destroy) so jsdom never touches a real MediaSource; assert the manifest URL via the mocked loadSource spy (see PlaybackTroubleshootingScreen.test.tsx).

6. Tests

  • vitest, colocated *.test.ts / *.test.tsx next to the source file.
  • Every screen with meaningful logic gets a screen test; every API client module gets a param-mapping / URL-building test (e.g. logs.test.ts next to logs.ts).
  • web/src/App.test.tsx covers navigation + the route table, including regressions like the sub-path bug in §2 (see the tests around PlayoutsRouteScreen, ~line 1682+, that click into /app/playouts/{id}/... sub-paths and assert the correct sub-screen rendered).
  • Nav-label test-selector care: getByRole('link'/'button', { name: /Regex/ }) matches by substring by default — a loose regex can match more than one nav item. Verified example: the System nav button is matched with an anchored regex (name: /^System/) rather than a bare /System/, specifically to avoid ambiguous matches against other labels that start with or contain "System". Anchor (^/$) or use exact strings in getByRole name matchers whenever a new label could be a substring of (or share a substring with) an existing one — check App.tsx's nav label: list for collisions before picking a new label.

7. Verification gate — run before every commit touching web/

From web/:

npm test          # vitest
npm run lint      # eslint .
npm run build     # tsc -b && vite build

Also run npm run check:api if you touched anything OpenAPI-relevant (see api-conventions.md §5) — it regenerates src/api/generated/v1.d.ts and fails the build if it's out of sync with what's committed.