feat(ci): add ESLint to both workspaces with a defect-only CI gate (#60)
SonarQube Analysis / sonarqube (pull_request) Successful in 3m24s
Tests / lint (pull_request) Successful in 1m54s
Tests / backend-unit (pull_request) Successful in 43s
Tests / frontend-e2e (pull_request) Failing after 8m27s

TypeScript's strict mode checks types and nothing else, so nothing enforced the React hook rules, the SonarJS rules, or unhandled-promise detection. Adds a flat config per workspace, a lint script in each, and a lint job in tests.yml.

The rule selection is the substance of this change and is measured rather than guessed. A full-strength config reports 435 violations across 50 files, but 325 of those are the no-unsafe-* family from recommendedTypeChecked, every one downstream of pool.query() returning any rows and untyped fetch responses. Typing those boundaries is the whole of #65, so enabling the rules here would ship a linter whose output is three-quarters another issue's backlog — the reliable way to teach everyone to ignore lint output. This enables recommended plus the two type-aware rules that catch defects rather than describe type debt, which leaves 110 findings.

Both configs downgrade every preset to a warning and then list the error rules explicitly at the bottom, so the CI gate is readable in one place instead of inferred from four presets' defaults. Errors are no-floating-promises, no-misused-promises, rules-of-hooks, exhaustive-deps and jsx-a11y/alt-text; everything else warns. No --max-warnings flag is needed because ESLint already exits non-zero on errors and zero on warnings. no-misused-promises runs with checksVoidReturn.attributes false, since onClick={async () => ...} is idiomatic React and safe when the handler catches its own errors — at the default it flags every antd button in the admin screens, 25 of its 28 hits, and a rule that is 89% noise gets switched off within a week.

The 37 errors this surfaced were mostly not the mechanical fix they looked like. The plan assumed the 30 floating promises were fire-and-forget loaders that already handled their own failures, which was true of the one sampled when writing the design and false for most of the rest: Admin, Categories, Customers, Tags, Settings, Account and CustomerAuthContext all had no rejection handling at all, so `void` on them would have hidden real failures rather than annotated deliberate ones. Each of those loaders now catches and surfaces the failure before the call site voids it. The CustomerAuthContext one was a live bug — a rejected fetchMe left loading true forever, rendering as a permanent spinner instead of a signed-out page.

Admin's load became a useCallback so its effect can name it honestly rather than suppress the dependency, Categories' drop handler was split so the function antd receives returns void as its type says, and Cart's effect now names refreshCartContext, which is a useCallback with an empty dependency list and so cannot re-run it. The only disable added is in asyncRoute, where returning a promise where Express expects void is the entire point of the wrapper and the promise cannot reject.

Two of the issue's premises did not survive measurement, both recorded in the spec: exhaustive-deps flags 2 cases rather than the 10 inferred from empty dependency arrays, and the backend was already clean on the defect rules because #59 wrapped every async route.

Verified: lint, build, 78 unit, 134 integration and 83 e2e all pass in both workspaces, and the CI gate was confirmed to fail by introducing a deliberate violation rather than by assuming the job is wired correctly.

Closes #60
This commit is contained in:
2026-08-19 14:08:37 -05:00
parent 3cb6a42fb3
commit c058b3ed2e
21 changed files with 5351 additions and 65 deletions
+11 -1
View File
@@ -93,7 +93,7 @@ A storefront for one-of-a-kind items (quantity 1 per item — once sold, it's go
| # | Step | Gate before moving on |
| --- | --- | --- |
| 1 | Implement on a branch, verify locally | Unit + integration + e2e pass; `tsc --noEmit` and `npm run build` clean |
| 1 | Implement on a branch, verify locally | `npm run lint`, unit, integration and e2e all pass; `tsc --noEmit` and `npm run build` clean |
| 2 | Commit and push | `git branch -a --contains <sha>` lists the pushed branch |
| 3 | Open a PR and merge to `main` | The merge actually contains the expected commits |
| 4 | **Build and deploy to QA, and review it in a browser** | The change does what it claims, behind authentik |
@@ -226,6 +226,16 @@ sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c
- **Design specs live in `docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md`** and are committed before implementation starts.
- **`.superpowers/` stays gitignored, but design artifacts inside it must be lifted out before they are lost.** That directory is scratch state belonging to the brainstorming tool and contains a session token, PID files, and absolute local paths — none of which belong in the repo. The mockups it holds *are* worth keeping, so copy them into `docs/superpowers/specs/<date>-<topic>-mockups/` and wrap them as standalone pages (they are served as fragments inside a tool-provided frame, so they need its style tokens and `toggleSelect` helper inlined to open on their own). Keep the rejected options, not just the chosen one — the value is in the comparison.
## Linting
`npm run lint` in each workspace (`backend/eslint.config.mjs`, `frontend/eslint.config.mjs`, flat config, `.mjs` because neither package is `"type": "module"`). Added in #60, with a `lint` job in `tests.yml`.
The severity split is deliberate and is the whole design: every preset is downgraded to a warning, and only the rules that catch real defects are errors — `no-floating-promises`, `no-misused-promises`, `rules-of-hooks`, `exhaustive-deps`, `jsx-a11y/alt-text`. They are listed explicitly at the bottom of each config, so the CI gate is readable in one place. No `--max-warnings` flag is needed: ESLint exits non-zero on errors and zero on warnings by itself. Expect roughly 10 warnings on the backend and 34 on the frontend — that is the intended state, not a backlog someone forgot.
**`recommendedTypeChecked` is deliberately not enabled.** Its `no-unsafe-*` family reports ~325 violations, all of them downstream of `pool.query()` returning `any` rows and untyped `fetch(...).json()`. That is #65's work, and turning the rules on before that work is done buries the ~44 warnings worth reading under a backlog belonging to another issue. #65 enables them as it types those boundaries.
`@typescript-eslint/no-misused-promises` runs with `checksVoidReturn: { attributes: false }`, because `onClick={async () => ...}` is idiomatic React and safe when the handler catches its own errors — left at the default the rule flags every antd button in the admin screens.
## Testing
- **Unit tests**: `cd backend && npm run test:unit` — no DB required