logos-web/docs/code-quality-followups.md

# Code-Quality Follow-ups

This document tracks items that were identified during the code-quality pass
but need design or infrastructure decisions before code can land. Each
section spells out the gap, the proposed fix, and the artifacts the
implementer needs.

---

## i18n: single-locale today, multi-locale infra in place

### Status

Configured for English only.

- `apps/web/i18n/routing.ts` declares `locales: ['en']`.
- Only `apps/web/messages/en.json` exists.
- `packages/content/src/locales/registry.ts` defaults to `['en']` until the
  app calls `setActiveLocales` with more.
- `apps/web/scripts/strip-default-locale-prefix.sh` runs after `next build`
  to strip the `/en` prefix from the static-export output (single-locale
  optimization).

### When adding `fr` / `ko` / etc

1. Add the locale code to `apps/web/i18n/routing.ts` `locales` array.
2. Create `apps/web/messages/<locale>.json` mirroring the `en.json` keys.
3. Boot the registry at startup with the new list (the next-intl provider
   already does this for the `routing.locales` array).
4. For each PageCopy-driven route, create `<locale>.json` files alongside the
   existing English copy under `content/pages/<route>/<locale>.json`.
5. Re-run the build; `strip-default-locale-prefix.sh` continues to strip
   the default locale (`en`) prefix only.

### Why not yet

No translated content shipped. Locking single-locale wiring keeps the
codebase honest until that arrives.

---

## `knip` dead-export audit

### Status

Open. 102 `export` keywords appear under `apps/web/components/`. Likely some
are unused after the previous refactors (sub-component extractions,
re-exports from barrel files).

### Run path

```bash
pnpm dlx knip --workspace apps/web
```

A typical first-run baseline:

- Unused exports flagged → review one batch per feature area; some are
  intentional public API for sibling files in the same barrel and should be
  marked with `// @public` or moved into a private file.
- Unused dependencies flagged → confirm in `package.json` before removing.

### Why not yet

Running knip casually creates noise; the right pass is once the structural
refactors settle, then bake `knip --no-progress` into CI as a warn-only step.

---

## Bundle analyzer

### Status

Open. Heavy dependencies (`motion`, `leaflet`, `react-leaflet-cluster`) ship
to clients but actual cost is unmeasured.

### Wiring (when ready)

```bash
pnpm add -D @next/bundle-analyzer --filter web
```

Then in `next.config.mjs`:

```js
import withBundleAnalyzer from '@next/bundle-analyzer'
const analyzer = withBundleAnalyzer({ enabled: process.env.ANALYZE === 'true' })
export default withNextIntl(analyzer(nextConfig))
```

`pnpm turbo run build --filter=web` with `ANALYZE=true` emits HTML reports under
`.next/analyze/`.

### Why not yet

devDep install needs explicit user approval; once installed, also worth
adding a CI bundle-size budget (e.g. `bundlewatch`).

---

## Typography token migration

### Status

Open. Tokens exist in `@acid-info/logos-tokens`; raw `font-*` + `text-[Npx]` patterns
appear ~171 times across the codebase. Migrating without a concurrent Figma
spec audit risks visible regressions on typography-heavy frames (press,
design-systems, blog).

### What "done" looks like

1. Inventory the top-five token equivalences from raw classes:

   | Raw pattern | Proposed token |
   | --- | --- |
   | `font-mono text-[10px] font-semibold leading-[1.35] uppercase` | `text-eyebrow` |
   | `font-mono text-[10px] leading-[1.3]` | `text-mono-s` |
   | `font-display text-[36px] leading-none tracking-[-0.03em]` | `text-h3-serif` |
   | `font-sans text-[18px] leading-[1.15] tracking-[-0.01em]` | `text-subhead-sans` |
   | `font-sans text-[14px] leading-[1.2]` | `text-body-sans` |

2. Cross-check each pattern against the corresponding Figma frame; if a frame
   uses a one-off variant, keep the raw class and add an inline comment so the
   next reader doesn't "fix" it.
3. Replace the raw classes with the token name file by file. Keep one PR per
   feature area (press, builders-hub, circles) for safer Figma diffing.
4. After migration, add an ESLint rule (`no-restricted-syntax`) that flags
   `font-mono`/`font-sans`/`font-display` outside the tokens file.

### Why not yet

The work is mechanical *only* if the existing raw classes already match the
token spec. A spot check found at least three near-matches that diverge from
the token by 1px or 0.05em — those would silently change visuals when the
token replaces them.

---

## `alt=""` audit

### Status

Open. 45 instances of `alt=""` across `apps/web`.

### Required content-schema change

In `packages/content/src/schemas/circles.ts`, the `image` field currently
allows `alt: ''`. Make `alt` required + non-empty for content images.

