104 lines
5.2 KiB
Markdown
104 lines
5.2 KiB
Markdown
# 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.
|
||
- **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 `require`d **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 (target):** every error body is `{ error, message, code, field? }` with
|
||
the correct HTTP status. Throw the factories from `utils/apiError.cts`
|
||
(`ValidationError`, `NotFoundError`, `ConflictError`, …) 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 should not wrap everything in
|
||
`try/catch`. Clients read `message`/`code` + status. **Canonical example:**
|
||
`routes/categories.cts`. (The terminal handler already normalizes any legacy
|
||
`standardizeError`/`{ error }` body to the same shape, so converted and not-yet-converted
|
||
routes emit identical responses — convert opportunistically.)
|
||
- **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 (target)
|
||
|
||
- 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:
|
||
|
||
```ts
|
||
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.
|