BillTracker/docs/CODE_STANDARDS.md

6.9 KiB
Raw Blame History

Code Standards

The living rulebook for this codebase. One canonical way to do each thing, enforced in CI so it can't drift. Sections marked (enforced) fail CI; (target) are being rolled out. Update this doc when a standard changes.

Tooling & enforcement (enforced)

  • Formatting: Prettier (.prettierrc.json) — single quotes, semicolons, 2-space, printWidth: 100, trailing commas. Run npm run format; CI runs format:check.
  • Lint: npm run lint (eslint .) covers client and server. Focused rule set (react-hooks on the client; js/recommended + TS-unused on the server). Errors block CI; warnings are advisory (ratchet down over time).
  • Typecheck: npm run typecheck (client, strict + noUncheckedIndexedAccess) and npm run typecheck:server (server). Both run in CI. All routes, middleware, workers, server.cts, and most services are fully type-checked. Only 8 files keep @ts-nocheck — 4 db migration/seed factories + 4 dynamic parsers (spreadsheet/userDbImport/subscription/oidc) that parse inherently-unknown input or mutate arbitrary historical schemas; each carries an INTENTIONAL rationale header. A new bare @ts-nocheck on a route or handler is a regression (B0 recon counts them; steady state ≤ 8). Dynamic DB rows stay any by design (see types/db.d.ts); annotate params/callbacks rather than suppressing whole files.
  • Local hooks (lefthook): pre-commit auto-formats staged files + lints; pre-push typechecks. Installed by npm install (the prepare script).
  • Dead code: npm run knip (advisory) for unused exports/deps.
  • Full gate: npm run ci = format:check + lint + typecheck (×2) + check:server + tests (server + client) + build. The CI pipeline runs the same.

Modules & files

  • Server is CommonJS TypeScript .cts, run by Node via type-stripping (no build step). Every migrated module is required with the explicit .cts/.mts extension (extensionless does not resolve). Each .cts leads with an import type … line so type-stripping activates.
  • Client is ESM .ts/.tsx with the @/* path alias.
  • Money is integer cents in the DB and on the wire; convert to dollars only at the edge via the money helpers. Branded Cents/Dollars types where practical.

HTTP API conventions

  • Wire format: JSON, snake_case field names (mirrors DB columns), money as integer cents.
  • Error responses (enforced): every error body is { error, message, code, field? } with the correct HTTP status. Throw the factories from utils/apiError.cts (ValidationError, NotFoundError, ConflictError, AuthError, ForbiddenError, or new ApiError(code, message, status, { field }) for anything else) and let the single terminal error handler format them — do not hand-roll res.status().json({ error }). Express 5 forwards rejected async handlers automatically, so route bodies do not wrap everything in try/catch; a thrown ApiError keeps its message on the wire while any other 5xx is masked + logged by the terminal handler (that masking is the whitelist: wrap a message in ApiError only when it is deliberately user-facing). Clients read message/code + status. Canonical example: routes/categories.cts. Enforced by: the eslint no-restricted-imports ratchet on routes/** (all 29 route files converted 2026-07). Documented exceptions (each commented at the site): routes/transactions.cts (internal { error } parse-result convention, rethrown at the boundary via throwStandardized), routes/import.cts (sendImportError error-id envelope), routes/admin.cts sendError (backup download can fail mid-stream after headers are sent), payments' DUPLICATE_SUSPECTED 409 and transactions' bulk-unmatch 500 (both carry extra payload fields the client renders), routes/calendarFeed.cts (text/plain for ICS clients).
  • Client error shape: client/api.ts builds every thrown error through toApiError() — one construction site; on a 401 outside /auth/* it dispatches auth:expired so AuthProvider/RequireAuth redirect to login. Mutations follow the React Query patterns in client/hooks/useQueries.ts / usePaymentActions.ts (invalidate + toast on error).
  • Success envelope (target): reads return the bare resource/list; mutations return { success: true, … }.
  • Validation (target): validate inputs with the shared validators (throwing ValidationError); do not scatter ad-hoc parseInt/range checks.

Security (target; see also SECURITY.md)

  • Every mutating route carries CSRF + auth + an appropriate rate-limit + input validation. Admin routes require requireAdmin.
  • Every user-scoped query filters user_id and deleted_at IS NULL. Never interpolate user input into SQL — use bound parameters.
  • Never log secrets, tokens, or raw financial values.
  • Fail fast at boot on missing required config (e.g. TOKEN_ENCRYPTION_KEY in production).

Logging (enforced)

  • Use utils/logger.cts (const { log } = require('.../utils/logger.cts')) — log.debug/info/warn/error, console-compatible. Level via LOG_LEVEL (default: debug in dev, info in prod). It redacts secrets/PII (sensitive keys, bearer/JWT) so tokens/passwords never reach logs. All server runtime console.* has been migrated. Do not use raw console.* in server code.

Data integrity

  • Every multi-statement write runs inside db.transaction().
  • Migrations are append-only versioned entries in db/migrations/versionedMigrations.cts (idempotent; guard column adds with PRAGMA table_info).

Testing

  • Server: node:test in tests/*.test.js. Client: Vitest (*.test.ts[x]). E2E: Playwright (npm run test:e2e, probe subset test:e2e:probe). Add coverage for new handlers; prove money invariants with property tests where practical.

Typed SQL (Kysely — available, adopt incrementally)

db/kysely.cts provides Kysely as a type-safe query builder executed synchronously. The app is sync (better-sqlite3) and Kysely's .execute() is async, so we never execute through Kysely — we build the query (columns + user_id/deleted_at scoping + types all checked at compile time), then run it via the sync helpers:

const { kysely, all, get, run } = require('../db/kysely.cts');
const cats = all(
  kysely()
    .selectFrom('categories')
    .select(['id', 'name'])
    .where('user_id', '=', userId)
    .where('deleted_at', 'is', null),
);

Compiled SQL is always parameterized (injection-safe). Prefer this for new/refactored user-scoped queries; extend the Database schema in db/kysely.cts as tables are adopted. Proven byte-identical to raw SQL in tests/kyselyPoc.test.js.

Future / modernization (tracked)

Zod (validation) · node:sqlite (drop the native dep) · ts-rest + OpenAPI · Vite 6/7. Adopt where each makes the code more uniform; see the standardization plan.