```ts
// proposed
image: z.object({
  src: z.string().min(1),
  alt: z.string().min(1, 'image.alt must describe the image; use a decorative container if the image is purely visual'),
}),
```

### Component-side rule

| Image purpose | `alt` value |
| --- | --- |
| Article thumbnail / podcast cover | `{title}` from Logos Press Engine |
| Decorative blur / pattern background | `alt=""` (intentional, leave a comment) |
| Hero foreground portrait | descriptive copy from translations |

### Action items

1. Tighten the schemas above.
2. Re-run `getPageCopy` integration to surface any data files that violate.
3. Keep Press Engine thumbnails mapped with article titles as their alt text.
4. Audit all 45 sites; add a comment next to each *intentional* `alt=""`.
5. Enable `eslint-plugin-jsx-a11y/alt-text` at error level.

---

## E2E happy-path coverage

### Status

Open. No test runner is configured in `apps/web` yet.

### Bootstrap path

```bash
pnpm add -D playwright @playwright/test --filter web
pnpm exec playwright install --with-deps
```

Add to `apps/web/package.json`:

```json
"scripts": {
  "test:e2e": "playwright test",
  "test:e2e:ui": "playwright test --ui"
}
```

### First flows to cover (in order of incident risk)

1. `/` — home renders, hero loads, navigation overlay opens/closes.
2. `/press` — articles list renders ≥ one article from the API; cards link
   externally.
3. `/circles` — settings + circles + events all resolve; map renders.
4. `/active-circles` — Hasura fetch path works; stat cards show non-zero data.
5. `/builders-hub/ideas/[slug]` and `/builders-hub/rfps/[slug]` — both happy
   path and `notFound()` path (use a known-bad slug).

### Why not yet

Playwright bootstrap and CI integration is its own multi-hour task; bundling
it into the quality pass would obscure the diff.

---

## `default` → `named` export cleanup

### Status

Mostly safe. Constraint: Next.js *requires* `export default` on `page.tsx`,
`layout.tsx`, `loading.tsx`, `error.tsx`, `not-found.tsx`, `template.tsx`,
`route.ts`, and `middleware.ts`.

### Conversion targets (safe)

- `apps/web/components/locale/locale-switcher-select.tsx` — default
  `LocaleSwitcherSelect` → named export.
- `apps/web/components/site-header/site-header-client.tsx` — `SiteHeaderClient`.
- All `components/sections/**/*.tsx` files using `export default` — switch to
  named export, update barrel re-exports accordingly.

### Conversion targets (must stay default)

- `apps/web/app/**/page.tsx`, `layout.tsx`, `loading.tsx`, `error.tsx`,
  `not-found.tsx`.

### Why not yet

Each conversion is mechanical but cascades through the per-section barrels.
Doing it incrementally (one feature area per PR) avoids a 50-file diff.

---

## Vitest infra

### Already done

- `apps/web/vitest.config.ts` and the first test file under
  `apps/web/lib/__tests__/` are in place.

### Still required

- Add the `test` script to `apps/web/package.json` (`vitest run`).
- Wire CI to run it (`turbo run test`).
- Author tests for `lib/reward.ts`, `lib/cn.ts`, `lib/page-sections.ts`.
- Configure coverage thresholds (suggested: 80% for `lib/`).

---

## `cn()` adoption sweep

### Status

Open. `lib/cn.ts` exists; ~25 components still use template-literal
className composition.

### Codemod

Many call sites match the pattern:

```tsx
className={`base-classes ${dynamicClass} ${className ?? ''}`}
```

A safe rewriter is `jscodeshift` with a small transform that detects this
exact shape and replaces it with `cn('base-classes', dynamicClass, className)`.
Hand-written variants will need manual review.

### Why not yet

A naive sed across 25 files breaks template literals that interpolate
non-className expressions; a real codemod is the right tool but requires
setup.

---

## `resolveLocale` + `LocaleParams` adoption

### Status

Open. Helpers in `lib/route-params.ts`; 13 page.tsx files still inline the
`isActiveLocale(locale)` guard, 36 still inline `params: Promise<{ locale }>`.

### Codemod

The transformation is uniform:

```diff
- export default async function FooPage({
-   params,
- }: {
-   params: Promise<{ locale: string }>
- }) {
-   const { locale } = await params
-   if (!isActiveLocale(locale)) {
-     throw new Error(`FooPage received non-active locale "${locale}"`)
-   }
+ export default async function FooPage({ params }: LocaleParams) {
+   const locale = await resolveLocale(params, 'FooPage')
```

A jscodeshift transform that recognises the function-name string in the
error message keeps the page-name argument correct.