331 lines
14 KiB
Markdown
331 lines
14 KiB
Markdown
# Coding Conventions
|
||
|
||
**Analysis Date:** 2026-06-09
|
||
|
||
## Naming Patterns
|
||
|
||
**Files:**
|
||
- Backend route handlers: `camelCase.ts` — `events.ts`, `me.ts`, `health.ts` (`apps/api/src/routes/`)
|
||
- Broker modules: `camelCase.ts` — `poller.ts`, `sync.ts`, `write.ts`, `expand.ts` (`apps/api/src/broker/`)
|
||
- Frontend components: `PascalCase.tsx` — `EventForm.tsx`, `CalendarShell.tsx`, `InstallPrompt.tsx` (`apps/pwa/src/components/`)
|
||
- Frontend utilities: `camelCase.ts` — `colorUtils.ts`, `hydrateEvents.ts`, `eventDateTime.ts`, `loginRedirect.ts` (`apps/pwa/src/lib/`)
|
||
- Tests: `{filename}.test.ts` or `.test.tsx` co-located with source
|
||
|
||
**Functions:**
|
||
- Private helpers (not exported): `camelCase` — `claimStr()`, `getBreakpointGroup()`, `viewStorageKey()`, `resolveUserId()`
|
||
- Exported async handlers: `camelCase` — `fetchMe()`, `createEvent()`, `expandOccurrences()`, `upsertUser()`
|
||
- React hooks (Zustand): `useCalendarStore`, `useXxxx` pattern — follows React convention
|
||
- Type guard / coercion functions: `camelCase` — `deriveDisplayName()`, `claimStr()`
|
||
|
||
**Variables:**
|
||
- Constants (module-level): `SCREAMING_SNAKE_CASE` — `MAX_WINDOW_DAYS`, `SHARED_FAMILY_COLOR`, `COLOR_PALETTE`, `FIXTURES`
|
||
- Local state: `camelCase` — `currentUserId`, `targetCalendarUrl`, `windowStartDate`, `eventRow`
|
||
- Zustand store methods: `camelCase` setters — `setSelectedView()`, `setEventForm()`, `setLastSyncedUid()`
|
||
- Store state keys: `camelCase` — `selectedView`, `openEventId`, `eventFormOpen`, `deleteDialogUid`
|
||
- Destructured auth claims: `camelCase` — `iss`, `sub`, `email`, `displayName`
|
||
- Database column mappings: `snake_case` in schema → `camelCase` in TypeScript (Drizzle handles mapping)
|
||
|
||
**Types/Interfaces:**
|
||
- TypeScript interfaces: `PascalCase` — `MeUser`, `MeResponse`, `CalendarOccurrence`, `WritableCalendar`, `CalendarStore`, `SyncStatus`
|
||
- Zod schemas: `camelCase` + `Schema` suffix — `eventsQuerySchema`, `eventFieldsSchema`, `syncStatusQuerySchema`
|
||
- Union types (enums): `PascalCase` or quoted literals in types — `'create' | 'update' | 'delete'`, `'pending' | 'done' | 'failed' | 'dead'`
|
||
- Database table names: `snake_case` — `calendar_events`, `calendar_outbox`, `member_credentials`
|
||
- DB column names: `snake_case` — `dtstart_utc`, `dtstart_date`, `oidc_iss`, `oidc_sub`
|
||
|
||
**Drizzle ORM tables:**
|
||
- Table function: `mysqlTable('table_name', {...})`
|
||
- Column names in schema def: use snake_case strings — `int('user_id')`, `varchar('oidc_iss', ...)`
|
||
- TypeScript field names (destructured queries): auto-convert to camelCase via Drizzle's default mode
|
||
- Primary keys: `id: int().primaryKey().autoincrement()` (all tables follow this)
|
||
- Foreign keys: `references(() => targetTable.id, { onDelete: 'cascade' })` (explicit cascade behavior)
|
||
- Indexes: named with `idx_` prefix — `idx_calendar_events_dtstart_utc`, `idx_outbox_user_status`
|
||
- Unique constraints: named with `uniq_` prefix — `uniq_oidc_identity`, `uniq_calendar_uid`, `uniq_calendar_user_url`
|
||
|
||
## Code Style
|
||
|
||
**Formatting:**
|
||
- No explicit ESLint or Prettier config files in the codebase (uses project defaults)
|
||
- 2-space indentation (inferred from source code)
|
||
- Single quotes for strings (`'string'`, not `"string"`)
|
||
- Semicolons at end of statements
|
||
- No trailing commas in function calls; trailing commas in object/array literals (modern style)
|
||
|
||
**Linting:**
|
||
- TypeScript: `strict: true` in both backend and frontend `tsconfig.json`
|
||
- Module resolution: `NodeNext` (backend), `Bundler` (frontend)
|
||
- No `any` types — use `Context` from Hono where typing is available
|
||
|
||
**Example formatting (from `routes/events.ts` line 64):**
|
||
```typescript
|
||
async function resolveUserId(c: Context): Promise<number | null> {
|
||
const devUser = c.get('user') as { id: number } | undefined
|
||
if (devUser) return devUser.id
|
||
|
||
const auth = await getAuth(c)
|
||
if (!auth) return null
|
||
|
||
const iss = (auth.iss as string | undefined) ?? ''
|
||
const sub = auth.sub ?? ''
|
||
// ...
|
||
}
|
||
```
|
||
|
||
## Import Organization
|
||
|
||
**Order:**
|
||
1. Node.js built-ins (`import { ... } from 'node:...'`)
|
||
2. Third-party packages (`import { ... } from 'hono'`, `import { ... } from 'drizzle-orm'`)
|
||
3. Local absolute imports (backend: none; frontend: none visible — no path aliases configured)
|
||
4. Local relative imports (`import { ... } from '../dir/file.js'` or `../../...`)
|
||
5. Side-effect imports (import without destructuring, placed last) — `import '../auth/devBypass.js'`
|
||
|
||
**Path extensions:**
|
||
- All imports use explicit `.js` extensions — `from './index.js'`, `from '../db/client.js'`
|
||
- Applies to both backend and frontend (ESM module resolution)
|
||
|
||
**Example (from `routes/events.ts` lines 24–38):**
|
||
```typescript
|
||
import { randomUUID } from 'node:crypto' // Node.js built-in
|
||
import { Hono } from 'hono' // Third-party
|
||
import type { Context } from 'hono'
|
||
import { zValidator } from '@hono/zod-validator' // Third-party (Hono ecosystem)
|
||
import { z } from 'zod'
|
||
import { and, or, eq, desc } from 'drizzle-orm'
|
||
import { sql } from 'drizzle-orm'
|
||
import { db } from '../db/client.js' // Relative local import
|
||
import { calendarEvents, calendars, ... } from '../db/schema.js'
|
||
import { expandOccurrences } from '../broker/expand.js'
|
||
import { getAuth } from '../auth/middleware.js'
|
||
import { upsertUser, deriveDisplayName } from '../auth/user.js'
|
||
import '../auth/devBypass.js' // Side-effect import (last)
|
||
```
|
||
|
||
## Error Handling
|
||
|
||
**Patterns:**
|
||
|
||
**Backend (Hono routes):**
|
||
- Early return with typed `c.json(...)` on validation or auth failure — `return c.json({ error: 'message' }, statusCode)`
|
||
- Try-catch blocks wrap DB/external I/O, catch logs error + returns 503 Service Unavailable
|
||
- No unhandled rejections — every async operation has explicit error handling
|
||
- Auth failures: return 401 Unauthorized; authorization failures: return 403 Forbidden; missing resource: return 404
|
||
- Validation failures: return 400 Bad Request with error envelope
|
||
|
||
**Example (from `routes/events.ts` lines 126–225):**
|
||
```typescript
|
||
eventsRouter.get('/', zValidator('query', eventsQuerySchema), async (c) => {
|
||
const currentUserId = await resolveUserId(c)
|
||
if (currentUserId === null) return c.json({ error: 'Unauthorized' }, 401)
|
||
|
||
const { start, end } = c.req.valid('query')
|
||
|
||
const spanDays = (windowEndDate.getTime() - windowStartDate.getTime()) / (1000 * 60 * 60 * 24)
|
||
if (spanDays > MAX_WINDOW_DAYS || spanDays <= 0) {
|
||
return c.json({ error: 'Date window must be between 1 and 90 days' }, 400)
|
||
}
|
||
|
||
try {
|
||
const rows = await db.select(...).from(...).where(...)
|
||
const allOccurrences = rows.flatMap((row) => expandOccurrences(...))
|
||
return c.json({ occurrences: allOccurrences })
|
||
} catch (err) {
|
||
console.error('[events] DB query or expansion failed:', err)
|
||
return c.json({ error: 'Service unavailable' }, 503)
|
||
}
|
||
})
|
||
```
|
||
|
||
**Frontend (React + TanStack Query):**
|
||
- Fetch client throws on non-ok response; caller handles redirect logic (`maybeRedirectToLogin()`)
|
||
- API client checks `res.type === 'opaqueredirect'` and `res.status === 401` to detect auth failure (CORS-safe 302 handling)
|
||
- Component state via Zustand; server state via React Query
|
||
- No inline try-catch in components — defer to query error states
|
||
|
||
**Example (from `api/client.ts` lines 28–53):**
|
||
```typescript
|
||
export async function fetchMe(): Promise<MeResponse> {
|
||
const res = await fetch('/api/me', {
|
||
credentials: 'include',
|
||
redirect: 'manual',
|
||
})
|
||
|
||
if (res.type === 'opaqueredirect' || res.status === 401) {
|
||
throw new Error('GET /api/me: authentication required')
|
||
}
|
||
|
||
if (!res.ok) {
|
||
throw new Error(`GET /api/me failed: ${res.status}`)
|
||
}
|
||
|
||
return res.json() as Promise<MeResponse>
|
||
}
|
||
```
|
||
|
||
## Logging
|
||
|
||
**Framework:** Console methods only (`console.log`, `console.error`, `console.warn`)
|
||
|
||
**Patterns:**
|
||
- Errors logged with context prefix in square brackets — `console.error('[events]', message)`, `console.error('[broker/sync]', message)`
|
||
- Startup messages logged at info level — `console.log('FamilySync API running on ...')`
|
||
- Dev-mode warnings prefixed with warning emoji-ish symbol — `console.warn('⚠ DEV_AUTH_BYPASS active ...')`
|
||
- No structured logging (JSON); plain text OK for small household app
|
||
- Errors include the full exception object for stack trace — `console.error('[events] DB query failed:', err)`
|
||
|
||
**Example (from `index.ts` lines 23, 111):**
|
||
```typescript
|
||
if (devBypassActive) {
|
||
console.warn('⚠ DEV_AUTH_BYPASS active — OIDC guard DISABLED. Never use in production.')
|
||
}
|
||
// ...
|
||
serve({ fetch: app.fetch, port: 3000 }, (info) => {
|
||
console.log(`FamilySync API running on http://localhost:${info.port}`)
|
||
})
|
||
```
|
||
|
||
## Comments
|
||
|
||
**When to Comment:**
|
||
- Complex algorithms or non-obvious business logic — e.g., window date filtering in `routes/events.ts` (lines 142–151)
|
||
- Security assertions or threat-model references — e.g., ownership checks (T-03-06), CSRF-token patterns
|
||
- Architectural invariants — e.g., "broker boundary: this route reads ONLY from cache" (routes/events.ts:4)
|
||
- Non-standard patterns — e.g., `isMainModule()` check to gate cron startup (index.ts:81–99)
|
||
- Workarounds and why they exist — e.g., "WR-04: carrier/groupId for edit-as-move txn" (routes/events.ts:373)
|
||
|
||
**JSDoc/TSDoc:**
|
||
- Used for public exported functions, not for every function
|
||
- Single-line for simple functions; multi-line with `@param` and `@returns` for complex signatures
|
||
- Comments on types (interfaces) to document contract — e.g., `CalendarOccurrence` interface (api/client.ts:71–87)
|
||
|
||
**Example (from `auth/user.ts` lines 25–32):**
|
||
```typescript
|
||
/**
|
||
* Accessible, visually-distinct palette for per-member color assignment.
|
||
* A new member is given the first entry not already in use (see upsertUser).
|
||
*
|
||
* Ordering matters: the shared-family calendar is reserved rose (#F25C7A, D-06),
|
||
* so the warm near-rose hues (coral, amber) are placed LAST. Early members get
|
||
* cool colors (blue, green, teal) that read clearly distinct from the shared
|
||
* lane — otherwise a member's coral was mistaken for the shared rose.
|
||
* Values are Claude's choice per D-06.
|
||
*/
|
||
export const COLOR_PALETTE: string[] = [...]
|
||
```
|
||
|
||
## Function Design
|
||
|
||
**Size:** Prefer short, single-responsibility functions. Route handlers are the exception — they bundle validation, ownership check, and response assembly (pragmatism for Hono idiom).
|
||
|
||
**Parameters:**
|
||
- Use Hono's `Context` type rather than destructuring everything — `async (c: Context)`
|
||
- Explicit parameters for helper functions; Hono context passed implicitly where possible
|
||
- Zod validators return typed objects via `c.req.valid('json')` or `c.req.valid('query')`
|
||
|
||
**Return Values:**
|
||
- Async functions return typed values or throw — `Promise<T>` or `Promise<void>`
|
||
- Error responses returned explicitly (not thrown) — callers handle 4xx/5xx in same try-catch
|
||
- Database queries return typed Drizzle result objects; destructure as needed
|
||
|
||
**Example (from `auth/user.ts` lines 79–142):**
|
||
```typescript
|
||
export async function upsertUser(
|
||
oidcIss: string,
|
||
oidcSub: string,
|
||
displayName?: string | null,
|
||
) {
|
||
// 1. Look up by composite identity key...
|
||
const existing = await db.select().from(users).where(...).limit(1)
|
||
if (existing[0]) {
|
||
// Update displayName if changed
|
||
if (displayName != null && displayName !== existing[0].displayName) {
|
||
await db.update(users).set({ displayName }).where(...)
|
||
return { ...existing[0], displayName }
|
||
}
|
||
return existing[0]
|
||
}
|
||
|
||
// 2. Assign color from palette...
|
||
// 3. Insert new row...
|
||
// 4. Re-select and return
|
||
}
|
||
```
|
||
|
||
## Module Design
|
||
|
||
**Exports:**
|
||
- Named exports for functions and types — `export const TABLE`, `export function handler()`, `export interface Type`
|
||
- No default exports (exception: SPA app shell `App.tsx` uses default export)
|
||
- Re-export from middleware modules for convenience — `auth/middleware.ts` re-exports `@hono/oidc-auth` functions
|
||
|
||
**Barrel Files:**
|
||
- No wildcard re-exports (`export * from ...`) — explicit named exports only
|
||
- Top-level index files not used (each module imported directly)
|
||
|
||
**Example (from `auth/middleware.ts` lines 24–26):**
|
||
```typescript
|
||
export { oidcAuthMiddleware, processOAuthCallback, getAuth } from '@hono/oidc-auth'
|
||
```
|
||
|
||
## Database Patterns
|
||
|
||
**Drizzle conventions (critical):**
|
||
- Schema definition: `mysqlTable('name', { id: int().primaryKey().autoincrement(), ... }, (t) => [...])`
|
||
- Foreign keys: ALWAYS include `{ onDelete: 'cascade' }` to propagate deletes cleanly
|
||
- Indexes: Explicit index names with `idx_` prefix on frequently filtered columns
|
||
- Unique constraints: Explicit unique names with `uniq_` prefix on identity/natural keys
|
||
- Never use `db:push` on populated MariaDB (false destructive diffs) — ALWAYS use `generate + migrate`
|
||
|
||
**Query patterns:**
|
||
- Use Drizzle's type-safe query builder: `db.select(...).from(table).where(...).limit(...)`
|
||
- Raw SQL via `` sql`...` `` for complex predicates (e.g., multi-condition OR chains in events.ts:167–201)
|
||
- Parameterized values via `sql` template tag prevent SQL injection
|
||
- Joins: explicitly `innerJoin()` or `leftJoin()` with `.on(eq(...))` conditions
|
||
|
||
**Example (from `db/schema.ts` lines 96–123):**
|
||
```typescript
|
||
export const calendarEvents = mysqlTable(
|
||
'calendar_events',
|
||
{
|
||
id: int().primaryKey().autoincrement(),
|
||
calendarId: int('calendar_id')
|
||
.notNull()
|
||
.references(() => calendars.id, { onDelete: 'cascade' }),
|
||
uid: varchar('uid', { length: 512 }).notNull(),
|
||
// ... more columns
|
||
},
|
||
(t) => [
|
||
index('idx_calendar_events_dtstart_utc').on(t.dtstartUtc),
|
||
index('idx_calendar_events_has_rrule').on(t.hasRrule),
|
||
unique('uniq_calendar_uid').on(t.calendarId, t.uid),
|
||
],
|
||
)
|
||
```
|
||
|
||
## Reactive State (Frontend)
|
||
|
||
**TanStack Query (Server State):**
|
||
- All calendar events, lists, user profile live in React Query
|
||
- Queries keyed by API endpoint + windowing params — `['events', { start, end }]`
|
||
- Mutations handle POST/PATCH/DELETE; invalidate cache on success
|
||
- Use `useQuery` for reads, `useMutation` for writes; never mix server state into Zustand
|
||
|
||
**Zustand (UI State):**
|
||
- Owns only UI-shape state: `selectedView`, `openEventId`, `eventFormOpen`, `deleteDialogOpen`, etc.
|
||
- Persists breakpoint-scoped `selectedView` to `localStorage`
|
||
- Never store server data (user profile, events) — keep it in React Query
|
||
- Setters are synchronous; no side effects (except localStorage in `setSelectedView`)
|
||
|
||
**Example (from `store/calendarStore.ts` lines 1–25):**
|
||
```typescript
|
||
/**
|
||
* Zustand UI-state store for the calendar shell.
|
||
*
|
||
* Owns ONLY UI-shape state — no server data ever enters this store.
|
||
* Server state (events, user profile) lives in TanStack Query.
|
||
*/
|
||
```
|
||
|
||
---
|
||
|
||
*Convention analysis: 2026-06-09*
|