Skip to content

code-style.md

Latest commit

 

History

History
326 lines (246 loc) · 15.6 KB

code-style.md

File metadata and controls

326 lines (246 loc) · 15.6 KB

Code Style

Conventions for TypeScript code in src/ and tests/. Enforced by biome where mechanical, by review where judgment-call.

Prefer functional array methods

// Avoid
for (let i = 0; i < items.length; i++) { ... }
for (const item of items) { ... }

// Prefer
items.map(item => …)
items.filter(item => …)
items.reduce((acc, item) => …, initial)
items.find(item => …)
items.flatMap(item => …)

Exceptions: when traversal needs to mutate an external accumulator with branching control flow that doesn't compose cleanly with reduce, a for…of loop is fine. Prefer the functional form when both work.

Discriminated unions over open string types

// Avoid — `string` collapses with `'arrow'` and breaks narrowing
type Drawing = { kind: 'arrow' | string; ... }

// Prefer — discriminator is a closed literal union; open detail goes elsewhere
type Drawing =
  | { kind: 'arrow'; from: [number, number]; to: [number, number] }
  | { kind: 'geo'; geo: 'rectangle' | 'ellipse' | string; bbox: [number, number, number, number]; ... }

Narrow unknown over casting any

any opts out of typechecking entirely. unknown forces narrowing at the use site, which catches drift when a third-party schema changes shape.

// Avoid
function getStore(snapshot: any): Record<string, any> | null { ... }

// Prefer
function getStore(snapshot: unknown): Record<string, unknown> | null {
  if (!snapshot || typeof snapshot !== 'object') return null;
  const s = snapshot as { document?: { store?: unknown }; store?: unknown };
  if (s.document?.store && typeof s.document.store === 'object') {
    return s.document.store as Record<string, unknown>;
  }
  ...
}

When a legitimate any is necessary (untyped third-party schema, non-standard DOM API), document it inline:

// biome-ignore lint/suspicious/noExplicitAny: caretRangeFromPoint is non-standard but supported in headless Chromium
const range = (document as any).caretRangeFromPoint?.(x, y);

Inline styles use CSS variables, never literal colours

// Avoid
<div style={{ color: '#5b6cff', borderRadius: '8px', padding: '14px 28px' }}>

// Prefer
<div style={{
  color: 'var(--accent)',
  borderRadius: 'var(--radius-md)',
  padding: 'var(--space-sm) var(--space-md)',
}}>

If the value isn't a token, the token is missing. Add it to src/styles/tokens.css first, then use it.

Auth check at the top of every route

export async function GET(req: Request, ctx: { params: Promise<{ id: string }> }) {
  const ident = await identify(req);
  if (!ident) return NextResponse.json({ error: 'unauthorized' }, { status: 401 });
  // …
}

identify is the single source of truth — see docs/api/auth.md. Never reimplement the cookie / Bearer parsing locally.

The only routes that skip this check are the three documented public surfaces (GET /api/health, POST /api/auth/login, POST /api/auth/setup) — see docs/api/INDEX.md. New public surfaces must be added to that table and carry an inline comment explaining why no identity is required.

Error responses follow the snake_case-code shape

return NextResponse.json({ error: 'invalid_body' }, { status: 400 });
return NextResponse.json({ error: 'patch_conflict', file: 'index.html' }, { status: 409 });

The error field is the machine-readable identifier in snake_case. Optional fields carry context. Clients match on error, never on a human-readable message.

Service functions return data, throw on bug-class errors

Services in src/lib/<surface>/service.ts are the business-logic layer. They:

  • Take an input object (named keys, no positional args)
  • Return the persisted record(s) on success
  • Return null for "not found" cases (route handler maps to 404)
  • Throw on programmer errors (invariants violated, unreachable branches)
export async function updateAnnotationTldraw(id: string, snapshot: unknown) {
  const annotation = await prisma.annotation.findUnique({ where: { id } });
  if (!annotation) return null;          // → route returns 404
  // … happy path writes the file and returns the row
  return annotation;
}

For typed errors, use a class that the route can instanceof-check:

export class DiffApplyError extends Error {
  constructor(public readonly reason: 'conflict' | 'malformed') {
    super(`diff_apply_failed:${reason}`);
    this.name = 'DiffApplyError';
  }
}

File and folder names

  • Routes under src/app/api/<surface>/[…]/route.ts follow Next.js App Router conventions (route.ts, dynamic segments in brackets)
  • Components live under src/components/<ComponentName>/<ComponentName>.tsx — the folder hosts the component file, optional CSS, and tests
  • Lib helpers under src/lib/<surface>/<name>.ts — small focused files, one responsibility per file
  • Tests under tests/<unit|integration>/<surface>/<name>.test.ts mirror the file under test where practical

Comments

Default to writing none. Add a comment only when the why is non-obvious — a hidden constraint, a workaround for a specific bug, a choice that would surprise a reader. Don't comment what the code already says.

