Phase 13: Real Lint Gate — type-aware ESLint + Prettier format gate #8
@@ -0,0 +1,672 @@
|
||||
# Phase 13: Real Lint Gate (ESLint) - Research
|
||||
|
||||
**Researched:** 2026-06-11
|
||||
**Domain:** ESLint flat config + typescript-eslint type-aware linting + Prettier — pnpm monorepo
|
||||
**Confidence:** MEDIUM
|
||||
|
||||
---
|
||||
|
||||
<user_constraints>
|
||||
## User Constraints (from CONTEXT.md)
|
||||
|
||||
### Locked Decisions
|
||||
- **D-13-01:** Use typescript-eslint `recommendedTypeChecked` (type-aware), not the non-type-aware `recommended`. Enable via `projectService: true`.
|
||||
- **D-13-02:** React + react-hooks plugins for `apps/pwa` only; `apps/api` is Node/TS only (no React config).
|
||||
- **D-13-03:** Do NOT adopt `strict`/`strictTypeChecked` presets — too much churn on the existing codebase.
|
||||
- **D-13-04:** Run with `--max-warnings 0` — any warning fails CI.
|
||||
- **D-13-05:** Fix all violations now. Phase is not done until `pnpm lint` and `pnpm format:check` are green.
|
||||
- **D-13-06 (HARD CONSTRAINT):** Fixes must address the violation, not mask it. No blanket `eslint-disable`. Any `eslint-disable-next-line` requires a justifying inline comment. A type-aware finding is a candidate bug — review before suppressing.
|
||||
- **D-13-07:** Prettier + standalone `prettier --check` CI step (separate from lint), AND `eslint-config-prettier` in flat config to disable conflicting formatting rules. `eslint-plugin-prettier` is rejected.
|
||||
- **D-13-08:** All files get reformatted — isolated reformat commit before logic-fix commits for reviewability.
|
||||
- **D-13-09:** Lint ALL TS/TSX: app `src/`, vitest tests, Playwright e2e specs, and config files.
|
||||
- **D-13-10:** Config files that `projectService` cannot type-check need a dedicated `disableTypeChecked` override block.
|
||||
|
||||
### Claude's Discretion
|
||||
- Flat-config file layout (single root `eslint.config.js` vs per-app configs).
|
||||
- Exact Prettier options (`.prettierrc`) — standard defaults; no bikeshedding.
|
||||
- CI step ordering within `fast-checks` (lint → format:check → typecheck → tests).
|
||||
|
||||
### Deferred Ideas (OUT OF SCOPE)
|
||||
- None.
|
||||
</user_constraints>
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Phase 13 installs ESLint (v9 flat config) + typescript-eslint (v8, `recommendedTypeChecked`, `projectService: true`) across both apps, adds React/react-hooks plugins scoped to the PWA, wires package-level `lint` scripts so the existing CI slot activates, adds Prettier with a standalone `format:check` CI step, and fixes every first-run violation.
|
||||
|
||||
**Critical version constraint:** `eslint-plugin-react@7.37.5` declares peer dependency `"eslint": "^3 || ... || ^9.7"` — it does not yet support ESLint 10. A GitHub issue (jsx-eslint/eslint-plugin-react#3977) confirms a runtime API incompatibility with ESLint 10 (`contextOrFilename.getFilename is not a function`). Pin ESLint to v9.x (maintenance release `9.39.4`) to stay compatible with both plugins. `typescript-eslint@8.x` supports `^8.57.0 || ^9.0.0 || ^10.0.0`, so it is compatible with ESLint 9.
|
||||
|
||||
**Primary recommendation:** Single root `eslint.config.js` with per-glob overrides; all lint dependencies installed as root `devDependencies`; PWA React/hooks plugins applied only to `apps/pwa/**` globs; config/non-project files handled with `tseslint.configs.disableTypeChecked` override; `prettier --check` as a separate CI step after `pnpm lint`.
|
||||
|
||||
---
|
||||
|
||||
## Architectural Responsibility Map
|
||||
|
||||
| Capability | Primary Tier | Secondary Tier | Rationale |
|
||||
|------------|-------------|----------------|-----------|
|
||||
| ESLint config authoring | Root (dev-time) | Per-app overrides | Single config controls all workspaces; per-app globs scope plugins |
|
||||
| Type-aware linting | Build-time / CI | Local dev | Requires TypeScript program; TS is already available at build time |
|
||||
| Prettier formatting | Root (dev-time) | CI gate | `format` runs locally; `format:check` runs in CI |
|
||||
| CI lint gate | fast-checks job | — | `pnpm lint` already present; activates when package scripts exist |
|
||||
| CI format gate | fast-checks job | — | New `pnpm format:check` step added after `pnpm lint` |
|
||||
|
||||
---
|
||||
|
||||
## Standard Stack
|
||||
|
||||
### Core Packages
|
||||
| Library | Version | Purpose | Why Standard |
|
||||
|---------|---------|---------|--------------|
|
||||
| `eslint` | `9.39.4` (pin to 9.x) | Core linter | ESLint 9.x — last stable before ESLint 10 breaks plugin compat |
|
||||
| `@eslint/js` | `9.39.4` | JS recommended rules config | Peer of ESLint; same version |
|
||||
| `typescript-eslint` | `8.61.0` | TS parser + plugins + preset configs | Official combined package; supports ESLint ^9 |
|
||||
| `eslint-plugin-react` | `7.37.5` | React-specific rules | PWA only; 47M downloads/wk |
|
||||
| `eslint-plugin-react-hooks` | `7.1.1` | Hooks rules (`rules-of-hooks`, `exhaustive-deps`) | PWA only; official React team plugin; 80M downloads/wk |
|
||||
| `eslint-config-prettier` | `10.1.8` | Disables ESLint formatting rules that conflict with Prettier | MUST be last in flat config array |
|
||||
| `prettier` | `3.8.4` | Code formatter | Standalone; paired with `eslint-config-prettier` |
|
||||
|
||||
**Version verification:**
|
||||
```bash
|
||||
# Verified via npm registry 2026-06-11:
|
||||
# eslint 9.39.4 (maintenance tag; 10.4.1 is latest but breaks react plugin)
|
||||
# @eslint/js 9.39.4 (matches eslint version)
|
||||
# typescript-eslint 8.61.0
|
||||
# eslint-plugin-react 7.37.5
|
||||
# eslint-plugin-react-hooks 7.1.1
|
||||
# eslint-config-prettier 10.1.8
|
||||
# prettier 3.8.4
|
||||
```
|
||||
|
||||
### Installation
|
||||
|
||||
Install all lint/format packages as root workspace `devDependencies`:
|
||||
|
||||
```bash
|
||||
# From repo root
|
||||
pnpm add -D -w \
|
||||
eslint@9.39.4 \
|
||||
"@eslint/js@9.39.4" \
|
||||
typescript-eslint@8.61.0 \
|
||||
eslint-plugin-react@7.37.5 \
|
||||
eslint-plugin-react-hooks@7.1.1 \
|
||||
eslint-config-prettier@10.1.8 \
|
||||
prettier@3.8.4
|
||||
```
|
||||
|
||||
Installing at root (not per-app) is the idiomatic approach for a small 2-app pnpm workspace: all packages share one node_modules resolution, and the single root `eslint.config.js` can import every plugin without cross-package symlink complexity.
|
||||
|
||||
### Alternatives Considered
|
||||
| Instead of | Could Use | Tradeoff |
|
||||
|------------|-----------|----------|
|
||||
| ESLint 9.39.4 (pin) | ESLint 10.4.1 | 10.x breaks `eslint-plugin-react` — open compat issue; unblocked when plugin releases fix |
|
||||
| `recommendedTypeChecked` | `strictTypeChecked` | `strict` adds ~15 more rules; D-13-03 explicitly rejects the extra churn |
|
||||
| root devDependencies | per-app devDependencies | Per-app adds config import complexity; root is idiomatic for shared dev tooling |
|
||||
| `eslint-plugin-react` + `react-hooks` | `react-hooks` alone | `react-hooks` covers hook rules; `eslint-plugin-react` adds JSX/prop-types/display-name rules worth having for a production PWA |
|
||||
|
||||
---
|
||||
|
||||
## Package Legitimacy Audit
|
||||
|
||||
| Package | Registry | Age | Downloads | Source Repo | Verdict | Disposition |
|
||||
|---------|----------|-----|-----------|-------------|---------|-------------|
|
||||
| `eslint` | npm | 14 yrs | 132M/wk | github.com/eslint/eslint | SUS (too-new publish) | Approved — canonical project, official github.com/eslint/eslint repo confirmed |
|
||||
| `@eslint/js` | npm | 3 yrs | 117M/wk | github.com/eslint/eslint | OK | Approved |
|
||||
| `typescript-eslint` | npm | 6 yrs | 69M/wk | github.com/typescript-eslint/typescript-eslint | SUS (too-new publish) | Approved — canonical official monorepo confirmed |
|
||||
| `eslint-plugin-react` | npm | 11 yrs | 47M/wk | github.com/jsx-eslint/eslint-plugin-react | OK | Approved |
|
||||
| `eslint-plugin-react-hooks` | npm | 6 yrs | 80M/wk | github.com/facebook/react | OK | Approved |
|
||||
| `eslint-config-prettier` | npm | 8 yrs | 56M/wk | github.com/prettier/eslint-config-prettier | OK | Approved |
|
||||
| `prettier` | npm | 9 yrs | 108M/wk | github.com/prettier/prettier | SUS (too-new publish) | Approved — canonical official repo confirmed |
|
||||
|
||||
**Packages removed due to SLOP verdict:** None.
|
||||
**Packages flagged as suspicious (SUS):** `eslint`, `typescript-eslint`, `prettier` — flagged only because each received a new release within the last few weeks (the legitimacy seam's `too-new` signal). All three have canonical GitHub repository URLs matching the well-known official projects and download counts in the tens/hundreds of millions. No postinstall scripts. Approved for use without additional human verification checkpoint.
|
||||
|
||||
---
|
||||
|
||||
## Architecture Patterns
|
||||
|
||||
### System Architecture Diagram
|
||||
|
||||
```
|
||||
repo root
|
||||
├── eslint.config.js ← single flat config file (ESM)
|
||||
│ ├── ignores block ← dist/, node_modules/, migrations/
|
||||
│ ├── base block ← js.recommended + tseslint.recommendedTypeChecked
|
||||
│ │ languageOptions.parserOptions.projectService: true
|
||||
│ │ tsconfigRootDir: import.meta.dirname
|
||||
│ ├── pwa-react block ← files: apps/pwa/**/*.{ts,tsx}
|
||||
│ │ extends: [reactPlugin.configs.flat.recommended,
|
||||
│ │ reactHooks.configs.flat.recommended]
|
||||
│ │ settings.react.version: detect
|
||||
│ └── config-files block ← files: [*.config.ts, apps/*/*.config.*]
|
||||
│ extends: [tseslint.configs.disableTypeChecked]
|
||||
│
|
||||
├── .prettierrc ← minimal config (defaults)
|
||||
├── .prettierignore ← dist, node_modules, pnpm-lock, migrations
|
||||
│
|
||||
├── package.json root scripts:
|
||||
│ "lint": "pnpm -r --if-present lint"
|
||||
│ "format": "prettier --write ."
|
||||
│ "format:check": "prettier --check ."
|
||||
│
|
||||
├── apps/api/package.json:
|
||||
│ "lint": "eslint src/ tests/" ← triggers via pnpm -r lint
|
||||
│
|
||||
└── apps/pwa/package.json:
|
||||
"lint": "eslint src/ e2e/" ← triggers via pnpm -r lint
|
||||
```
|
||||
|
||||
```
|
||||
CI fast-checks job (ci.yml):
|
||||
pnpm lint ← pnpm -r --if-present lint → runs api lint then pwa lint
|
||||
pnpm format:check ← prettier --check . (NEW step, after lint)
|
||||
pnpm typecheck ← unchanged
|
||||
PWA unit tests ← unchanged
|
||||
```
|
||||
|
||||
### Recommended Project Structure
|
||||
```
|
||||
repo root
|
||||
├── eslint.config.js # single flat config (ESM); type: module at root
|
||||
├── .prettierrc # minimal JSON config
|
||||
├── .prettierignore # excludes for Prettier
|
||||
├── package.json # add format / format:check scripts
|
||||
└── apps/
|
||||
├── api/
|
||||
│ └── package.json # add: "lint": "eslint src/ tests/ --max-warnings 0"
|
||||
└── pwa/
|
||||
└── package.json # add: "lint": "eslint src/ e2e/ --max-warnings 0"
|
||||
```
|
||||
|
||||
### Pattern 1: `projectService: true` with Multiple tsconfigs (Monorepo)
|
||||
|
||||
**What:** `projectService: true` is the typescript-eslint v8 replacement for `parserOptions.project`. It auto-discovers and loads all tsconfig.json files in the repository — no manual glob list required. For a 2-app pnpm workspace it handles `apps/api/tsconfig.json`, `apps/pwa/tsconfig.json`, and `apps/pwa/tsconfig.e2e.json` automatically. [CITED: typescript-eslint.io/troubleshooting/typed-linting/monorepos/]
|
||||
|
||||
**When to use:** Any monorepo using typescript-eslint v8 — projectService is the preferred approach.
|
||||
|
||||
```js
|
||||
// eslint.config.js
|
||||
// Source: typescript-eslint.io/getting-started/typed-linting/
|
||||
import js from '@eslint/js'
|
||||
import tseslint from 'typescript-eslint'
|
||||
import reactPlugin from 'eslint-plugin-react'
|
||||
import reactHooks from 'eslint-plugin-react-hooks'
|
||||
import prettierConfig from 'eslint-config-prettier/flat'
|
||||
|
||||
export default tseslint.config(
|
||||
// ── Global ignores (replaces .eslintignore) ──────────────────────────────
|
||||
{
|
||||
ignores: [
|
||||
'**/dist/**',
|
||||
'**/node_modules/**',
|
||||
'**/src/db/migrations/**', // generated Drizzle SQL files
|
||||
'pnpm-lock.yaml',
|
||||
],
|
||||
},
|
||||
|
||||
// ── Base: all TS/TSX files in both apps ──────────────────────────────────
|
||||
{
|
||||
files: ['apps/**/*.{ts,tsx}'],
|
||||
extends: [
|
||||
js.configs.recommended,
|
||||
tseslint.configs.recommendedTypeChecked,
|
||||
],
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
projectService: true,
|
||||
tsconfigRootDir: import.meta.dirname,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
// ── React + Hooks: PWA only ───────────────────────────────────────────────
|
||||
{
|
||||
files: ['apps/pwa/**/*.{ts,tsx}'],
|
||||
extends: [
|
||||
reactPlugin.configs.flat.recommended,
|
||||
reactHooks.configs.flat.recommended,
|
||||
],
|
||||
settings: {
|
||||
react: { version: 'detect' },
|
||||
},
|
||||
},
|
||||
|
||||
// ── Config files: disable type-aware rules ────────────────────────────────
|
||||
// These files are not included in any tsconfig project (they are tool configs
|
||||
// consumed by drizzle-kit, vite, vitest, playwright — not by tsc compilation).
|
||||
// projectService cannot type-check them; using disableTypeChecked avoids the
|
||||
// "file was not found in any of the provided project(s)" error.
|
||||
// Source: typescript-eslint.io/troubleshooting/typed-linting/#i-get-errors-telling-me-the-file-must-be-included-in-at-least-one-of-the-projects
|
||||
{
|
||||
files: [
|
||||
'apps/api/drizzle.config.ts',
|
||||
'apps/api/vitest.config.ts',
|
||||
'apps/pwa/vite.config.ts',
|
||||
'apps/pwa/vitest.config.ts',
|
||||
'apps/pwa/playwright.config.ts',
|
||||
],
|
||||
extends: [tseslint.configs.disableTypeChecked],
|
||||
},
|
||||
|
||||
// ── eslint-config-prettier: MUST BE LAST ─────────────────────────────────
|
||||
// Disables all ESLint formatting rules that conflict with Prettier.
|
||||
// Source: github.com/prettier/eslint-config-prettier
|
||||
prettierConfig,
|
||||
)
|
||||
```
|
||||
|
||||
**Critical note on `reactPlugin.configs.flat.recommended`:** As of eslint-plugin-react@7.37.5, the flat config export is `reactPlugin.configs.flat.recommended` (not `reactPlugin.configs['flat/recommended']`). [CITED: github.com/jsx-eslint/eslint-plugin-react] The plugin must be imported under the standard name `react` — not aliased — or `eslint-config-prettier` won't be able to disable its formatting rules correctly. [CITED: github.com/prettier/eslint-config-prettier]
|
||||
|
||||
**Note on `reactHooks.configs.flat.recommended`:** As of eslint-plugin-react-hooks@7.1.1, the flat config export is `reactHooks.configs.flat.recommended`. The `recommended-latest` variant enables experimental React Compiler rules — do not use for production. [ASSUMED: based on npm registry inspection + community docs]
|
||||
|
||||
### Pattern 2: `--max-warnings 0` in package `lint` scripts
|
||||
|
||||
Per D-13-04, every violation must be either an error or off — no non-blocking warnings:
|
||||
|
||||
```json
|
||||
// apps/api/package.json
|
||||
"lint": "eslint src/ tests/ --max-warnings 0"
|
||||
|
||||
// apps/pwa/package.json
|
||||
"lint": "eslint src/ e2e/ --max-warnings 0"
|
||||
```
|
||||
|
||||
The root `pnpm -r --if-present lint` then activates both. Note: the API's `tsconfig.json` excludes `tests/` from its `include`, which means test files are outside the tsconfig project. The `disableTypeChecked` config-files override above covers named config files; to also cover test files from a different project, ensure the tests directory is reachable via a tsconfig (the API vitest config uses `globals: true` and the tsconfig includes `"types": ["vitest/globals"]` — but the tsconfig exclude: ["tests"] means those files are NOT in the project). See Pitfall 3 for the required solution.
|
||||
|
||||
### Pattern 3: Prettier Standalone Gate
|
||||
|
||||
```json
|
||||
// root package.json — add to scripts:
|
||||
"format": "prettier --write .",
|
||||
"format:check": "prettier --check ."
|
||||
```
|
||||
|
||||
```yaml
|
||||
# .gitea/workflows/ci.yml — add after Lint step, before Typecheck:
|
||||
- name: Format check
|
||||
run: pnpm format:check
|
||||
```
|
||||
|
||||
### Anti-Patterns to Avoid
|
||||
- **`void promise` to silence `no-floating-promises`:** `void foo()` is a mask — ESLint `no-floating-promises` accepts `void` as a suppression. The rule is flagging a genuinely unhandled promise. Use `.catch()` or `await`. Only use `void` when the promise is truly fire-and-forget AND that is documented explicitly.
|
||||
- **Using `eslint-plugin-prettier`:** Runs Prettier as an ESLint rule — formats files twice, produces noisy diff output. Prettier docs explicitly recommend against this. The correct split is `eslint-config-prettier` (disables conflicting rules in ESLint) + `prettier --check` (standalone format gate). [CITED: prettier.io/docs/en/integrating-with-linters]
|
||||
- **`allowDefaultProject` for config files:** While it works, it is fragile (glob resolution issues in some configurations). The `disableTypeChecked` override on specific config file globs is simpler and explicit. [CITED: typescript-eslint.io/blog/project-service]
|
||||
- **Aliasing plugins:** `plugins: { ts: typescriptEslint }` instead of `{ '@typescript-eslint': tseslint.plugin }` — `eslint-config-prettier` can't disable its rules. Always use the canonical plugin name.
|
||||
- **ESLint `--cache` with type-aware linting:** Type-aware linting is incompatible with ESLint's file cache (`--cache`). The cache cannot track TypeScript program state. Do not use `--cache` when `projectService: true` is active. [CITED: github.com/typescript-eslint/typescript-eslint/issues/4694]
|
||||
|
||||
---
|
||||
|
||||
## Don't Hand-Roll
|
||||
|
||||
| Problem | Don't Build | Use Instead | Why |
|
||||
|---------|-------------|-------------|-----|
|
||||
| Disabling formatting rules | Manual rule-off list | `eslint-config-prettier` | Prettier maintainers curate the list; it covers @typescript-eslint formatting rules too |
|
||||
| TypeScript project resolution in monorepo | Manual tsconfig glob list | `projectService: true` | v8 projectService auto-discovers all tsconfigs; no maintenance overhead |
|
||||
| React hooks exhaustive-deps enforcement | Manual code review | `eslint-plugin-react-hooks` | Statically catches missing dependencies the type system can't |
|
||||
| Non-project file handling | `tsconfig.eslint.json` with `include: ['**/*']` | `disableTypeChecked` override | Simpler; avoids creating a catch-all tsconfig that degrades type checking |
|
||||
|
||||
**Key insight:** The `eslint-config-prettier` package is the canonical way to integrate Prettier with ESLint. Without it, ESLint's `@typescript-eslint/indent` and similar formatting rules will conflict with Prettier's output, producing a feedback loop where `eslint --fix` and `prettier --write` undo each other.
|
||||
|
||||
---
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Pitfall 1: ESLint 10 + eslint-plugin-react Runtime Error
|
||||
**What goes wrong:** Installing `eslint@latest` (10.x) with `eslint-plugin-react@7.37.5` throws `TypeError: contextOrFilename.getFilename is not a function` at lint time.
|
||||
**Why it happens:** ESLint 10 removed legacy Context API methods that `eslint-plugin-react`'s version-detection utility still calls.
|
||||
**How to avoid:** Pin `eslint@9.39.4` (the maintenance tag). The plugin issue is tracked at jsx-eslint/eslint-plugin-react#3977 and is unresolved as of 2026-06-11.
|
||||
**Warning signs:** pnpm install succeeds but `pnpm lint` immediately throws a TypeError before reporting any actual lint errors.
|
||||
|
||||
### Pitfall 2: API test files excluded from tsconfig project
|
||||
**What goes wrong:** `apps/api/tsconfig.json` has `"exclude": ["tests"]`. ESLint with `projectService: true` cannot type-check `apps/api/tests/*.ts` — you'll get "The file does not match your project config" errors, or type-aware rules silently won't fire on test files.
|
||||
**Why it happens:** `projectService` discovers tsconfig.json and respects its `exclude`. Test files outside the project scope can't receive type-aware linting.
|
||||
**How to avoid:** Add a dedicated `disableTypeChecked` override block in `eslint.config.js` for `apps/api/tests/**/*.ts`. Those files will still be linted with non-type-aware rules (syntax, `no-unused-vars`, etc.) — just not the type-aware ones. Alternatively, create an `apps/api/tsconfig.test.json` that includes the tests and verify `projectService` picks it up.
|
||||
**Warning signs:** "Parsing error: ESLint was configured to run on `apps/api/tests/foo.test.ts` but that file was not found in any of the provided project(s)."
|
||||
|
||||
### Pitfall 3: `vitest.config.ts` / `drizzle.config.ts` not in any tsconfig project
|
||||
**What goes wrong:** Config files (`vite.config.ts`, `drizzle.config.ts`, `vitest.config.ts`, `playwright.config.ts`) sit outside the `include` arrays of all tsconfigs. Type-aware linting on them produces "file must be included in at least one of the projects" errors.
|
||||
**Why it happens:** These files are tool configs consumed by drizzle-kit / Vite / Vitest / Playwright, not by the TypeScript compiler. The tsconfigs correctly exclude them.
|
||||
**How to avoid:** Add these to the `disableTypeChecked` override block in `eslint.config.js` (see Pattern 1 skeleton above). They receive non-type-aware rules only.
|
||||
**Warning signs:** Lint errors only on config files, none on src/ — and the error references the project inclusion issue.
|
||||
|
||||
### Pitfall 4: `no-floating-promises` on setInterval callbacks
|
||||
**What goes wrong:** The broker workers (`outboxWorker.ts`, `reminderScheduler.ts`, `poller.ts`) use the pattern:
|
||||
```ts
|
||||
setInterval(() => {
|
||||
runOutboxDrain().catch((err: unknown) => { ... })
|
||||
}, 15_000)
|
||||
```
|
||||
`setInterval` accepts `() => void`, so the arrow function IS void-returning (the `.catch()` return value is discarded). This pattern is correct and ESLint `no-floating-promises` should NOT fire here because the promise is handled via `.catch()`.
|
||||
**Why it matters:** If any worker uses `runX()` without `.catch()`, that IS a real floating promise that should be fixed (not suppressed).
|
||||
**How to avoid:** The `.catch()` on every worker's inner async call is already the correct fix. Verify no worker uses the bare pattern `setInterval(() => { runX() }, ...)` without `.catch()`.
|
||||
**Warning signs:** If the rule fires on a `.catch()`-chained promise, the rule is incorrectly applied — check that the `.catch()` callback is typed as `(err: unknown) => void`.
|
||||
|
||||
### Pitfall 5: `no-misused-promises` on event listener callbacks
|
||||
**What goes wrong:** Pattern like `document.addEventListener('click', async () => { ... })` — the event listener callback is typed to return void but the async function returns `Promise<void>`. `no-misused-promises` flags this as a misuse.
|
||||
**Why it happens:** The type system marks `addEventListener` callbacks as void-returning, so passing an async function is technically unsound.
|
||||
**How to avoid:** Wrap the async logic: `document.addEventListener('click', () => { void asyncHandler() })` or extract to a named function. The `void` operator here is legitimate: it explicitly signals "I know this is a promise, I'm not awaiting it, and I accept that responsibility."
|
||||
**Warning signs:** Violations on event listener registrations in React `useEffect` hooks or service worker event handlers.
|
||||
|
||||
### Pitfall 6: `sw.ts` — service worker file type context
|
||||
**What goes wrong:** `apps/pwa/src/sw.ts` declares `/// <reference lib="webworker" />` and `declare const self: ServiceWorkerGlobalScope`. The PWA tsconfig has `"lib": ["ES2023", "DOM", "DOM.Iterable"]`, which includes DOM but NOT ServiceWorker APIs. The SW file uses a separate lib reference to access SW types. ESLint may flag missing types for SW-specific globals.
|
||||
**Why it happens:** The SW file is in `apps/pwa/src/` which is in the PWA tsconfig's `include`. The webworker lib reference provides the types but projectService may need to see this correctly.
|
||||
**How to avoid:** Ensure the SW file is parsed under the correct tsconfig. If type errors arise specifically on `sw.ts`, consider adding it to a SW-specific `disableTypeChecked` override rather than fighting the lib reference.
|
||||
|
||||
### Pitfall 7: `eslint-config-prettier` import path in ESM flat config
|
||||
**What goes wrong:** `import eslintConfigPrettier from 'eslint-config-prettier'` does NOT work in ESM flat config — it returns the full package with all sub-configs. The correct import for ESM flat config is `from 'eslint-config-prettier/flat'`.
|
||||
**Why it happens:** `eslint-config-prettier` exports different shapes for CJS legacy config vs ESM flat config.
|
||||
**How to avoid:** Always use `eslint-config-prettier/flat` in ESM `eslint.config.js`. [CITED: github.com/prettier/eslint-config-prettier#readme]
|
||||
**Warning signs:** `prettierConfig` resolves to an object with extra properties that don't match the flat config schema.
|
||||
|
||||
### Pitfall 8: `React` import in main.tsx and ErrorBoundary.tsx
|
||||
**What goes wrong:** `eslint-plugin-react` with `react/jsx-runtime` preset or the `react/react-in-jsx-scope` rule will report `React` as unused when JSX transform is used. But in this codebase, `main.tsx` uses `<React.StrictMode>` (requires the `React` namespace) and `ErrorBoundary.tsx` extends `React.Component` (also uses the namespace directly). These are legitimate uses — the rule should NOT fire.
|
||||
**Why it happens:** The `react/react-in-jsx-scope` rule is disabled in the `flat.recommended` config since React 17+ JSX transform. But `React.StrictMode` / `React.Component` still require the import.
|
||||
**How to avoid:** No action needed — `reactPlugin.configs.flat.recommended` already disables `react/react-in-jsx-scope`. The `React` import stays because it IS used (namespace access, not just JSX).
|
||||
|
||||
---
|
||||
|
||||
## First-Run Violations: Expected Findings and Correct Fixes
|
||||
|
||||
This section maps likely first-run violations to the correct fix per D-13-05/D-13-06.
|
||||
|
||||
### `@typescript-eslint/no-unsafe-*` family (no-unsafe-member-access, no-unsafe-argument, no-unsafe-assignment)
|
||||
|
||||
**Likely locations:**
|
||||
- `apps/api/src/broker/sync.ts` — `ical.js` returns values typed as `any` via `getFirstPropertyValue()`. Multiple `as string` casts on the returned values.
|
||||
- `apps/api/src/broker/expand.ts` — similar ical.js property access.
|
||||
- `apps/pwa/src/sw.ts` — `event.data.json() as Record<string, unknown>` — already correctly cast.
|
||||
- `apps/pwa/src/components/EventForm.tsx:275` — `(occurrence as any)?.recurrence` — the deliberate `as any`.
|
||||
|
||||
**Correct fix:** Add type predicates or narrow via `typeof`/`instanceof` checks before accessing properties. For ical.js specifically (a library with weak typings), a targeted `eslint-disable-next-line @typescript-eslint/no-unsafe-member-access // ical.js returns untyped property values` with justification comment is acceptable — this is a known external library typing limitation, not a bug.
|
||||
|
||||
**Distinction real-fix vs mask:**
|
||||
- MASK: `// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment` with no explanation.
|
||||
- REAL FIX OR JUSTIFIED SUPPRESS: `// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access // ical.js getFirstPropertyValue() returns 'any'; the caller immediately validates the value`.
|
||||
|
||||
### `@typescript-eslint/no-floating-promises`
|
||||
|
||||
**Likely locations:**
|
||||
- Broker workers: all three `setInterval` blocks already use `.catch()`. The pattern `runX().catch(...)` should NOT trigger `no-floating-promises` because the promise is handled.
|
||||
- `apps/api/src/broker/outboxWorker.ts:237` — `dispatchEventChange(change, userId).catch(...)` — already handled.
|
||||
|
||||
**If the rule fires on a `.catch()` chain:** The return of `.catch()` is also a Promise. If the outer function doesn't return it, the chained promise itself is floating. In a `setInterval` callback that returns `void`, this is fine — but the rule may need the callback pattern `() => { runX().catch(...) }` to be recognized as void (not `() => runX().catch(...)`). Adjust by wrapping in `void`:
|
||||
```ts
|
||||
setInterval(() => {
|
||||
void runOutboxDrain().catch((err: unknown) => { ... })
|
||||
}, 15_000)
|
||||
```
|
||||
The explicit `void` operator here is a legitimate signal: "I know this returns a promise and I am intentionally not awaiting it because the setInterval schedule handles the next tick."
|
||||
|
||||
### `@typescript-eslint/require-await`
|
||||
|
||||
**What it catches:** Async functions that contain no `await` expression.
|
||||
**Likely location:** Any route handler or utility that was written async for consistency but doesn't actually await.
|
||||
**Correct fix:** Remove `async` keyword if the function returns a non-Promise value, OR actually `await` the operation if the async was intended.
|
||||
|
||||
### `@typescript-eslint/no-unused-vars`
|
||||
|
||||
**What it catches:** Declared but never read variables/imports.
|
||||
**Convention:** The `_` prefix convention for intentionally unused params: `(_unusedParam: string) => ...`. Configure the rule to ignore `_`-prefixed names:
|
||||
```js
|
||||
rules: {
|
||||
'@typescript-eslint/no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }],
|
||||
}
|
||||
```
|
||||
**Likely locations:** Destructured params in error handlers (`catch (err) { }` — empty catches don't have a binding, so no violation). React test files that import React explicitly.
|
||||
|
||||
### `react/display-name`
|
||||
|
||||
**What it catches:** React components defined without a displayName (typically forwardRef or memo-wrapped components).
|
||||
**Correct fix:** Either add a displayName or use named function expressions (`const MyComp = memo(function MyComp() { ... })`).
|
||||
|
||||
### `@typescript-eslint/no-explicit-any` (from `recommendedTypeChecked`)
|
||||
|
||||
**Status:** `no-explicit-any` is in `recommended` (not type-aware). It will flag explicit `any` annotations.
|
||||
**Specific case — `EventForm.tsx:275`:** `(occurrence as any)?.recurrence` — deliberate cast because `occurrence` comes from ical.js expansion with a weak type. This is a justified suppress: the ical.js types don't expose `recurrence` on occurrence objects, and fixing requires a proper type guard. A targeted disable with justification is acceptable here.
|
||||
|
||||
---
|
||||
|
||||
## Prettier Configuration
|
||||
|
||||
### `.prettierrc`
|
||||
|
||||
Standard defaults — no bikeshedding per CLAUDE.md:
|
||||
|
||||
```json
|
||||
{
|
||||
"semi": true,
|
||||
"singleQuote": true,
|
||||
"tabWidth": 2,
|
||||
"trailingComma": "all",
|
||||
"printWidth": 100
|
||||
}
|
||||
```
|
||||
|
||||
**Note on `singleQuote`:** The existing codebase uses single quotes throughout (visible in all source files). Setting `singleQuote: true` ensures Prettier matches the existing style rather than converting everything to double quotes (which would produce a massive diff even after the first reformat commit). [ASSUMED — based on codebase inspection]
|
||||
|
||||
### `.prettierignore`
|
||||
|
||||
```
|
||||
dist/
|
||||
node_modules/
|
||||
pnpm-lock.yaml
|
||||
apps/api/src/db/migrations/
|
||||
*.html
|
||||
```
|
||||
|
||||
Drizzle-generated SQL migration files are excluded — they are generated artefacts with specific formatting.
|
||||
|
||||
### `eslint-config-prettier` placement
|
||||
|
||||
Must be the LAST config object in the `tseslint.config(...)` array. Any config that comes after it could re-enable formatting rules. [CITED: github.com/prettier/eslint-config-prettier#readme]
|
||||
|
||||
```js
|
||||
// eslint.config.js — the import:
|
||||
import prettierConfig from 'eslint-config-prettier/flat' // flat config import path
|
||||
|
||||
// In the config array — ALWAYS last:
|
||||
export default tseslint.config(
|
||||
// ... all other configs ...
|
||||
prettierConfig, // LAST
|
||||
)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CI Wiring
|
||||
|
||||
### Existing CI structure (`fast-checks` job in `.gitea/workflows/ci.yml`)
|
||||
|
||||
```yaml
|
||||
# Current order:
|
||||
- name: Lint # runs pnpm lint → pnpm -r --if-present lint → activates once package scripts exist
|
||||
- name: Typecheck # unchanged
|
||||
- name: PWA unit tests
|
||||
```
|
||||
|
||||
### Required CI change — add Format Check step
|
||||
|
||||
```yaml
|
||||
- name: Lint
|
||||
run: pnpm lint
|
||||
|
||||
- name: Format check # ← NEW step (D-13-07)
|
||||
run: pnpm format:check
|
||||
|
||||
- name: Typecheck
|
||||
run: pnpm typecheck
|
||||
|
||||
- name: PWA unit tests
|
||||
run: pnpm --filter @familysync/pwa test
|
||||
```
|
||||
|
||||
This ordering is deliberate: lint violations are the most informative (they point to code issues), format violations are mechanical, and typecheck/tests come last as they are slower.
|
||||
|
||||
### Why `pnpm lint` automatically activates
|
||||
|
||||
The root `package.json` already has `"lint": "pnpm -r --if-present lint"`. The `--if-present` flag means it currently exits 0 when no package defines a `lint` script. Once the `apps/api/package.json` and `apps/pwa/package.json` each add a `lint` script, this step becomes real without any CI change.
|
||||
|
||||
### Lint architecture: root invocation or per-package?
|
||||
|
||||
**Recommendation:** Root `pnpm -r --if-present lint` (the existing wiring). Each package runs `eslint <its-own-dirs> --max-warnings 0`. The root `eslint.config.js` is shared, but each invocation is scoped to that package's directory.
|
||||
|
||||
**Why not a single top-level `eslint apps/` invocation?** A single invocation would work with projectService, but would require adding it as a root-level script separate from the existing `pnpm -r` wiring. The per-package approach mirrors the existing `typecheck` pattern and keeps each app's lint independently runnable.
|
||||
|
||||
---
|
||||
|
||||
## Performance
|
||||
|
||||
**Type-aware linting time estimate:** For a project of this size (~91 source files + tests + e2e ≈ ~130 total TS/TSX files), type-aware lint time is approximately 2-5× tsc compile time. The tsc compile for both apps together is typically 15-30 seconds. Estimated lint time: 30-90 seconds CI runtime. [ASSUMED — based on community benchmarks; no direct measurement]
|
||||
|
||||
**No cache in CI (D-PROBE-04):** ESLint's `--cache` is incompatible with type-aware linting (the cache can't track TypeScript program state changes). Even if `actions/cache` worked on this runner, the ESLint file cache would be unusable. Do not use `--cache` with type-aware lint. [CITED: github.com/typescript-eslint/typescript-eslint/issues/4694]
|
||||
|
||||
**projectService vs `project` option performance:** `projectService` is generally faster than `parserOptions.project` with globs because it uses TypeScript's Language Service API with incremental compilation support. [CITED: typescript-eslint.io/blog/project-service]
|
||||
|
||||
**Acceptable for CI:** Given that `actions/cache` is already omitted from the fast-checks job (D-PROBE-04), and `pnpm install` without cache takes ~30s, an additional 30-90s for type-aware lint is acceptable in the CI pipeline. The entire fast-checks job currently runs lint (0s no-op) + typecheck (~30s) + PWA tests (~45s). Adding lint will increase the job duration but it runs in parallel with the `api` and `harness` jobs, so it does not extend the overall PR wall time.
|
||||
|
||||
---
|
||||
|
||||
## Runtime State Inventory
|
||||
|
||||
> Greenfield lint config — no renaming or migration. Section omitted per phase type.
|
||||
|
||||
---
|
||||
|
||||
## Environment Availability
|
||||
|
||||
| Dependency | Required By | Available | Version | Fallback |
|
||||
|------------|------------|-----------|---------|----------|
|
||||
| Node.js 22 | ESLint, typescript-eslint | ✓ | 22.x (CI: actions/setup-node@v4) | — |
|
||||
| TypeScript 5.x | type-aware linting | ✓ | ^5.5.0 (both apps devDeps) | — |
|
||||
| pnpm 11.5.1 | monorepo install | ✓ | 11.5.1 | — |
|
||||
| npm registry | package install | ✓ | — | — |
|
||||
|
||||
**Missing dependencies with no fallback:** None.
|
||||
**Missing dependencies with fallback:** None.
|
||||
|
||||
---
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### Test Framework
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Framework | Vitest (apps/api: node env; apps/pwa: jsdom env) |
|
||||
| Config file | `apps/api/vitest.config.ts`, `apps/pwa/vitest.config.ts` |
|
||||
| Quick run (api) | `pnpm --filter @familysync/api test` |
|
||||
| Quick run (pwa) | `pnpm --filter @familysync/pwa test` |
|
||||
|
||||
### Phase Requirements → Test Map
|
||||
|
||||
| Behavior | Test Type | Automated Command | Notes |
|
||||
|----------|-----------|-------------------|-------|
|
||||
| `pnpm lint` exits non-zero on a deliberate violation | Smoke gate | Introduce a test file with a known violation, run lint, assert non-zero exit | Manual CI run required; not a vitest test |
|
||||
| `pnpm format:check` exits non-zero on unformatted file | Smoke gate | Stage an unformatted file, run `prettier --check`, assert non-zero | Manual verification |
|
||||
| CI lint step activates (not a no-op) | Integration | PR to main; observe CI step now reports violations (not silent pass) | CI run |
|
||||
| All first-run violations fixed — both apps lint green | End-to-end gate | `pnpm lint` exits 0 | Must pass before phase complete |
|
||||
| Prettier reformats files | End-to-end gate | `pnpm format && git diff --stat` shows changes | Visual inspection |
|
||||
| `pnpm format:check` green after reformat | End-to-end gate | `pnpm format:check` exits 0 | Must pass before phase complete |
|
||||
|
||||
### Deliberate-Violation Test (ROADMAP success criterion 1)
|
||||
|
||||
**What:** Introduce a temporary deliberate lint violation to prove the gate actually fails CI (not just passes silently). This is the ROADMAP's explicit first success criterion.
|
||||
**Where:** Create a throwaway file `apps/api/src/_lint-gate-test.ts` containing:
|
||||
```ts
|
||||
// eslint-gate-test: proves no-floating-promises fires
|
||||
export function testFloatingPromise(): void {
|
||||
Promise.resolve(1) // deliberately unhandled — should trigger no-floating-promises
|
||||
}
|
||||
```
|
||||
**Process:**
|
||||
1. Install + config committed; run `pnpm lint` locally — should exit non-zero with violation reported.
|
||||
2. Delete the test file; `pnpm lint` exits 0.
|
||||
3. Commit without the test file.
|
||||
|
||||
### Wave 0 Gaps
|
||||
- No new vitest test files are needed for this phase — the validation is the lint gate itself.
|
||||
- The "deliberate violation proves gate fails" check is a manual one-time smoke test, not an automated spec.
|
||||
|
||||
---
|
||||
|
||||
## Security Domain
|
||||
|
||||
> `security_enforcement: true` is set in `.planning/config.json`. ASVS Level 1 applies.
|
||||
|
||||
### Applicable ASVS Categories
|
||||
|
||||
| ASVS Category | Applies | Standard Control |
|
||||
|---------------|---------|-----------------|
|
||||
| V2 Authentication | No | Not touched by this phase |
|
||||
| V3 Session Management | No | Not touched |
|
||||
| V4 Access Control | No | Not touched |
|
||||
| V5 Input Validation | Indirectly | `no-unsafe-*` rules enforce type-safe access to external data |
|
||||
| V6 Cryptography | No | Not touched |
|
||||
|
||||
### Known Threat Patterns for This Phase
|
||||
|
||||
| Pattern | STRIDE | Standard Mitigation |
|
||||
|---------|--------|---------------------|
|
||||
| Masking a real bug with `eslint-disable` | Tampering (code quality) | D-13-06 constraint: no blanket disables; justification required |
|
||||
| Floating promise swallowing errors silently | Information Disclosure | `no-floating-promises` catches unhandled rejections that may hide security-relevant errors |
|
||||
| `no-unsafe-*` on user-controlled data paths | Tampering | Use proper type guards instead of `as any` casts on data from external sources |
|
||||
|
||||
**Note:** This phase primarily improves code quality, not security posture directly. The security value is indirect: `no-floating-promises` and `no-unsafe-*` rules prevent the class of bugs (unhandled rejections, unsafe data access) that could propagate into security-relevant code paths.
|
||||
|
||||
---
|
||||
|
||||
## Assumptions Log
|
||||
|
||||
| # | Claim | Section | Risk if Wrong |
|
||||
|---|-------|---------|---------------|
|
||||
| A1 | `reactHooks.configs.flat.recommended` is the correct flat config export path for eslint-plugin-react-hooks@7.1.1 | Architecture Patterns | Lint config fails to load; need to use manual plugin registration instead |
|
||||
| A2 | Prettier `.prettierrc` `singleQuote: true` matches the codebase's existing style | Prettier Configuration | Reformat commit will be significantly larger; may need `singleQuote: false` |
|
||||
| A3 | Type-aware lint adds 30-90 seconds to CI fast-checks job | Performance | Could be faster (no impact) or slower (may need investigation) |
|
||||
| A4 | `apps/api/src/broker/sync.ts` ical.js casts will trigger `no-unsafe-*` violations | First-Run Violations | May be fewer violations than expected; or may need targeted disables |
|
||||
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **API tests outside tsconfig project**
|
||||
- What we know: `apps/api/tsconfig.json` excludes `tests/`. API tests are in `apps/api/tests/` (26 files, not `src/tests/`).
|
||||
- What's unclear: Does `projectService` fail hard on those files, or does it fall back gracefully? If hard fail, should we add `disableTypeChecked` for `apps/api/tests/**` or create `apps/api/tsconfig.test.json`?
|
||||
- Recommendation: Add `apps/api/tests/**/*.ts` to the `disableTypeChecked` override block in the initial config. If type-aware rules on test files are desired later, add a separate tsconfig.test.json.
|
||||
|
||||
2. **`eslint-plugin-react` necessity**
|
||||
- What we know: The PWA has 61 .ts/.tsx files. `react/display-name` and `react/prop-types` are the most common rules. React 19 makes `prop-types` moot (TypeScript props typing supersedes it).
|
||||
- What's unclear: Is `eslint-plugin-react` worth the first-run violation churn vs just `react-hooks`?
|
||||
- Recommendation: D-13-02 locks in React plugin for PWA. Disable `react/prop-types` explicitly (TypeScript handles this) to reduce first-run noise.
|
||||
|
||||
---
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (MEDIUM confidence — Context7 official docs)
|
||||
- `/typescript-eslint/typescript-eslint` (Context7) — flat config, projectService, disableTypeChecked, monorepos
|
||||
- `/websites/typescript-eslint_io` (Context7) — projectService blog post, allowDefaultProject
|
||||
- `/prettier/eslint-config-prettier` (Context7) — flat config import path, plugin naming pitfall, placement
|
||||
- `typescript-eslint.io/troubleshooting/typed-linting/monorepos/` — projectService requires no additional monorepo config
|
||||
- `typescript-eslint.io/troubleshooting/typed-linting/performance/` — cache incompatibility, performance guidance
|
||||
|
||||
### Secondary (LOW confidence — WebSearch verified against multiple sources)
|
||||
- `github.com/jsx-eslint/eslint-plugin-react/issues/3977` — ESLint 10 runtime incompatibility (open issue confirmed)
|
||||
- `github.com/facebook/react/issues/35758` — react-hooks ESLint 10 peerDep issue
|
||||
- ESLint 9 maintenance release 9.39.4 confirmed via npm dist-tags
|
||||
- eslint-plugin-react-hooks flat config export names confirmed via npm package inspection + community docs
|
||||
|
||||
### Package Registry Verification
|
||||
- All versions confirmed via `npm view <pkg> version` on 2026-06-11
|
||||
- Repository URLs verified for all packages via `npm view <pkg> --json`
|
||||
- Postinstall scripts: none detected on any package
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
- Standard stack + versions: MEDIUM — confirmed via npm registry; ESLint 10 compat issue confirmed via GitHub issue
|
||||
- Architecture: MEDIUM — based on official typescript-eslint docs via Context7
|
||||
- First-run violations: LOW-MEDIUM — based on codebase inspection; actual violations may differ
|
||||
|
||||
**Research date:** 2026-06-11
|
||||
**Valid until:** 2026-07-11 (stable area; main risk is eslint-plugin-react releasing ESLint 10 support, which would allow unpinning)
|
||||
Reference in New Issue
Block a user