job-search/web/CODE_CONVENTIONS.md

# Frontend code conventions

These are the conventions for the frontend in `web/`: a Vite, SolidJS,
Tailwind CSS (v4) and TypeScript app.

TypeScript, ESLint and Prettier check what they can (see
[Tooling](#tooling)); the rest are applied by hand and checked in review.

## Tooling

```bash
npm run check          # typecheck, lint, format:check and test; run before committing
npm run typecheck      # tsc -b: both tsconfigs (plain tsc would check neither)
npm run lint           # eslint .
npm run format:check   # prettier --check .
npm run format         # prettier --write .: formats every file
npm run test           # vitest run: every test, once
npm run test:watch     # vitest: reruns the tests affected by each change
npm run generate:api   # regenerate src/types/api.ts from the backend's OpenAPI description
npm run build          # typecheck, then vite build into dist/
npm run sync           # copy dist/ to ../internal/frontend/dist/, which the backend embeds
npm run dev            # dev server; /api and /ws go to the backend (API_PROXY_TARGET, default localhost:9393)
```

- **TypeScript** runs in `strict` mode, with `noUncheckedIndexedAccess`
  (an indexed value may be `undefined` until checked). `tsconfig.app.json`
  also sets `verbatimModuleSyntax`, `erasableSyntaxOnly`,
  `noUnusedLocals`, `noUnusedParameters` and `noFallthroughCasesInSwitch`:
  in particular, no TypeScript-only runtime syntax such as constructor
  parameter properties or `enum`, and types are imported with
  `import type`. `tsconfig.node.json` checks `vite.config.ts` and
  `scripts/`.
- **ESLint** (`eslint.config.js`): ESLint's and typescript-eslint's
  recommended rules, with type information, so it catches unsafe uses of
  `any` and promises that are dropped; eslint-plugin-solid's rules;
  eslint-comments' rules; simple-import-sort; and the rules below.
- **Prettier** (`.prettierrc.json`) formats everything: see
  [Formatting](#formatting). `eslint-config-prettier` turns off the ESLint
  rules that would disagree with it.
- Generated code (`src/types/api.ts`) isn't linted or formatted, and is
  never edited by hand.

### Linters that check these conventions

| Rule                                                             | Checks                                                                                              |
| ---------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- |
| `simple-import-sort/imports`, `/exports`                         | Imports in two groups, each sorted ([Imports](#imports)); `--fix` sorts them                        |
| `no-restricted-imports`                                          | Imports from another folder use `@/`, not `../` ([Imports](#imports))                               |
| `@typescript-eslint/consistent-type-imports`                     | Types are imported as types, never as values ([Imports](#imports))                                  |
| `@typescript-eslint/naming-convention`                           | camelCase, PascalCase and UPPER_SNAKE_CASE where [Naming](#naming) says; backend keys are exempt    |
| `@typescript-eslint/no-explicit-any`, `no-unsafe-*`              | No `any`, and no untyped values used as if typed ([TypeScript](#typescript))                        |
| `@typescript-eslint/explicit-module-boundary-types`              | Exported functions have their parameter and return types written out ([TypeScript](#typescript))    |
| `@typescript-eslint/no-misused-promises`, `no-floating-promises` | Promises are awaited or handled, including in event handlers ([SolidJS](#solidjs))                  |
| `solid/no-destructure`, `solid/reactivity`                       | Props aren't destructured, and reactive values are read where they can update ([SolidJS](#solidjs)) |
| `solid/prefer-for`                                               | Lists use `<For>` ([SolidJS](#solidjs))                                                             |
| `solid/no-innerhtml`                                             | No `innerHTML` from data ([Security](#security))                                                    |
| `@eslint-community/eslint-comments/require-description`          | Every `eslint-disable` comment says why                                                             |
| `eqeqeq`                                                         | `===` and `!==`, never `==`                                                                         |

ESLint's `no-unassigned-vars` is off for `.tsx` files: Solid assigns refs
itself (`ref={button}`), which ESLint can't see. Any other rule is turned
off only for a line, naming the rule and saying why after `--`:

```ts
// eslint-disable-next-line rule-name -- why the rule is wrong here
```

Most `no-unsafe-*` reports don't need one: give the `any` value a type of
`unknown` and narrow it, as for `JSON.parse`:

```ts
const parsed: unknown = JSON.parse(release.data);
if (isRelease(parsed)) { ... }
```

## Files and folders

| Folder            | Holds                                                                                              |
| ----------------- | -------------------------------------------------------------------------------------------------- |
| `src/api/`        | The only code that talks to the backend: HTTP (`client.ts`) and its WebSocket events (`events.ts`) |
| `src/types/`      | The backend's types (generated, see [Backend types](#backend-types)) and the app's own             |
| `src/primitives/` | Solid primitives: state and loading, no markup (`createX.ts`)                                      |
| `src/components/` | Components, one exported component per file                                                        |
| `src/lib/`        | Plain functions: no Solid, no DOM where avoidable                                                  |
| `scripts/`        | Development scripts run by npm, in TypeScript (`generateApiTypes.ts`, `syncDist.ts`)               |

- Components' files are PascalCase, named after the component
  (`MarkMenu.tsx`). Other files are usually camelCase (`format.ts`); the
  type files' PascalCase names (`Opportunities.ts`) are fine too.
- A file is named after what it exports, exactly: `OpportunityTable.tsx`
  exports `OpportunityTable`. An import must name the file as it is
  spelt on disk.
- The top of `src/` holds only the entry point (`index.tsx`), the app's
  layout (`App.tsx`), the stylesheet (`index.css`) and Vite's type
  declarations (`vite-env.d.ts`).
- `components/styles.ts` holds the Tailwind class strings shared by
  several components (see [Styling](#styling)).

## Imports

Two groups, separated by a blank line: packages (`solid-js`, `node:fs`,
...) first, then the app's own modules, those from other folders (`@/`)
before those from the same folder (`./`). Each group is sorted by module;
a type import comes before the value import from the same module.
simple-import-sort checks this, and `npm run lint -- --fix` sorts them.

```ts
import { createSignal, onCleanup } from 'solid-js';

import { api } from '@/api/client';
import type { OpportunityMark } from '@/types/Annotations';
import { normaliseMarks } from '@/types/Annotations';
import { MARK_BG } from './styles';
```

- An import from another folder of `src` starts with `@/`, which is
  `src/` (set in both `tsconfig.app.json` and `vite.config.ts`):
  `@/api/client`, not `../api/client`. One from the same folder starts
  with `./`. So an import reads the same from every file, and doesn't
  change when the importing file moves. `../` isn't used
  (`no-restricted-imports` reports it).
- Types are imported with `import type`, in their own statement rather
  than inline (`import { api, type Api }`). `consistent-type-imports`
  checks that types are imported as types; the separate statement is
  kept by hand.

## Exports

- Exports are named: `export function MarkMenu`, not
  `export default function`. A named export is imported under one name
  everywhere, so renaming and searching are reliable.
- A default export only where a tool requires one.

## Naming

Names are descriptive: `opportunity`, `classification`, `controller`.
Short names are fine where the meaning is obvious from a few lines
around them: a one-line callback's parameter (`(mark) =>` or
`(m) =>`), an event (`e` or `event`), a caught error (`err`), a plain
index (`i`).

| Kind                                                               | Case                            | Example                                 |
| ------------------------------------------------------------------ | ------------------------------- | --------------------------------------- |
| Variables, functions, props                                        | camelCase                       | `toggleMark`, `opportunityTitle`        |
| Components, types, interfaces                                      | PascalCase                      | `MarkMenu`, `OpportunityAnnotation`     |
| Module-level constants holding fixed data (numbers, lookups, keys) | UPPER_SNAKE_CASE                | `MENU_WIDTH`, `MARK_BG`, `STORAGE_KEYS` |
| Tailwind class strings                                             | camelCase                       | `buttonQuiet`, `fieldLabel`             |
| Primitives                                                         | `create` + PascalCase           | `createOpportunityAnnotations`          |
| Event props                                                        | `on` + PascalCase               | `onToggle`, `onChange`                  |
| Booleans                                                           | `is`, `has`, `can`, `should`... | `isSaving`, `hasDocuments`              |

- A word keeps one case throughout a name: `opportunityTitle`, not
  `OpportunityTitle` for a prop, and `OPPORTUNITY_MARKS`, not
  `Opportunity_MARKS`. A local variable is never PascalCase
  (`opportunities`, not `Opportunities`).
- Initialisms keep one case: `apiBase`, `API_BASE`, `uuid`, `OCDSRelease`.
- Backend field names are kept as the backend sends them (`buyer_name`,
  `has_documents`): see [Backend types](#backend-types). A boolean's
  prefix applies to the app's own names.

## TypeScript

- No `any`. A value of unknown shape is `unknown`, narrowed before use
  (`err instanceof Error`, a type guard such as `isOpportunityMark`).
- A type assertion (`as T`) only where TypeScript can't know a fact the
  code does, such as the shape of `fetch` JSON; never to silence an error.
  Non-null assertions (`!`) only for elements that must exist, such as the
  root element, and Solid refs assigned before use
  (`let button!: HTMLButtonElement`).
- With `noUncheckedIndexedAccess`, a value read by index or key
  (`marks[0]`, `byUuid[uuid]`) may be `undefined`: check it, or give a
  default with `??`.
- Fixed lists are `as const` arrays, with a union type derived from them,
  rather than an `enum` (which `erasableSyntaxOnly` forbids):

  ```ts
  export const OPPORTUNITY_MARKS = ['hidden', 'skipped'] as const;
  export type OpportunityMark = (typeof OPPORTUNITY_MARKS)[number];
  ```

- Object shapes are `interface`s; unions and derived types are `type`s.
- Exported functions have their parameter and return types written out
  (`explicit-module-boundary-types`); others where the type isn't
  obvious.

## Formatting

Prettier formats every file (`npm run format`), so formatting isn't
reviewed. Its settings: two-space indentation, semicolons, single quotes
in TypeScript and double quotes in JSX attributes, trailing commas, and a
100-character line. prettier-plugin-tailwindcss sorts the classes in
`class` strings into Tailwind's order, so the same classes always read the
same way.

Blank lines are still applied by hand: one between a file's top-level
declarations, and between the steps of a long function.

## SolidJS

Solid components run once; only reactive reads (signals, store fields,
`props.x`) update the page. The conventions keep reactivity working:

- **Props are never destructured**, in the parameter list or the body:
  `props.marks`, not `({ marks })`. Destructuring reads the value once and
  stops it updating. Derived values are functions:
  `const marks = () => annotation().marks`.
- Default prop values come from `mergeProps`, and props passed on to a
  child are separated with `splitProps`: both keep the props reactive,
  where destructuring wouldn't.
- A component's props are an `interface Props` in its file. Callbacks are
  `on…` props; components don't reach into shared state they weren't
  given.
- Lists use `<For>`, conditions `<Show>` and `<Switch>`/`<Match>`, not
  `.map()` or `&&` in JSX.
- State lives in primitives (`src/primitives/`): a `createX()` function
  returning an interface of accessors and actions, documented on the
  interface. Components call primitives; primitives don't render.
- Data is loaded with `createResource`, which tracks loading and errors
  and refetches when its source changes. `<Suspense>` shows a placeholder
  while resources load, and an `<ErrorBoundary>` around a part of the
  page shows an error in it instead of breaking the whole page.
- Work started in a primitive or component is cleaned up with
  `onCleanup`: listeners removed, timers cleared, and fetches aborted
  through an `AbortController`. An aborted request isn't reported as an
  error.
- A change may be applied optimistically, before the request, where
  instant feedback matters and the request almost always succeeds (as
  marks do in `createOpportunityAnnotations`). It is then undone if the
  request fails, and the failure shown.
- An event handler doesn't return a promise: Solid ignores it, so a
  rejection would go unnoticed. A handler calls an async function that
  handles its own errors, and marks the call with `void`:
  `onClick={() => void annotations.toggleMark(uuid, mark)}`.

## Styling

- Styles are Tailwind utility classes in `class`. Conditional classes use
  `classList`. No CSS files beyond `src/index.css`.
- Class names are always written out in full in the source: Tailwind only
  generates classes it can see, so never build them at runtime
  (`` `bg-mark-${mark}` ``). A lookup from a value to its classes is a
  record with every class written out, as `MARK_BG` in `styles.ts` is.
- Colours and fonts are theme tokens in the `@theme` block of
  `src/index.css` (`bg-surface`, `text-muted`), not raw values. Dark mode
  swaps the tokens under `.dark`, so components never use `dark:`
  variants.
- Class strings used by more than one component go in
  `components/styles.ts`; ones used by one component are constants in its
  file.

## Backend types

The backend's types are generated from its OpenAPI 3.1 description
(`/api/openapi.json`) into `src/types/api.ts`, by `npm run generate:api`
(`scripts/generateApiTypes.ts`). They're never written by hand (with one
exception, the WebSocket events, below), so they can't drift from the
backend.

- `src/types/Opportunities.ts` gives the generated types the names the
  app uses (`Opportunity`, `OpportunityPage`, `Problem`); code imports
  them from there, not from `api.ts`.
- After a change to the backend's types or handlers, regenerate: run
  `go generate ./...` in the repository, then `npm run generate:api`, and
  commit `api.ts` with the change. With the backend running,
  `npm run generate:api -- http://localhost:9393/api/openapi.json` reads
  the description from it instead.
- Keys are the Go structs' `json` tags, snake_case included
  (`latest_release_date`).
- A field the backend can send as `null` (a Go pointer, or a slice that
  can be nil) is typed `| null`, from the description's `x-nullable`, so
  TypeScript makes the code handle it: a list is read with `?? []`
  (`opportunity.classifications ?? []`).
- Times are RFC 3339 strings (`DateTimeString`), converted to `Date` only
  for display.
- The one exception is the WebSocket events (`src/types/Events.ts`): the
  OpenAPI description only covers HTTP routes, so the events' envelopes are
  written by hand, to match the backend's events, and use the generated
  types for their contents. Messages are parsed as `unknown` and narrowed
  (`src/api/events.ts`); unknown events are ignored.

## API calls

- All requests go through `src/api/client.ts`: one function per
  operation on the exported `api` object, and one `request` helper that
  builds the URL from `VITE_API_BASE_URL` (default `/api`), sets the
  headers and turns a failed response into an `ApiError`.
- Every function that loads data takes an optional `AbortSignal`.
- Path segments taken from data are encoded with `encodeURIComponent`.
- Unsafe requests (PUT, DELETE, POST) send the CSRF token the backend
  issues at `GET /api/csrf`, in the header it names; the client fetches
  and keeps it, so callers don't handle it.

## Errors

- Errors are caught as `unknown` and turned into text with one helper
  (`errorText`), never by assuming they are `Error`s.
- A message shown to the user says what didn't happen, in plain words,
  and then the cause: `Your marks didn't load: …`. Plain or typographic
  quotes and apostrophes are both fine.
- Errors are shown where the user is looking, with `role="alert"`, and can
  be retried or dismissed; they're never only logged to the console.
- An action that failed (a mark that wasn't saved) is shown as a toast in
  the bottom right corner (`createToasts`, `ToastList`), which goes after
  15 seconds or when it's dismissed. A load that failed is shown in place of what didn't
  load, with a way to retry, and as a toast too.

## Security

- Data from the API is shown as text, never as HTML: no `innerHTML`
  with it (`solid/no-innerhtml` reports one). Releases and descriptions
  come from outside sources, so HTML in them could run scripts.
- Only settings that are safe to publish go in `VITE_` variables: Vite
  builds them into the JavaScript every visitor downloads. Nothing
  secret is ever in the frontend.
- `.env` holds local settings and isn't committed; `.env.example`
  lists every variable, with a comment on what it does.

## Formatting values

- Dates, money and numbers are formatted for display by the functions in
  `lib/format.ts`, with `Intl` and a fixed locale (`en-GB`) and currency
  (`GBP`), so they read the same on every machine. Components don't call
  `toLocaleString` or format by hand.
- A missing or zero value (a null date, the zero time) is shown as a dash
  or left empty, not as a misleading default such as `01/01/0001`.

## Comments

- Exported functions, components, primitives, interfaces and their
  non-obvious fields have a `/** … */` comment saying what they are or do.
- `//` comments explain why code is the way it is
  (`// Tailwind only generates classes it can see …`), not what the next
  line does.
- JSX comments use `{/* … */}`.

### Spelling

Comments, documentation and user-facing text use British English
("normalise", "colour", "behaviour"). Names that must match something
external keep its spelling (`Authorization`, CSS `color`).

## Accessibility

- Every interactive element is a real `<button>` (with `type="button"`),
  `<input>`, `<label>` or link, so it works with a keyboard.
- Controls without visible text have an `aria-label`; one whose text is
  ambiguous in a table row includes what it belongs to ("Marks for
  <title>").
- Menus and popups set `aria-expanded` and `aria-controls`, close on
  Escape, and return focus to the control that opened them.
- Status that changes on its own (loading, saving, errors) uses
  `role="alert"`, `aria-busy` or a live region.

## Browser storage

- `localStorage` is only used through `lib/storage.ts`
  (`readStored`, `writeStored`), which never throws: storage can be
  disabled or full, and a preference then just isn't kept.
- Keys are listed in `STORAGE_KEYS`, prefixed with the app's name.
- Only preferences are stored (theme, panel state, filters); data comes
  from the API every time.

## Dependencies

- `package-lock.json` is committed, and installs use `npm ci`, so every
  machine gets the same versions.
- Development tools (linters, formatters, generators) are pinned to exact
  versions, as a new version can report new problems or generate
  different code. The app's own dependencies use `^` ranges, held by the
  lock file.
- A new or upgraded package is a release at least two weeks old, and is
  added deliberately, in the same change as the code that needs it.

## Tests

- Tests use Vitest, which shares Vite's configuration (the `test` block
  of `vite.config.ts`), and sit next to the file they test as
  `name.test.ts`. `npm run check` runs them.
- They run in Node, not a browser: Solid's reactivity works there, but
  there is no DOM. Browser APIs the code uses (`fetch`, `WebSocket`,
  `window.location`) are replaced with fakes for the test, with
  `vi.stubGlobal`, and restored after it.
- They start with the code in `src/lib/` and `src/api/`, which has no UI
  to render, then the primitives.
- When components are tested, it's with `@solidjs/testing-library` (and
  a DOM such as jsdom), through what the user sees and does (text,
  roles, clicks), not their internals.
- No ESLint plugin checks accessibility in Solid well, so component tests
  run an accessibility check (`axe`) on what they render.