Merge branch 'main' into bugfix/67-sonar-frontend-skipped
This commit is contained in:
@@ -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.
|
||||
|
||||
## SonarQube
|
||||
|
||||
Server is **SonarQube 9.9.8 LTA, Community edition**, at the URL in `SONARQUBE_URL`. Scan settings live in `sonar-project.properties` at the repo root, not as inline `-D` args, so a local scan and the CI scan analyse the same thing; only the host and token come from Gitea secrets.
|
||||
|
||||
@@ -15,6 +15,39 @@ on:
|
||||
workflow_dispatch:
|
||||
|
||||
jobs:
|
||||
# Fails only on the rules with real defect-catching value — unhandled
|
||||
# promises, hook dependencies, missing alt text. Everything else is a warning
|
||||
# and does not block, which is why no --max-warnings flag appears here:
|
||||
# ESLint exits non-zero on errors and zero on warnings on its own. The split,
|
||||
# and the measurements behind it, are in
|
||||
# docs/superpowers/specs/2026-08-19-eslint-design.md.
|
||||
lint:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Setup Node
|
||||
uses: actions/setup-node@v4
|
||||
with:
|
||||
node-version: '20'
|
||||
|
||||
- name: Install backend deps
|
||||
run: npm install
|
||||
working-directory: backend
|
||||
|
||||
- name: Lint backend
|
||||
run: npm run lint
|
||||
working-directory: backend
|
||||
|
||||
- name: Install frontend deps
|
||||
run: npm install
|
||||
working-directory: frontend
|
||||
|
||||
- name: Lint frontend
|
||||
run: npm run lint
|
||||
working-directory: frontend
|
||||
|
||||
backend-unit:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import js from '@eslint/js';
|
||||
import tseslint from 'typescript-eslint';
|
||||
import sonarjs from 'eslint-plugin-sonarjs';
|
||||
import globals from 'globals';
|
||||
|
||||
// Named `.mjs` because this package is CommonJS — `eslint.config.js` would be
|
||||
// parsed as CJS and the imports above would fail.
|
||||
//
|
||||
// Policy: every preset is downgraded to advisory, and the rules that actually
|
||||
// fail the build are listed once at the bottom. That way the CI gate is
|
||||
// readable in one place rather than inferred from four presets' defaults.
|
||||
// The reasoning behind the split, and the measurements it rests on, are in
|
||||
// docs/superpowers/specs/2026-08-19-eslint-design.md.
|
||||
|
||||
/**
|
||||
* Rewrites a preset's enabled rules to `warn`, preserving each rule's options.
|
||||
* Rules the preset explicitly turned off stay off — a preset that disables a
|
||||
* rule means it, and flipping those to `warn` turns the whole of SonarJS's
|
||||
* opt-in catalogue (file headers, naming conventions) into daily noise.
|
||||
*/
|
||||
const advisory = (config) => ({
|
||||
...config,
|
||||
rules: Object.fromEntries(
|
||||
Object.entries(config.rules ?? {}).map(([rule, level]) => {
|
||||
const severity = Array.isArray(level) ? level[0] : level;
|
||||
if (severity === 'off' || severity === 0) return [rule, level];
|
||||
return [rule, Array.isArray(level) ? ['warn', ...level.slice(1)] : 'warn'];
|
||||
})
|
||||
),
|
||||
});
|
||||
|
||||
export default tseslint.config(
|
||||
{ ignores: ['dist/**', 'coverage/**', 'eslint.config.mjs'] },
|
||||
|
||||
...[js.configs.recommended, ...tseslint.configs.recommended, sonarjs.configs.recommended].map(
|
||||
advisory
|
||||
),
|
||||
|
||||
{
|
||||
// `tests/` is deliberately out of scope for now: tsconfig.json only includes
|
||||
// `src`, so type-aware linting has no program for the test files, and
|
||||
// widening it is a separate change with its own violation count.
|
||||
files: ['src/**/*.ts'],
|
||||
languageOptions: {
|
||||
globals: globals.node,
|
||||
parserOptions: {
|
||||
projectService: true,
|
||||
tsconfigRootDir: import.meta.dirname,
|
||||
},
|
||||
},
|
||||
rules: {
|
||||
// The two rules this repo has actually been bitten by. #59 is the whole
|
||||
// argument: an async handler whose rejection nothing forwards produces no
|
||||
// response at all, and the request hangs rather than failing visibly.
|
||||
'@typescript-eslint/no-floating-promises': 'error',
|
||||
'@typescript-eslint/no-misused-promises': [
|
||||
'error',
|
||||
{ checksVoidReturn: { attributes: false } },
|
||||
],
|
||||
},
|
||||
}
|
||||
);
|
||||
Generated
+1526
-1
File diff suppressed because it is too large
Load Diff
@@ -5,6 +5,7 @@
|
||||
"main": "dist/server.js",
|
||||
"scripts": {
|
||||
"build": "tsc",
|
||||
"lint": "eslint src",
|
||||
"start": "node dist/server.js",
|
||||
"dev": "tsx watch src/server.ts",
|
||||
"test": "npm run test:unit",
|
||||
@@ -30,6 +31,7 @@
|
||||
"pg": "^8.12.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "^9.39.5",
|
||||
"@types/bcryptjs": "^2.4.6",
|
||||
"@types/cookie-parser": "^1.4.7",
|
||||
"@types/express": "^4.17.21",
|
||||
@@ -40,10 +42,14 @@
|
||||
"@types/nodemailer": "^6.4.15",
|
||||
"@types/pg": "^8.11.6",
|
||||
"@types/supertest": "^6.0.2",
|
||||
"eslint": "^9.39.5",
|
||||
"eslint-plugin-sonarjs": "^4.2.0",
|
||||
"globals": "^17.11.0",
|
||||
"jest": "^29.7.0",
|
||||
"supertest": "^7.0.0",
|
||||
"ts-jest": "^29.2.4",
|
||||
"tsx": "^4.16.5",
|
||||
"typescript": "^5.5.4"
|
||||
"typescript": "^5.5.4",
|
||||
"typescript-eslint": "^8.67.0"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,11 @@ import { Request, Response, NextFunction, RequestHandler } from 'express';
|
||||
export function asyncRoute(
|
||||
handler: (req: Request, res: Response, next: NextFunction) => unknown
|
||||
): RequestHandler {
|
||||
// Returning a promise where Express expects void is the entire point of this
|
||||
// wrapper, and the promise cannot reject — `.catch(next)` is the last link in
|
||||
// the chain. Express ignores the return value; the unit tests await it to
|
||||
// observe that next() was called.
|
||||
// eslint-disable-next-line @typescript-eslint/no-misused-promises
|
||||
return (req, res, next) => {
|
||||
// Promise.resolve also captures a synchronous throw, so both failure modes
|
||||
// reach the same place.
|
||||
|
||||
+15
-6
@@ -3,8 +3,8 @@ import app from './app';
|
||||
import { pool } from './db';
|
||||
import { sendMail } from './mailer';
|
||||
|
||||
// Release cart holds whose expiry has passed, every 5 minutes.
|
||||
setInterval(async () => {
|
||||
// Release cart holds whose expiry has passed.
|
||||
async function sweepExpiredCarts(): Promise<void> {
|
||||
try {
|
||||
const { rows } = await pool.query(
|
||||
`DELETE FROM cart_items WHERE expires_at < now() RETURNING item_id`
|
||||
@@ -15,10 +15,11 @@ setInterval(async () => {
|
||||
} catch (err) {
|
||||
console.error('cart expiry sweep failed:', (err as Error).message);
|
||||
}
|
||||
}, 5 * 60 * 1000);
|
||||
}
|
||||
|
||||
// Daily cart reminder emails at 9am server time, for customers who opted into marketing email.
|
||||
cron.schedule('0 9 * * *', async () => {
|
||||
// Remind customers who opted into marketing email about items still held in
|
||||
// their cart.
|
||||
async function sendCartReminders(): Promise<void> {
|
||||
try {
|
||||
const { rows } = await pool.query(`
|
||||
SELECT c.email, c.name, i.name AS item_name, ci.expires_at, ci.id AS cart_item_id
|
||||
@@ -53,7 +54,15 @@ cron.schedule('0 9 * * *', async () => {
|
||||
} catch (err) {
|
||||
console.error('daily cart reminder job failed:', (err as Error).message);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Neither scheduler has anything to await these with, so `void` states that the
|
||||
// promise is deliberately dropped. That is only safe because both functions
|
||||
// catch their own errors above — an escaping rejection would be unhandled, and
|
||||
// Node terminates the process on those by default, so a database blip during
|
||||
// the sweep would take the container down with it.
|
||||
setInterval(() => void sweepExpiredCarts(), 5 * 60 * 1000);
|
||||
cron.schedule('0 9 * * *', () => void sendCartReminders());
|
||||
|
||||
const PORT = parseInt(process.env.PORT || '3000', 10);
|
||||
app.listen(PORT, () => console.log(`redefined-designs listening on ${PORT}`));
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
# ESLint — Design
|
||||
|
||||
**Issue:** [#60 — No ESLint anywhere](https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/60)
|
||||
**Date:** 2026-08-19
|
||||
**Status:** Implemented
|
||||
|
||||
## Goal
|
||||
|
||||
Put a linter in front of the code that catches, at authoring time, the classes of defect this project has already shipped and then fixed by hand. Two workspaces, one CI job, green on the day it lands.
|
||||
|
||||
The issue's own framing is the target: TypeScript's `strict: true` checks types and nothing else, so `rules-of-hooks`, `exhaustive-deps`, `no-floating-promises` and the SonarJS rules currently run either never or on a server after the fact.
|
||||
|
||||
## What the code actually reports
|
||||
|
||||
Every number below comes from running the candidate rule set against `backend/src` and `frontend/src` before writing any config, rather than from estimating. A full-strength config — `typescript-eslint` recommendedTypeChecked, `sonarjs`, `react-hooks` v7, `jsx-a11y` — reports **435 violations across 50 files**.
|
||||
|
||||
| Rule family | Count | Disposition |
|
||||
| --- | --- | --- |
|
||||
| `@typescript-eslint/no-unsafe-*` (5 rules) | 325 | **Deferred to #65** |
|
||||
| `no-floating-promises` | 30 | Error — fix here |
|
||||
| `no-misused-promises` | 28 | Error, but 25 disappear under `checksVoidReturn: { attributes: false }` |
|
||||
| `sonarjs/prefer-read-only-props` | 16 | Warn |
|
||||
| `react-hooks` compiler rules (`set-state-in-effect`, `purity`) | 9 | Warn |
|
||||
| `no-explicit-any`, `no-unnecessary-type-assertion` | 11 | Warn |
|
||||
| `sonarjs/no-nested-conditional`, `cognitive-complexity` | 7 | Warn |
|
||||
| `react-hooks/exhaustive-deps` | 2 | Error — fix here |
|
||||
| `jsx-a11y/alt-text` | 2 | Error — fix here |
|
||||
| `rules-of-hooks` | 0 | Error (nothing to fix) |
|
||||
| Remaining singles | 5 | Warn |
|
||||
|
||||
### Two of the issue's premises do not survive measurement
|
||||
|
||||
**`exhaustive-deps` flags 2, not 10.** The issue inferred ten suspects from ten empty dependency arrays. The rule's own analysis clears eight of them — an empty array is only wrong when the effect closes over something that changes, and most of these genuinely do not. The two real ones are `Admin.tsx:62` (missing `load`) and `Cart.tsx:132` (missing `refreshCartContext`). The "decide case by case rather than blanket-autofix" instinct in the issue is still right; it applies to two cases.
|
||||
|
||||
**The backend is already clean on the defect rules.** #59 wrapped every async route, and it shows: zero floating promises in `backend/src`. Its three `no-misused-promises` hits are all benign, and each was read rather than assumed — `asyncRoute.ts:13` is the wrapper performing its entire purpose, and the two `server.ts` scheduled jobs (`setInterval` cart sweep, `cron.schedule` reminder emails) already `try`/`catch` internally, so no rejection escapes.
|
||||
|
||||
## Decisions
|
||||
|
||||
| Area | Decision |
|
||||
| --- | --- |
|
||||
| Config format | Flat config (`eslint.config.mjs`), one per workspace |
|
||||
| Config location | `backend/` and `frontend/` separately — not a root config |
|
||||
| Type-aware linting | On, but narrowly: `recommended`, **not** `recommendedTypeChecked` |
|
||||
| `no-unsafe-*` family | Deferred to #65 |
|
||||
| `no-misused-promises` | `checksVoidReturn: { attributes: false }` |
|
||||
| React Compiler rules | Warn |
|
||||
| CI | New `lint` job in `tests.yml` |
|
||||
| Fail/warn split | Defect rules error; stylistic and advisory rules warn |
|
||||
|
||||
### Why not `recommendedTypeChecked`
|
||||
|
||||
It is the obvious choice and it is the wrong one here. Its `no-unsafe-*` family accounts for 325 of the 435 findings — 75% — and every one of them traces back to two untyped boundaries: `pool.query()` returning `any` rows on the backend, and `fetch(...).json()` on the frontend. Fixing them means typing those boundaries, which is the entire content of [#65](https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/65).
|
||||
|
||||
Turning them on here would produce a lint run whose output is 75% another issue's backlog, which is the reliable way to teach everyone to ignore lint output. Instead this issue enables `recommended` plus the two type-aware rules that catch defects rather than describe type debt:
|
||||
|
||||
```js
|
||||
'@typescript-eslint/no-floating-promises': 'error',
|
||||
'@typescript-eslint/no-misused-promises': ['error', { checksVoidReturn: { attributes: false } }],
|
||||
```
|
||||
|
||||
Both need type information, so `parserOptions.projectService` is still required — the cost of type-aware linting is paid either way. This is a choice about which findings to surface, not about whether to run the type-aware engine.
|
||||
|
||||
Issue #65 turns the `no-unsafe-*` rules on as it types those boundaries. That is the change that makes them green, so that is the change that should own them.
|
||||
|
||||
### Why `checksVoidReturn: { attributes: false }`
|
||||
|
||||
Left at its default, `no-misused-promises` flags every `onClick={async () => …}` — 25 of its 28 hits, all antd buttons in the admin screens. Passing an async function to a React event handler is idiomatic and safe when the function handles its own errors, which these do. typescript-eslint's own documentation recommends this option for React codebases. Without it the rule is 89% noise and would be switched off within a week; with it, the three remaining hits are all real signal.
|
||||
|
||||
### Why the React Compiler rules only warn
|
||||
|
||||
`eslint-plugin-react-hooks` v7 ships the React Compiler rule set — `set-state-in-effect`, `purity`, `static-components` and others — alongside the two classic rules. This is React 18 with no compiler in the build, so those rules are advising on a stricter model than the code is written against.
|
||||
|
||||
They are not worthless: `purity` correctly flags `Date.now()` being called during render in `Cart.tsx:160`. That is a genuine render-purity smell worth seeing. It is not worth failing a build over in a codebase that never opted into the model, and fixing all nine would mean restructuring effects that behave correctly today — behaviour change well outside what #60 asked for.
|
||||
|
||||
Warn keeps the finding visible and the build honest.
|
||||
|
||||
### Why the files are named `.mjs`
|
||||
|
||||
Neither `package.json` sets `"type": "module"`, so an `eslint.config.js` would be parsed as CommonJS and every `import` in it would fail. `.mjs` is the supported flat-config filename for exactly this case. Adding `"type": "module"` to either package instead would change how every other `.js` file in it is interpreted, which is a much larger change than a linter should make.
|
||||
|
||||
### Why per-workspace configs rather than one at the root
|
||||
|
||||
CI already runs `npm install` separately in `backend` and `frontend`, and the two have genuinely different rule needs — React and a11y rules are meaningless in `backend/src`, and each workspace has its own `tsconfig.json` that type-aware linting must point at. A root config would need file-pattern overrides to express a split the directory structure already expresses, and would put React plugins in the backend's dependency tree for no reason.
|
||||
|
||||
The cost is a small amount of duplication between the two config files. That is cheaper than the alternative, and the files are short.
|
||||
|
||||
## Fail/warn split
|
||||
|
||||
The issue asks whether to fail CI on day one or start as warnings, and names the middle path: fail on the rules with real defect-catching value, warn on the rest. That is what this does.
|
||||
|
||||
**Error — build fails (≈37 sites to fix):**
|
||||
|
||||
| Rule | Sites | Fix |
|
||||
| --- | --- | --- |
|
||||
| `no-floating-promises` | 30 | `.catch` on six loaders, then `void` at the call sites |
|
||||
| `no-misused-promises` | 3 | Two restructured, one disabled with a reason |
|
||||
| `exhaustive-deps` | 2 | Triage individually |
|
||||
| `jsx-a11y/alt-text` | 2 | Add `alt` |
|
||||
| `rules-of-hooks` | 0 | — |
|
||||
|
||||
**Warn — visible, does not block (≈73 sites):** SonarJS complexity and style, remaining jsx-a11y, React Compiler rules, `no-explicit-any`.
|
||||
|
||||
No `--max-warnings` flag is needed: ESLint exits non-zero on errors and zero on warnings, so the split falls out of rule severity alone.
|
||||
|
||||
### On `void load()` being a real fix rather than a disguised suppression
|
||||
|
||||
**Corrected during implementation.** The paragraph below was written after reading one loader — `App.tsx`'s, which does catch — and generalised from it. That generalisation was wrong. Checking the other nine sites found that most of the admin loaders (`Admin.tsx`, `Categories.tsx`, `Customers.tsx`, `Tags.tsx`, `Settings.tsx`), plus `Account.tsx` and `CustomerAuthContext.tsx`, had no rejection handling at all. `void` on those would have been exactly the disguised suppression this section warns against.
|
||||
|
||||
So the fix was larger than planned: each of those loaders got a `.catch` that surfaces the failure — `message.error(...)` in the admin screens, and in `CustomerAuthContext` a `.finally` that clears the loading flag, since a rejected `fetchMe()` previously left the app on a permanent spinner rather than showing a signed-out page. Only then does `void` at the call site state something true.
|
||||
|
||||
This is the linter finding real defects on its first run, which is the outcome the issue predicted. It is recorded here because the original reasoning was sound but its premise was not checked widely enough — the per-site check is what caught it.
|
||||
|
||||
Thirty floating promises sounds like thirty bugs; it is not, and the distinction matters because `void` can be either an honest annotation or a way to silence a rule without thinking.
|
||||
|
||||
These are fire-and-forget calls to `load()`-style `useCallback`s in effects and event handlers. The callbacks were read before choosing this fix: each one sets its own error state — `App.tsx`'s `load` calls `setFailed(true)` — so a rejection is already handled inside, and nothing is being swallowed. `void` states "deliberately not awaited," which is exactly true here.
|
||||
|
||||
Had any of them lacked internal handling, the fix for that one would be a `.catch`, not a `void`. The implementation plan checks each site rather than applying `void` mechanically.
|
||||
|
||||
## Files
|
||||
|
||||
| File | Change |
|
||||
| --- | --- |
|
||||
| `backend/eslint.config.mjs` | New |
|
||||
| `frontend/eslint.config.mjs` | New |
|
||||
| `backend/package.json` | `lint` script, devDependencies |
|
||||
| `frontend/package.json` | `lint` script, devDependencies |
|
||||
| `.gitea/workflows/tests.yml` | New `lint` job |
|
||||
| `frontend/src/**` (10 files) | ≈37 violation fixes |
|
||||
| `backend/src/asyncRoute.ts`, `server.ts` | 3 targeted disables |
|
||||
| `.claude/project-context.md` | Record the linter and the deferral to #65 |
|
||||
|
||||
## Testing
|
||||
|
||||
The linter is its own test: `npm run lint` in each workspace must exit 0. Success criteria:
|
||||
|
||||
1. `npm run lint` passes in both workspaces with zero errors.
|
||||
2. The warning count is reported, not zero, and that is expected.
|
||||
3. `npm run build` still passes in both workspaces.
|
||||
4. Backend unit and integration suites still pass — the `void` and disable edits touch frontend files almost exclusively, but `server.ts` and `asyncRoute.ts` are on the backend.
|
||||
5. Playwright e2e passes, since ≈37 edits land in rendered components.
|
||||
6. The `lint` job runs in CI and fails the build when a rule in the error set is violated.
|
||||
|
||||
Point 6 is verified by deliberately introducing a violation locally and confirming a non-zero exit, not by assuming the job's configuration is correct.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **The `no-unsafe-*` family and the `any` backlog** — #65.
|
||||
- **Prettier or any formatting tool.** The issue asks for defect detection. Formatting is a separate argument with separate churn.
|
||||
- **Linting `tests/` and Playwright specs.** Worth doing; a second step once the source is green, so that this change's error count stays the measured one.
|
||||
- **Replacing SonarQube.** The CI scan stays. This puts the same rule families in the editor, where they are cheaper to act on.
|
||||
|
||||
## Follow-ups this creates
|
||||
|
||||
- **#65 owns turning on `no-unsafe-*`** as it types `pool.query` results and `fetch` responses.
|
||||
- **`routesAreWrapped.test.ts` can retire** once `no-misused-promises` runs on the backend in CI. The test predates the linter and covers the same ground by scanning source; it is more targeted and its failure message is better, so this is a judgement call for whoever next touches either. Recorded here so the duplication is deliberate rather than forgotten.
|
||||
@@ -0,0 +1,87 @@
|
||||
import js from '@eslint/js';
|
||||
import tseslint from 'typescript-eslint';
|
||||
import reactHooks from 'eslint-plugin-react-hooks';
|
||||
import jsxA11y from 'eslint-plugin-jsx-a11y';
|
||||
import sonarjs from 'eslint-plugin-sonarjs';
|
||||
import globals from 'globals';
|
||||
|
||||
// Named `.mjs` because this package is CommonJS — `eslint.config.js` would be
|
||||
// parsed as CJS and the imports above would fail.
|
||||
//
|
||||
// Policy: every preset is downgraded to advisory, and the rules that actually
|
||||
// fail the build are listed once at the bottom. That way the CI gate is
|
||||
// readable in one place rather than inferred from four presets' defaults.
|
||||
// The reasoning behind the split, and the measurements it rests on, are in
|
||||
// docs/superpowers/specs/2026-08-19-eslint-design.md.
|
||||
|
||||
/**
|
||||
* Rewrites a preset's enabled rules to `warn`, preserving each rule's options.
|
||||
* Rules the preset explicitly turned off stay off — a preset that disables a
|
||||
* rule means it, and flipping those to `warn` turns the whole of SonarJS's
|
||||
* opt-in catalogue (file headers, naming conventions) into daily noise.
|
||||
*/
|
||||
const advisory = (config) => ({
|
||||
...config,
|
||||
rules: Object.fromEntries(
|
||||
Object.entries(config.rules ?? {}).map(([rule, level]) => {
|
||||
const severity = Array.isArray(level) ? level[0] : level;
|
||||
if (severity === 'off' || severity === 0) return [rule, level];
|
||||
return [rule, Array.isArray(level) ? ['warn', ...level.slice(1)] : 'warn'];
|
||||
})
|
||||
),
|
||||
});
|
||||
|
||||
export default tseslint.config(
|
||||
{ ignores: ['dist/**', 'playwright-report/**', 'test-results/**', 'eslint.config.mjs'] },
|
||||
|
||||
...[
|
||||
js.configs.recommended,
|
||||
...tseslint.configs.recommended,
|
||||
sonarjs.configs.recommended,
|
||||
// v7 of this plugin ships the React Compiler rule set alongside the two
|
||||
// classic rules. This is React 18 with no compiler in the build, so those
|
||||
// extra rules advise against a stricter model than the code was written
|
||||
// for — worth seeing (`purity` catches a real `Date.now()` in render), not
|
||||
// worth failing a build over.
|
||||
reactHooks.configs.flat['recommended-latest'],
|
||||
{ rules: jsxA11y.flatConfigs.recommended.rules },
|
||||
].map(advisory),
|
||||
|
||||
{ plugins: { 'jsx-a11y': jsxA11y } },
|
||||
|
||||
{
|
||||
// `tests/` and the Playwright specs are deliberately out of scope for now:
|
||||
// tsconfig.json only includes `src`, so type-aware linting has no program
|
||||
// for them, and widening it is a separate change with its own count.
|
||||
files: ['src/**/*.{ts,tsx}'],
|
||||
languageOptions: {
|
||||
globals: globals.browser,
|
||||
parserOptions: {
|
||||
projectService: true,
|
||||
tsconfigRootDir: import.meta.dirname,
|
||||
},
|
||||
},
|
||||
rules: {
|
||||
// Hooks called conditionally break React outright.
|
||||
'react-hooks/rules-of-hooks': 'error',
|
||||
// The stale-closure rule. PR #11 fixed a cart badge that did not update
|
||||
// after account creation, which is precisely what this catches.
|
||||
'react-hooks/exhaustive-deps': 'error',
|
||||
// An unawaited promise fails silently — the same class of defect as the
|
||||
// unwrapped async routes in #59, and what the project's Playwright notes
|
||||
// already warn about for missing `await`.
|
||||
'@typescript-eslint/no-floating-promises': 'error',
|
||||
// `attributes: false` because `onClick={async () => ...}` is idiomatic
|
||||
// React and safe when the handler catches its own errors. Left at the
|
||||
// default this rule flags every antd button in the admin screens — 25 of
|
||||
// its 28 hits here — and a rule that is 89% noise gets switched off.
|
||||
'@typescript-eslint/no-misused-promises': [
|
||||
'error',
|
||||
{ checksVoidReturn: { attributes: false } },
|
||||
],
|
||||
// A storefront image with no alt text is unusable in a screen reader, and
|
||||
// the fix is one attribute.
|
||||
'jsx-a11y/alt-text': 'error',
|
||||
},
|
||||
}
|
||||
);
|
||||
Generated
+3497
File diff suppressed because it is too large
Load Diff
@@ -5,6 +5,7 @@
|
||||
"scripts": {
|
||||
"dev": "vite",
|
||||
"build": "tsc && vite build",
|
||||
"lint": "eslint src",
|
||||
"test:e2e": "playwright test"
|
||||
},
|
||||
"dependencies": {
|
||||
@@ -18,13 +19,20 @@
|
||||
"remark-gfm": "^4.0.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "^9.39.5",
|
||||
"@playwright/test": "^1.47.0",
|
||||
"@types/pg": "^8.23.1",
|
||||
"@types/react": "^18.3.3",
|
||||
"@types/react-dom": "^18.3.0",
|
||||
"@vitejs/plugin-react": "^4.3.1",
|
||||
"eslint": "^9.39.5",
|
||||
"eslint-plugin-jsx-a11y": "^6.10.2",
|
||||
"eslint-plugin-react-hooks": "^7.1.1",
|
||||
"eslint-plugin-sonarjs": "^4.2.0",
|
||||
"globals": "^17.11.0",
|
||||
"pg": "^8.23.0",
|
||||
"typescript": "^5.5.4",
|
||||
"typescript-eslint": "^8.67.0",
|
||||
"vite": "^5.4.0"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -92,7 +92,7 @@ export default function App() {
|
||||
return;
|
||||
}
|
||||
setLoading(true);
|
||||
const timer = setTimeout(load, FILTER_DEBOUNCE_MS);
|
||||
const timer = setTimeout(() => void load(), FILTER_DEBOUNCE_MS);
|
||||
return () => clearTimeout(timer);
|
||||
}, [load, awaitingAuth, needsFavoritesAuth]);
|
||||
|
||||
@@ -103,7 +103,7 @@ export default function App() {
|
||||
// Adding to cart flips an item to reserved, and the filter options' price
|
||||
// bounds shift as inventory changes.
|
||||
const reload = useCallback(() => {
|
||||
load();
|
||||
void load();
|
||||
fetchFilterOptions().then(setOptions).catch(() => undefined);
|
||||
}, [load]);
|
||||
|
||||
@@ -171,7 +171,7 @@ export default function App() {
|
||||
showIcon
|
||||
message="Couldn't load items"
|
||||
description="The server didn't return the catalogue. This is usually temporary."
|
||||
action={<Button size="small" onClick={() => { setLoading(true); load(); }}>Retry</Button>}
|
||||
action={<Button size="small" onClick={() => { setLoading(true); void load(); }}>Retry</Button>}
|
||||
/>
|
||||
) : needsFavoritesAuth ? (
|
||||
<Empty description="Sign in to see the items you have favorited">
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import {
|
||||
Layout, Table, Button, Form, Input, InputNumber, Upload, Modal,
|
||||
Space, Tag, Typography, Switch, message, Image as AntImage, theme, Tabs,
|
||||
@@ -43,24 +43,32 @@ function Inventory() {
|
||||
// arrive out of order and an older one can repaint stale rows over a newer
|
||||
// result. Only the most recently issued request is allowed to set state.
|
||||
const latestRequest = useRef(0);
|
||||
const load = (active: ItemFilters = filters) => {
|
||||
// useCallback rather than a plain function so the effect below can depend on
|
||||
// it honestly: a function rebuilt every render would either loop forever in
|
||||
// the dependency array or have to be suppressed out of it.
|
||||
const load = useCallback((active: ItemFilters = filters) => {
|
||||
const seq = ++latestRequest.current;
|
||||
return fetchAdminItems(active).then(rows => {
|
||||
if (seq === latestRequest.current) setItems(rows);
|
||||
});
|
||||
};
|
||||
return fetchAdminItems(active)
|
||||
.then(rows => {
|
||||
if (seq === latestRequest.current) setItems(rows);
|
||||
})
|
||||
// Without this the table simply keeps showing whatever it had, so a
|
||||
// failed refetch after a save looks identical to a save that did not
|
||||
// change anything.
|
||||
.catch(() => message.error('Could not load items'));
|
||||
}, [filters]);
|
||||
|
||||
// The item form needs the current category tree and tag list; both change
|
||||
// from the sibling tabs, so they're refetched whenever the modal opens.
|
||||
const loadOptions = () => Promise.all([
|
||||
const loadOptions = useCallback(() => Promise.all([
|
||||
fetchAdminCategories().then(setCategories),
|
||||
fetchAdminTags().then(setTags)
|
||||
]);
|
||||
]).catch(() => message.error('Could not load categories and tags')), []);
|
||||
|
||||
// Refetch whenever the filters change — filtering is server-side so the
|
||||
// result stays correct regardless of how many items exist.
|
||||
useEffect(() => { load(filters); }, [filters]);
|
||||
useEffect(() => { loadOptions(); }, []);
|
||||
useEffect(() => { void load(filters); }, [load, filters]);
|
||||
useEffect(() => { void loadOptions(); }, [loadOptions]);
|
||||
|
||||
function applyFilters(next: ItemFilters) { setFilters(next); }
|
||||
function clearFilters() { setFilters(EMPTY_FILTERS); }
|
||||
@@ -70,7 +78,7 @@ function Inventory() {
|
||||
form.resetFields();
|
||||
setFileList([]);
|
||||
setDescription('');
|
||||
loadOptions();
|
||||
void loadOptions();
|
||||
setModalOpen(true);
|
||||
}
|
||||
|
||||
@@ -84,7 +92,7 @@ function Inventory() {
|
||||
});
|
||||
setFileList([]);
|
||||
setDescription(item.description || '');
|
||||
loadOptions();
|
||||
void loadOptions();
|
||||
setModalOpen(true);
|
||||
}
|
||||
|
||||
@@ -114,8 +122,8 @@ function Inventory() {
|
||||
|
||||
message.success(editingItem ? 'Item updated' : 'Item added');
|
||||
setModalOpen(false);
|
||||
load();
|
||||
loadOptions();
|
||||
void load();
|
||||
void loadOptions();
|
||||
}
|
||||
|
||||
// Status changes silently did nothing on failure — the row simply stayed put
|
||||
@@ -127,7 +135,7 @@ function Inventory() {
|
||||
message.error(`Couldn't ${label} — ${(err as Error).message}`);
|
||||
return;
|
||||
}
|
||||
load();
|
||||
void load();
|
||||
}
|
||||
|
||||
async function handleDelete(id: number) {
|
||||
@@ -138,7 +146,7 @@ function Inventory() {
|
||||
return;
|
||||
}
|
||||
message.success('Item deleted');
|
||||
load();
|
||||
void load();
|
||||
}
|
||||
|
||||
async function handleDeleteImage(itemId: number, imageId: number) {
|
||||
@@ -149,7 +157,7 @@ function Inventory() {
|
||||
return;
|
||||
}
|
||||
message.success('Image removed');
|
||||
load();
|
||||
void load();
|
||||
setEditingItem(prev => prev && prev.id === itemId
|
||||
? { ...prev, images: prev.images.filter(img => img.id !== imageId) }
|
||||
: prev);
|
||||
@@ -162,7 +170,7 @@ function Inventory() {
|
||||
render: (images: Item['images']) =>
|
||||
images[0] ? (
|
||||
<span style={{ position: 'relative', display: 'inline-block' }}>
|
||||
<img src={images[0].image_path} style={{ width: 60 }} />
|
||||
<img src={images[0].image_path} alt="" style={{ width: 60 }} />
|
||||
{images.length > 1 && (
|
||||
<Tag style={{ position: 'absolute', bottom: -4, right: -8, fontSize: 10 }}>+{images.length - 1}</Tag>
|
||||
)}
|
||||
|
||||
@@ -74,10 +74,11 @@ export default function Categories() {
|
||||
expansionInitialized.current = true;
|
||||
}
|
||||
})
|
||||
.catch(() => message.error('Could not load categories'))
|
||||
.finally(() => setLoading(false));
|
||||
}
|
||||
|
||||
useEffect(() => { load(); }, []);
|
||||
useEffect(() => { void load(); }, []);
|
||||
|
||||
function openNew(parent: number | null) {
|
||||
setEditing(null);
|
||||
@@ -110,7 +111,7 @@ export default function Categories() {
|
||||
// Reveal where the category just landed instead of filing it out of sight.
|
||||
if (parentId !== null) expand(parentId);
|
||||
setModalOpen(false);
|
||||
load();
|
||||
void load();
|
||||
} catch (err) {
|
||||
message.error((err as Error).message);
|
||||
}
|
||||
@@ -137,7 +138,7 @@ export default function Categories() {
|
||||
`Deleted ${result.deleted_categories} categor${result.deleted_categories === 1 ? 'y' : 'ies'}, ` +
|
||||
`uncategorized ${result.uncategorized_items} item${result.uncategorized_items === 1 ? '' : 's'}`
|
||||
);
|
||||
load();
|
||||
void load();
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -145,7 +146,7 @@ export default function Categories() {
|
||||
// Dragging a node onto another reparents it. The server rejects a move that
|
||||
// would put a node beneath its own descendant, so a refused drop just
|
||||
// reloads the unchanged tree.
|
||||
const handleDrop: TreeProps['onDrop'] = async (info) => {
|
||||
const dropCategory = async (info: Parameters<NonNullable<TreeProps['onDrop']>>[0]) => {
|
||||
const dragId = Number(info.dragNode.key);
|
||||
const dropId = Number(info.node.key);
|
||||
const dropToGap = !info.dropToGap ? dropId : findNode(tree, dropId)?.parent_id ?? null;
|
||||
@@ -157,9 +158,14 @@ export default function Categories() {
|
||||
} catch (err) {
|
||||
message.error((err as Error).message);
|
||||
}
|
||||
load();
|
||||
void load();
|
||||
};
|
||||
|
||||
// Kept separate from dropCategory so the handler antd receives returns void,
|
||||
// as its type says. An async function here would hand Tree a promise it never
|
||||
// awaits.
|
||||
const handleDrop: TreeProps['onDrop'] = (info) => void dropCategory(info);
|
||||
|
||||
function toTreeData(nodes: CategoryNode[]): DataNode[] {
|
||||
return nodes.map((node) => ({
|
||||
key: node.id,
|
||||
|
||||
@@ -22,10 +22,14 @@ export default function Customers() {
|
||||
const [togglingId, setTogglingId] = useState<number | null>(null);
|
||||
|
||||
function load() {
|
||||
return fetchCustomers().then(rows => { setCustomers(rows); setLoading(false); });
|
||||
return fetchCustomers()
|
||||
.then(rows => { setCustomers(rows); setLoading(false); })
|
||||
// Without this a failed load leaves the spinner up forever, with no
|
||||
// indication that anything went wrong.
|
||||
.catch(() => { setLoading(false); message.error('Could not load customers'); });
|
||||
}
|
||||
|
||||
useEffect(() => { load(); }, []);
|
||||
useEffect(() => { void load(); }, []);
|
||||
|
||||
async function openDetail(id: number) {
|
||||
setDrawerOpen(true);
|
||||
@@ -83,7 +87,7 @@ export default function Customers() {
|
||||
setTogglingId(null);
|
||||
}
|
||||
message.success(disabling ? 'Account disabled' : 'Account re-enabled');
|
||||
load();
|
||||
void load();
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -103,7 +107,7 @@ export default function Customers() {
|
||||
// Refresh both the popup and the row count behind it, so the count can't
|
||||
// disagree with the list it opened from.
|
||||
setReserved(await fetchReservedItems(reservedFor.id));
|
||||
load();
|
||||
void load();
|
||||
}
|
||||
|
||||
const columns: ColumnsType<CustomerSummary> = [
|
||||
@@ -151,7 +155,7 @@ export default function Customers() {
|
||||
// The whole row opens the customer drawer, so without this the
|
||||
// click reaches both handlers and the drawer opens behind the
|
||||
// reserved-items dialog.
|
||||
onClick={(event) => { event.stopPropagation(); openReserved(customer); }}
|
||||
onClick={(event) => { event.stopPropagation(); void openReserved(customer); }}
|
||||
>
|
||||
{count} item{Number(count) === 1 ? '' : 's'}
|
||||
</Button>
|
||||
@@ -208,7 +212,7 @@ export default function Customers() {
|
||||
loading={loading}
|
||||
dataSource={customers}
|
||||
columns={columns}
|
||||
onRow={row => ({ onClick: () => openDetail(row.id), style: { cursor: 'pointer' } })}
|
||||
onRow={row => ({ onClick: () => void openDetail(row.id), style: { cursor: 'pointer' } })}
|
||||
pagination={{ pageSize: 10 }}
|
||||
/>
|
||||
|
||||
|
||||
@@ -9,10 +9,13 @@ export default function Settings() {
|
||||
const [loading, setLoading] = useState(true);
|
||||
|
||||
useEffect(() => {
|
||||
fetchAdminSettings().then(s => {
|
||||
form.setFieldsValue({ cartExpiryHours: s.cartExpiryHours });
|
||||
setLoading(false);
|
||||
});
|
||||
fetchAdminSettings()
|
||||
.then(s => {
|
||||
form.setFieldsValue({ cartExpiryHours: s.cartExpiryHours });
|
||||
})
|
||||
// A rejection here used to leave the form spinning indefinitely.
|
||||
.catch(() => message.error('Could not load settings'))
|
||||
.finally(() => setLoading(false));
|
||||
}, [form]);
|
||||
|
||||
async function handleSave() {
|
||||
|
||||
@@ -32,10 +32,11 @@ export default function Tags() {
|
||||
setLoading(true);
|
||||
return fetchAdminTags()
|
||||
.then(setTags)
|
||||
.catch(() => message.error('Could not load tags'))
|
||||
.finally(() => setLoading(false));
|
||||
}
|
||||
|
||||
useEffect(() => { load(); }, []);
|
||||
useEffect(() => { void load(); }, []);
|
||||
|
||||
function openNew() {
|
||||
setEditing(null);
|
||||
@@ -68,7 +69,7 @@ export default function Tags() {
|
||||
message.success('Tag added');
|
||||
}
|
||||
setModalOpen(false);
|
||||
load();
|
||||
void load();
|
||||
} catch (err) {
|
||||
message.error((err as Error).message);
|
||||
}
|
||||
@@ -83,7 +84,7 @@ export default function Tags() {
|
||||
onOk: async () => {
|
||||
await deleteTag(tag.id);
|
||||
message.success('Tag deleted');
|
||||
load();
|
||||
void load();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -45,14 +45,16 @@ export default function Cart() {
|
||||
|
||||
function loadAll() {
|
||||
setLoading(true);
|
||||
Promise.all([fetchCart(), fetchAddresses(), fetchConfig()]).then(([cartData, addrs, cfg]) => {
|
||||
void Promise.all([fetchCart(), fetchAddresses(), fetchConfig()]).then(([cartData, addrs, cfg]) => {
|
||||
setItems(cartData.items);
|
||||
setAddresses(addrs);
|
||||
const def = addrs.find(a => a.is_default);
|
||||
setSelectedAddressId(def ? def.id : (addrs[0]?.id ?? null));
|
||||
setConfig(cfg);
|
||||
setLoading(false);
|
||||
});
|
||||
})
|
||||
// Anything here failing left the cart on a spinner with no explanation.
|
||||
.catch(() => message.error('Could not load your cart'))
|
||||
.finally(() => setLoading(false));
|
||||
}
|
||||
|
||||
useEffect(() => { if (customer) loadAll(); }, [customer]);
|
||||
@@ -129,7 +131,10 @@ export default function Cart() {
|
||||
message.error('Checkout error, please try again.');
|
||||
}
|
||||
}).render('#paypal-cart-buttons');
|
||||
}, [paypalReady, selectedAddressId, items.length]);
|
||||
// refreshCartContext is a useCallback with an empty dependency list, so
|
||||
// naming it here cannot re-run this effect and re-render the PayPal
|
||||
// buttons — it just makes the dependency honest.
|
||||
}, [paypalReady, selectedAddressId, items.length, refreshCartContext]);
|
||||
|
||||
if (authLoading || loading) return <Spin style={{ margin: 48 }} />;
|
||||
|
||||
@@ -154,7 +159,7 @@ export default function Cart() {
|
||||
renderItem={item => (
|
||||
<List.Item actions={[<Button danger size="small" onClick={() => handleRemove(item.item_id)}>Remove</Button>]}>
|
||||
<List.Item.Meta
|
||||
avatar={item.images[0] && <img src={item.images[0].image_path} style={{ width: 60, height: 60, objectFit: 'cover' }} />}
|
||||
avatar={item.images[0] && <img src={item.images[0].image_path} alt="" style={{ width: 60, height: 60, objectFit: 'cover' }} />}
|
||||
title={item.name}
|
||||
description={
|
||||
<Text type={new Date(item.expires_at).getTime() - Date.now() < 60 * 60 * 1000 ? 'danger' : 'secondary'}>
|
||||
|
||||
@@ -56,7 +56,7 @@ export default function ItemCard({ item, onChanged }: Props) {
|
||||
setAuthModalOpen(true);
|
||||
return;
|
||||
}
|
||||
doAddToCart();
|
||||
void doAddToCart();
|
||||
}
|
||||
|
||||
// Asked only once a customer has actually favorited something, so the reason
|
||||
@@ -110,7 +110,7 @@ export default function ItemCard({ item, onChanged }: Props) {
|
||||
setAuthModalOpen(true);
|
||||
return;
|
||||
}
|
||||
doToggleFavorite();
|
||||
void doToggleFavorite();
|
||||
}
|
||||
|
||||
const hasMultiple = item.images.length > 1;
|
||||
@@ -183,8 +183,8 @@ export default function ItemCard({ item, onChanged }: Props) {
|
||||
onClose={() => setAuthModalOpen(false)}
|
||||
onSuccess={() => {
|
||||
setAuthModalOpen(false);
|
||||
if (pendingAction === 'favorite') doToggleFavorite();
|
||||
else doAddToCart();
|
||||
if (pendingAction === 'favorite') void doToggleFavorite();
|
||||
else void doAddToCart();
|
||||
}}
|
||||
/>
|
||||
</Card>
|
||||
|
||||
@@ -19,7 +19,7 @@ export default function Account({ onClose }: Props) {
|
||||
const navigate = useNavigate();
|
||||
|
||||
useEffect(() => {
|
||||
if (customer) fetchMyOrders().then(setOrders);
|
||||
if (customer) void fetchMyOrders().then(setOrders).catch(() => message.error('Could not load your orders'));
|
||||
}, [customer]);
|
||||
|
||||
useEffect(() => {
|
||||
|
||||
@@ -25,7 +25,12 @@ export function CustomerAuthProvider({ children }: { children: React.ReactNode }
|
||||
|
||||
const refresh = useCallback(() => {
|
||||
setLoading(true);
|
||||
fetchMe().then(c => { setCustomer(c); setLoading(false); });
|
||||
// A rejection here previously left `loading` true forever, which renders
|
||||
// as a permanent spinner rather than a signed-out page.
|
||||
void fetchMe()
|
||||
.then(c => setCustomer(c))
|
||||
.catch(() => setCustomer(null))
|
||||
.finally(() => setLoading(false));
|
||||
}, []);
|
||||
|
||||
useEffect(() => { refresh(); }, [refresh]);
|
||||
|
||||
Reference in New Issue
Block a user