// Avoid
const tldrawAbs = path.join(env().DATA_DIR, annotation.tldrawPath); // build absolute path

// Prefer (comment carries non-obvious intent)
// Invalidate intent sidecar BEFORE writing the new tldraw — readers that
// see the new mtime should never get a stale intent.json with the old key.
deleteIntentCache(annDir);

Tooltips: one primitive, no exceptions

The tooltip system is a single <div popover="hint" id="markup-tooltip"> rendered once in src/app/layout.tsx via TooltipPortal (see src/components/Tooltip/TooltipPortal.tsx). A capture-phase document listener watches for mouseenter / focusin on [data-tooltip] triggers, copies the text into the popover, positions it against the trigger's getBoundingClientRect, and calls showPopover(). popover="hint" paints the element in the browser's top-layer — position: fixed against the viewport — so the tooltip escapes every overflow ancestor and stacking context. No portal bookkeeping, no z-index fight, no clipping inside .rail .list { overflow-y: auto }.

Every interactive surface that needs a hover/focus hint MUST use the data-tooltip="text" attribute. The CanvasToolbar's Zoom in / Zoom out / Fullscreen buttons are the reference; rail tooltips, kebab tooltips, comment kebab tooltips, the emoji-picker Add reaction tooltip — all route through the same TooltipPortal.

Forbidden:

  • title="…" attributes on buttons, links, or any interactive role (use data-tooltip instead; keep aria-label for accessibility). See the overflow-disclosure exception below.
  • JSX tooltip components (no <Tooltip>…</Tooltip> wrappers, no portal-based one-off implementations).
  • aria-describedby paired with a visually-hidden element used as a tooltip.

Required:

<button
  type="button"
  data-tooltip="Keep expanded"
  // For triggers near the right edge of the viewport / a container,
  // anchor the bubble to the trigger's right edge so it extends left
  // and never overflows.
  data-tooltip-align="right"
  aria-label="Keep expanded"
>
  …
</button>

The trigger doesn't need any positioning context — the popover lives in the top-layer and positions itself against the trigger's getBoundingClientRect. Browser support: Chrome 114+, Safari 17+, Firefox 125+.

title= overflow-disclosure exception

title="…" is permitted on non-interactive display elements when the visible text is truncated by text-overflow: ellipsis. The attribute then serves as the overflow-disclosure primitive: screen readers announce the full string, mouse users get the native tooltip on hover, and keyboard users that tab to the element (when it is focusable) see the same. data-tooltip is the wrong tool here — it is for interactive hints on triggers, not for revealing clipped content.

Conditions for the exception:

  • The element is not a button, link, menuitem, or any interactive role. <h2>, <div>, <span>, <p>, <li> are typical hosts.
  • The same element (or a child) renders the user-supplied text that may overflow.
  • The CSS on the same element produces the overflow — text-overflow: ellipsis, -webkit-line-clamp, overflow: hidden etc.
  • The title value is the same string that is being clipped.

Reference call sites (kept by exception):

  • src/components/ProjectCard/ProjectCard.tsx — <h2 title={project.name}>
  • src/components/ProjectTree/TreeNode.tsx — <div title={displayLabel}>
  • src/components/ProjectTree/ProjectTree.tsx — <div title={m.name}> on mockup-row labels
  • src/app/mockups/MockupCard.tsx — <div className={styles.subtitle} title={subtitle}>

