Files
paseo/docs/coding-standards.md
Mohamed Boudra adc9b3b43c Preserve loader behavior while reducing hidden work (#1996)
* fix(app): preserve loader behavior in retained panels

Keep the running footer mounted across panel switches. Inactive loaders stop their animation work, then realign to the original wall-clock phase before they become visible.

* perf(app): remove inactive loader wrapper worklet

* perf(app): pause hidden turn elapsed timers

* perf(app): reduce loader animation work

Keep one wall-clock-aligned clock for visible loaders and detach retained
hidden loaders without unmounting them. Preserve the 950 ms sequence and
reduced-motion behavior.
2026-07-11 13:25:24 +02:00

9.6 KiB

Coding Standards

The core instinct: AI-generated code hedges — it covers every case, layers over instead of cutting in, scatters uncertainty everywhere, wraps in case. A senior engineer commits — to a shape, a boundary, a name, a happy path, a type — and lets everything else fall into place. Every rule below catches a different form of indecision.

For testing rules, see testing.md.

Core principles

  • Zero complexity budget — every abstraction must justify itself with a specific, current benefit.
  • YAGNI — build features and abstractions only when needed. A function called once is indirection, not abstraction.
  • No "while I'm at it" cleanups — make the change you came for. Drive-by edits hide in the diff.
  • Functional and declarative over object-oriented.
  • function declarations over arrow function assignments.
  • interface over type when both work.
  • No index.ts barrel files that only re-export — they create indirection and circular-dep risk. Import from the source.

Comments and noise

  • Delete any comment where removing it loses zero information. Comments explain why, not what.
  • No tutorial comments explaining language features (// Use destructuring to...).
  • No decorative section dividers (// ===== Helpers =====). Use files and modules to organize, not ASCII art.
  • No hedging comments (// might need to revisit, // should work for most cases). If you're unsure, investigate.
  • No commented-out code. Git remembers.
  • No console.log / debugger left behind. No TODO: implement stubs — if it needs to exist, write it.

Confidence: commit to a shape

  • Validate at boundaries (network, IPC, user input, file I/O), trust types internally. After the parse, the value is what its type says.
  • Every ?. and ?? past the validation boundary is unconfident code — either the boundary should resolve it, or the type should reflect reality.
  • No defensive checks for conditions the type system already rules out (if (!agent) return on a non-nullable parameter).
  • No try/catch "just in case." If you can't say what you're catching and why, don't catch.
  • Optionality is a design decision, not a migration shortcut. Distinct valid states → discriminated union. Intentionally empty → explicit null. Keep optionality at real boundaries.

Types

  • No any. No as casts to bypass errors. No @ts-ignore / @ts-expect-error. Narrow with if / schema validation; let the compiler check harder, not less.
  • If a Zod schema exists, the TypeScript type is z.infer<typeof schema>. Never hand-write a parallel type.
  • One canonical type per concept. Layer-specific views are Pick / Omit, not duplicated fields.
  • Name multi-property object shapes — no inline Array<{ ... }> or Promise<{ ... }> in signatures, returns, or generic args.
  • Use string literal unions, not raw string, when the value is one of a known set. Catches typos at compile time.
  • Object parameters past the obvious-name threshold: 3+ args, any boolean arg, any optional arg → object. (thing, true, false, true) is unreadable at the call site.
  • Make impossible states impossible — discriminated unions over { isLoading; error?; data? } bags.

Errors

  • Throw typed error classes that carry the fields a caller would want to read. Plain Error("Provider X not found") collapses structured info into a string.
  • Catch blocks branch on instanceof for what they can handle; rethrow the rest. No catch (e) { return null }.
  • Separate user-facing copy from log/debug strings — don't make one string serve telemetry, logs, and the UI.
  • Fail explicitly. If the caller asked for X and X isn't available, throw — don't silently substitute Y.

Density

  • Nested ternaries are forbidden. A single ternary is fine only when both branches are a single identifier or trivial access (x ? a : b).
  • Boolean expressions with 2+ clauses or mixed concerns → name the conditions.
  • Object literals assemble pre-computed values; don't pack branching and lookups into property positions.
  • Operations wrapping operations (Object.fromEntries(arr.filter(...).map(...)), Math.max(...xs.map(...))) → break into named intermediates.
  • Max 3 levels of nesting (callbacks, JSX, control flow). Above that, extract.

Structure and modules

  • A directory is a module, not a namespace. One intentional public surface; internal files stay internal.
  • Path is part of the name — prefer provider/registry.ts over provider/provider-registry.ts. If the filename has to do double duty, deepen the path.
  • Filenames ending in -utils, -helpers, -manager, -handler, -controller, -formatter, -builder are a smell — the path didn't carry enough domain.
  • Boundary returns answer the caller's question (getActiveAgents()), not "here's my storage" (getAgents().filter(...) repeated everywhere).
  • One adapter means a hypothetical seam; two adapters means a real one. Don't define a port until something actually varies across it.
  • Pass-through modules fail the deletion test — if removing the module makes callers go straight to what they wanted, delete it.
  • Centralize policy. The same discriminator (plan, provider, kind, status) branched in 3+ files → policy table, not another else if per case.
  • New features get a home before implementation. A feature smeared across 5 shared files is the same slop as a flat-peer namespace.
  • Don't drop new files at the nearest root just because placement is unclear — say so and ask.

Refactoring is a bolt-on test

  • A change should look like a thoughtful edit to existing code, not a new layer next to it. New coordinator wrapping a coordinator, new flag bypassing the normal path, new helper duplicating an existing selector — stop and reshape instead.
  • Refactors preserve behavior by default. No removing features to simplify code without explicit approval.
  • Have a verification plan before refactoring — name the invariants, confirm a test holds them, write one if not. See testing.md.
  • Migrate all callers and remove old paths in the same refactor. No fallback behavior unless explicitly designed.

React

  • useEffect is for synchronizing with external systems (DOM, network, timers, subscriptions). Not for transforming React state. Derived state → compute in render or useMemo.
  • No effect cascades — chains of effects setting state that triggers more effects almost always want React Query or a reducer.
  • useRef is for DOM refs and non-rendering identities (timer IDs, AbortController, latest-callback caches). If the value affects what renders next, it's state — model it explicitly with useReducer and a discriminated union.
  • Server state goes through React Query. Manual useState + useEffect + isLoading + error for fetched data is always worse.
  • Components render and dispatch — they don't compute transitions. Two-plus interacting useStates → extract a reducer.
  • Never define components inside other components. Module-scope only.
  • Subscribe narrowly: select primitives from stores, pass status not agent, use useShallow / deep-equal when returning derived arrays/objects.
  • Collection rows do not independently subscribe to a high-frequency global store. The collection owner selects structurally shared indexes once, derives a keyed row model with useMemo, and passes entries to rows. This keeps retained hidden collections current without running one selector per row on every store update.
  • Equality functions prevent React renders; they do not prevent selector callbacks from running. A selector attached to a hot store must be O(1) when its relevant source references have not changed.
  • Retained native panels use RetainedPanel. If an existing gesture/layout wrapper must own visibility, wrap its contents in RetainedPanelActivity instead. Keep keyed panel roots in a stable sibling order, include the newly active panel in the same render, centralize subscriptions, and gate genuine effects through useRetainedPanelActive. Do not use Suspense or render freezing for this on native: those techniques change native tree ownership instead of merely stopping work.
  • Infinite animations are subscriptions. Start them only while their retained panel is active, and cancel a shared clock when its final active consumer leaves. Synchronized animations use one clock per animation family and feed active instances through local shared values; retained hidden instances stay mounted but unsubscribed. Match state updates to the actual visual cadence; do not run every style worklet at 60 fps when the rendered value changes only a few times per second.
  • Stable references for props that cross memo boundaries or feed dependency arrays. Static literals at module scope as const; derived with useMemo; handlers with useCallback only when there's a memoized beneficiary.
  • Use stable ids for key, never array index for reorderable/filterable lists.
  • Context for stable values (theme, auth). Store with selectors for state that changes.

Naming

  • Names describe meaning, not mechanics. submitForm over handleOnClickButtonSubmit. running over filteredArrayOfRunningAgents.
  • The right length is the shortest unambiguous in context. Inside AgentManager, methods are start, stop, list.
  • Match the surrounding code's vocabulary. If the codebase uses getX, don't introduce fetchX / retrieveX for the same shape.
  • Don't leak implementation into names — getAgent, not queryPostgresForAgent. If swapping the impl would force a rename, the name is wrong.
  • Booleans read as yes/no questions: isX, hasX, canX. Avoid negative booleans (isNotConnected).
  • data, result, info, manager, temp are smells — say what the thing is.