Inline comment shorthand at each site (reference, don't paraphrase):

// title= is the overflow-disclosure exception — see docs/code-style.md

Adding new exception sites: the comment above is sufficient. No ADR required; the rule is bounded and self-checking (the same string must be both clipped and used as title).

Iframe title attribute (in MockupViewer/ViewerCanvas.tsx, mockups/[id]/diff/DiffViewer.tsx) is a separate WCAG-required accessible name for the iframe — not covered by this exception, but also not forbidden by the rule because it is not a tooltip.

Never use native browser dialogs

window.alert(), window.confirm(), and window.prompt() are banned in the codebase. They:

  • Ignore every design token (background, typography, spacing, motion); the user sees a chrome of the OS instead of the product.
  • Block the renderer on a synchronous event loop, which collides with automation (QA agents, Playwright) and assistive tech.
  • Hijack focus on touchscreens, often firing twice or behind the page chrome.

Always use the project's useConfirm() hook from @/components/ConfirmDialog for yes/no flows. The hook returns a promise — same imperative ergonomics as confirm(), styled like the rest of the floating chrome.

// Avoid
if (!window.confirm('Delete this comment?')) return;

// Prefer
const ok = await confirm({
  title: 'Delete comment',
  description: 'This cannot be undone.',
  confirmLabel: 'Delete',
  danger: true,
});
if (!ok) return;

For one-off messages (e.g. surfacing an API error), call confirm() with a single button + a cancel label of "Dismiss" — same component, no extra primitive. For multi-step flows or forms, build on top of @radix-ui/react-dialog and host the new component under src/components/<Name>Dialog/.

Glass surfaces: floatingSurface / floatingScrim, no exceptions

Every component that paints above other content — dialogs, popovers, tooltips, alerts, toasts, the rail, the canvas toolbar, the marking bar, the annotation composer, the sidebar, the command palette, every custom popup — must adopt the canonical glass treatment by composing from src/styles/glass.module.css. The utility ships two classes:

  • floatingSurface — sets background: var(--surface-glass-bg), backdrop-filter: blur(16px) saturate(140%), border: var(--surface-glass-border). Use on the floating card / panel / popover itself.
  • floatingScrim — sets background: var(--scrim-glass-bg), backdrop-filter: blur(16px) saturate(140%). Use on the dimming scrim under a modal.

Required pattern in any CSS Module that paints a floating overlay:

.myDialog {
  composes: floatingSurface from '../../styles/glass.module.css';
  /* …layout, padding, border-radius, box-shadow, z-index… */
}

Forbidden:

  • Re-declaring background, backdrop-filter, and border inline on a floating component. The utility owns these properties; the component owns position / size / radius / shadow.
  • Writing -webkit-backdrop-filter manually next to backdrop-filter. Lightning CSS de-dupes the pair and silently keeps only the prefixed form — Chrome then ignores it. Declare only the unprefixed property; the build tool auto-prefixes.
  • Using var() inside backdrop-filter. Lightning CSS strips that declaration too. The utility writes the value literally.

The tooltip (src/components/Tooltip/Tooltip.css) is the only floating element not composed from the utility — it's a global CSS file because popover="hint" lives at the document root. It declares the same property block inline by exception; keep its values in lockstep with the utility.

See docs/feature-catalog.md § glass-surface-standard.

Popovers: usePopover, no exceptions

Every popover (kebab menu, version chip, emoji picker, account menu, anchored option list, anything that floats next to a trigger) MUST use the usePopover hook from src/lib/popover/usePopover.ts. The hook wraps the native HTML popover API (popover="auto" + popovertarget) and pairs it with the project's position calculator (src/lib/popover/position.ts). The popover paints in the browser's top-layer — the same guarantee TooltipPortal uses — so it escapes every overflow ancestor and stacking context. The browser owns light-dismiss, ESC-to-close, and the single-active invariant.

The reference implementations are Comment.tsx (kebab), AnnotationCard.tsx (primary kebab + status group), VersionChip.tsx (nested popover for the per-row Promote/Delete menu), EmojiPicker.tsx, Topbar.tsx (account menu), and ProjectTree.tsx's TreeNodeKebab subcomponent.

Forbidden:

  • useState(menuOpen) + document.addEventListener('mousedown', …) outside-click effects. The browser already does this for popover="auto".
  • Generic <Dropdown> / <Menu> JSX wrappers. The hook is the primitive — there's no extra layer.
  • position: absolute on the popover element. The popover must position: fixed so the JS-written top/left win over the browser default inset: 0; margin: auto.

Required pattern:

const menu = usePopover<HTMLButtonElement, HTMLDivElement>('right');
return (
  <>
    <button
      ref={menu.triggerRef}
      type="button"
      data-tooltip="Open menu"
      aria-haspopup="menu"
      {...menu.triggerProps}
    >
      ⋮
    </button>
    <div {...menu.popoverProps} className={styles.menu} role="menu">
      <button
        role="menuitem"
        onClick={() => {
          menu.close();
          doAction();
        }}
      >
        Action
      </button>
    </div>
  </>
);

Required CSS:

.menu {
  position: fixed;
  inset: auto;
  margin: 0;
  display: none;
  flex-direction: column;
  /* …glass styling… */
}
.menu:popover-open {
  display: flex;
}

Nested popovers stack natively: opening one popover from inside another keeps the parent open via the HTML popover spec's ancestor relationship (VersionChip's per-row kebab demonstrates this). Each row that needs its own popover gets its own usePopover call — extract a subcomponent so the hook isn't called inside a loop. Browser support: Chrome 114+, Safari 17+, Firefox 125+ (same baseline as data-tooltip).

Prefer pure helpers over service methods on classes

Services are functions that take input and return output. Avoid classes with state — they hide control flow and break tree-shaking. The DiffApplyError class is an exception because it composes with instanceof.

Logging

Use logger from src/lib/logger.ts — structured pino with named children. Don't use console.log in src/; the lint rule will allow it in scripts/ since those are one-shot tools.

import { logger } from '@/lib/logger';
const log = logger.child({ name: 'mockup-service' });
log.info({ mockupId, versionId }, 'version_created');

The first argument is structured fields; the second is the human-readable message (a snake_case event name keeps logs greppable).