From c266a5e1a7b93574185899460f0ba1e78cecb665 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 13:20:10 +0200 Subject: [PATCH] chore(client): freeze typed promise, hook, size, store and global rules in the ESLint baseline The client lint ran untyped with two react-hooks rules, so nothing saw a floating promise, an async handler where a void one is expected, a whole-store Zustand subscription or a window event bus, all of which client/CLAUDE.md forbids. eslint.config.mjs now lints with the project service and adds, as warnings: no-floating-promises, no-misused-promises (void returns in JSX attributes included), the rest of react-hooks v7's recommended set, complexity 20, max-depth 4, max-params 5 and max-lines-per-function (120 in a .ts function, 300 in a .tsx one, whose body also holds its markup), no-console and no-non-null-assertion. Three local rules in scripts/lib/eslint-rules.mjs cover what no stock rule sees: an empty .catch() callback, use*Store() without a selector (the message points to useShallow, since an object selector without it loops on Zustand 5), and window.__* globals and window.dispatchEvent with an event of our own. scripts/eslint-baseline.json takes today's count of each new rule, so the existing code is frozen, not changed, and each new instance fails lint:warnings. The CI step gets the 8 GB heap typed linting needs. --- .github/workflows/test.yml | 4 + CLAUDE.md | 2 +- client/CLAUDE.md | 2 +- client/eslint.config.mjs | 49 +++++ client/scripts/eslint-baseline.json | 45 ++++- client/scripts/lib/eslint-rules.mjs | 183 ++++++++++++++++++ .../tests/unit/scripts/eslint-rules.test.ts | 66 +++++++ 7 files changed, 340 insertions(+), 11 deletions(-) create mode 100644 client/scripts/lib/eslint-rules.mjs create mode 100644 client/tests/unit/scripts/eslint-rules.test.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8d7b8b164..bc3d69793 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -383,6 +383,10 @@ jobs: - name: Lint # Fails on any ESLint error, like lint:check did, and on any rule # whose warning count rises above scripts/eslint-baseline.json. + # Typed linting (no-floating-promises, no-misused-promises) loads the + # whole TypeScript program, so it gets the heap the server lint gets. + env: + NODE_OPTIONS: --max-old-space-size=8192 run: cd client && npm run lint:warnings - name: Page pattern check diff --git a/CLAUDE.md b/CLAUDE.md index 7accd5e24..6f22572ba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,7 +38,7 @@ cd client && npm run lint:rtl # physical left/right sides m cd client && npm run lint:dup # lines in copied code may only shrink per file, at SonarCloud's thresholds (CI gate) cd client && npm run lint:pairs # desktop and phone views of a feature import its shared hook; every phone screen is listed (CI gate) cd client && npm run lint:size # no source file past 1000 lines, longer ones only shrink (CI gate; server has the same) -cd client && npm run lint:warnings # eslint errors fail, warnings per rule may only shrink (CI gate) +cd client && npm run lint:warnings # eslint (typed) errors fail, warnings per rule may only shrink, new rules start frozen (CI gate) cd server && npm run lint:warnings # the same for the server, against server/scripts/eslint-baseline.json (CI gate) cd server && npm run lint:format # Prettier outside the baseline list (CI gate; shared has the same in lint-prettier.yml) cd server && npm run lint:strict # strict type errors per file may only shrink, a new file has none (CI gate) diff --git a/client/CLAUDE.md b/client/CLAUDE.md index d6cc04e89..563473024 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -9,7 +9,7 @@ npm run dev # Vite dev server; proxies the API/ws/uploads/MCP/OAut npm run build # prebuild generates PWA icons, then vite build npm run typecheck # tsc --noEmit (CI) npm run lint # eslint . (CI runs lint:warnings instead, which fails on the same errors) -npm run lint:warnings # eslint, failing on any error and on any warning count per rule above scripts/eslint-baseline.json and on any entry above its count (app code, tests and eslint-disable'd messages counted apart; a `/* eslint : ... */` config comment fails outright; --update lowers it) +npm run lint:warnings # eslint (typed), failing on any error and on any warning count per rule above scripts/eslint-baseline.json and on any entry above its count (app code, tests and eslint-disable'd messages counted apart; a `/* eslint : ... */` config comment fails outright; --update lowers it). Frozen as warnings: floating/misused promises, `.catch(() => {})` (trek/no-swallowed-catch), react-hooks v7 recommended, complexity/max-depth/max-params/max-lines-per-function (120 per .ts function, 300 per .tsx), `use*Store()` without a selector, `window.__*` and window.dispatchEvent buses (trek/no-window-globals), no-console, no-non-null-assertion; the local rules live in scripts/lib/eslint-rules.mjs npm run lint:pages # enforce the Page pattern (CI gate) npm run lint:rtl # physical left/right sides and rtl-lint-disable comments may only shrink per file (CI gate; --list shows them, --update lowers both baselines) npm run lint:size # no source file or stylesheet past 1000 lines, no test (Playwright specs and fixtures under e2e/ included) past 2000, counted wrapped at 120 columns; longer ones may only shrink (CI gate; --update lowers the baseline) diff --git a/client/eslint.config.mjs b/client/eslint.config.mjs index 8887a2f44..2fe715cee 100644 --- a/client/eslint.config.mjs +++ b/client/eslint.config.mjs @@ -5,6 +5,7 @@ import eslintConfigPrettier from 'eslint-config-prettier'; import reactHooks from 'eslint-plugin-react-hooks'; import reactRefresh from 'eslint-plugin-react-refresh'; import tseslint from 'typescript-eslint'; +import trek from './scripts/lib/eslint-rules.mjs'; // Minimal stub so the existing `// eslint-disable-next-line react/no-danger` // directive in src/i18n/TransHtml.tsx resolves without pulling in the full @@ -46,6 +47,11 @@ export default tseslint.config( 'react-hooks': reactHooks, 'react-refresh': reactRefresh, react: reactStub, + trek, + }, + languageOptions: { + // Typed linting, for the rules that need to know a value is a promise. + parserOptions: { projectService: true, tsconfigRootDir: import.meta.dirname }, }, rules: { 'react/no-danger': 'off', @@ -55,6 +61,10 @@ export default tseslint.config( // (each rule below has pre-existing violations; surfaced as warnings, not blockers) // A hook called conditionally is a crash waiting for the render that skips it. + // The rest of react-hooks v7's recommended set (the React Compiler's checks: + // state set in effects or render, refs read in render, impure renders, + // mutated props) as warnings, each count held by scripts/eslint-baseline.json. + ...Object.fromEntries(Object.keys(reactHooks.configs['recommended-latest'].rules).map((rule) => [rule, 'warn'])), 'react-hooks/rules-of-hooks': 'error', 'react-hooks/exhaustive-deps': 'warn', @@ -73,6 +83,45 @@ export default tseslint.config( 'no-useless-escape': 'warn', 'no-useless-assignment': 'warn', 'preserve-caught-error': 'warn', + + // --- Added later as warnings; lint:warnings freezes what exists and fails on each new one --- + + // A promise nobody awaits or catches loses its failure, and an async + // handler where a void one is expected (onClick={async () => ...}) + // rejects into nowhere. + '@typescript-eslint/no-floating-promises': 'warn', + '@typescript-eslint/no-misused-promises': ['warn', { checksVoidReturn: { attributes: true } }], + 'trek/no-swallowed-catch': 'warn', + // A `!` silences the null the type admits instead of handling it. + '@typescript-eslint/no-non-null-assertion': 'warn', + // Whole-store subscriptions; the message points to useShallow. + 'trek/store-without-selector': 'warn', + // window.__dragData, window.__addToast and window.dispatchEvent buses. + 'trek/no-window-globals': 'warn', + 'no-console': 'warn', + }, + }, + { + // Size and branching, in app code only. ESLint's defaults for complexity + // (20) and max-depth (4). Five parameters: a hook with more takes an + // options object. A function body may hold 120 lines in a .ts file, which + // is logic only; a .tsx component also holds its markup, which is long by + // nature, so it gets 300, still well under the god components this is + // meant to stop (LoginPage was one function of 1300 lines). + files: ['src/**/*.{ts,tsx}'], + ignores: ['src/**/*.test.{ts,tsx}'], + rules: { + complexity: ['warn', 20], + 'max-depth': ['warn', 4], + 'max-params': ['warn', 5], + 'max-lines-per-function': ['warn', { max: 120, skipBlankLines: true, skipComments: true, IIFEs: true }], + }, + }, + { + files: ['src/**/*.tsx'], + ignores: ['src/**/*.test.tsx'], + rules: { + 'max-lines-per-function': ['warn', { max: 300, skipBlankLines: true, skipComments: true, IIFEs: true }], }, }, { diff --git a/client/scripts/eslint-baseline.json b/client/scripts/eslint-baseline.json index 0a13d682d..cae3e83b9 100644 --- a/client/scripts/eslint-baseline.json +++ b/client/scripts/eslint-baseline.json @@ -1,15 +1,33 @@ { "src": { + "@typescript-eslint/no-non-null-assertion": 407, + "@typescript-eslint/no-misused-promises": 877, + "no-console": 88, + "complexity": 231, "@typescript-eslint/no-explicit-any": 154, - "@typescript-eslint/no-unused-expressions": 2, - "@typescript-eslint/no-unused-vars": 155, - "no-empty": 19, - "no-useless-assignment": 7, - "no-useless-escape": 1, - "preserve-caught-error": 53, + "trek/no-swallowed-catch": 135, + "max-params": 35, + "trek/store-without-selector": 69, + "react-hooks/refs": 561, + "react-hooks/set-state-in-effect": 197, + "@typescript-eslint/no-floating-promises": 167, "react-hooks/exhaustive-deps": 121, + "@typescript-eslint/no-unused-vars": 155, + "react-hooks/immutability": 28, + "max-lines-per-function": 133, + "react-hooks/static-components": 78, + "react-hooks/preserve-manual-memoization": 15, + "no-useless-assignment": 7, + "no-empty": 19, + "trek/no-window-globals": 42, "react-refresh/only-export-components": 46, - "unused-disable-directive": 6 + "unused-disable-directive": 6, + "max-depth": 22, + "react-hooks/void-use-memo": 1, + "@typescript-eslint/no-unused-expressions": 2, + "react-hooks/purity": 2, + "preserve-caught-error": 53, + "no-useless-escape": 1 }, "suppressed": { "@typescript-eslint/no-explicit-any": 40, @@ -17,9 +35,18 @@ }, "tests": { "@typescript-eslint/no-explicit-any": 491, - "@typescript-eslint/no-this-alias": 1, - "@typescript-eslint/no-unsafe-function-type": 2, + "@typescript-eslint/no-non-null-assertion": 1949, + "@typescript-eslint/no-misused-promises": 22, "@typescript-eslint/no-unused-vars": 52, + "@typescript-eslint/no-floating-promises": 33, + "trek/no-window-globals": 332, + "trek/no-swallowed-catch": 14, + "react-hooks/globals": 5, + "react-hooks/refs": 22, + "@typescript-eslint/no-this-alias": 1, + "trek/store-without-selector": 3, + "@typescript-eslint/no-unsafe-function-type": 2, + "no-console": 2, "react-refresh/only-export-components": 1 } } diff --git a/client/scripts/lib/eslint-rules.mjs b/client/scripts/lib/eslint-rules.mjs new file mode 100644 index 000000000..98b467bab --- /dev/null +++ b/client/scripts/lib/eslint-rules.mjs @@ -0,0 +1,183 @@ +/* + * The client's own ESLint rules, as the local plugin `trek` in eslint.config.mjs. + * + * Each one finds a smell client/CLAUDE.md forbids and that no stock rule sees + * on its own. They run as warnings, so lint:warnings + * (scripts/eslint-ratchet.mjs) holds what exists today in + * scripts/eslint-baseline.json and fails on each new one. + */ + +/** The expression under any `as`, `!`, `satisfies` or type assertion, which do not change the value. */ +function unwrap(node) { + let current = node; + while ( + current && + (current.type === 'TSAsExpression' || + current.type === 'TSNonNullExpression' || + current.type === 'TSSatisfiesExpression' || + current.type === 'TSTypeAssertion') + ) + current = current.expression; + return current; +} + +const isWindow = (node) => { + const target = unwrap(node); + return ( + (target?.type === 'Identifier' && (target.name === 'window' || target.name === 'globalThis')) || + (target?.type === 'MemberExpression' && + !target.computed && + target.property.name === 'window' && + unwrap(target.object)?.type === 'Identifier' && + unwrap(target.object).name === 'globalThis') + ); +}; + +const propertyName = (member) => + !member.computed && member.property.type === 'Identifier' + ? member.property.name + : member.property.type === 'Literal' && typeof member.property.value === 'string' + ? member.property.value + : null; + +/** + * `.catch(() => {})`, `.catch(() => undefined)`, `.catch(() => null)`: the + * failure disappears without a trace. A write that fails needs a rollback and + * a toast; a read that may fail says why it may, in code that handles it. + */ +/** @type {import('eslint').Rule.RuleModule} */ +const noSwallowedCatch = { + meta: { + type: 'problem', + schema: [], + messages: { + swallowed: + 'This .catch() swallows the error. Roll back and tell the user (toast), or handle the failure on purpose ' + + 'and say why in the handler; never .catch(() => {}).', + }, + }, + create(context) { + const empty = (fn) => { + if (fn?.type !== 'ArrowFunctionExpression' && fn?.type !== 'FunctionExpression') return false; + const body = fn.body; + if (body.type === 'BlockStatement') return body.body.length === 0; + return ( + (body.type === 'Identifier' && body.name === 'undefined') || + (body.type === 'Literal' && body.value === null) || + (body.type === 'UnaryExpression' && body.operator === 'void') + ); + }; + return { + CallExpression(node) { + const callee = node.callee; + if (callee.type !== 'MemberExpression' || propertyName(callee) !== 'catch') return; + if (node.arguments.length === 1 && empty(node.arguments[0])) context.report({ node, messageId: 'swallowed' }); + }, + }; + }, +}; + +/** + * `useTripStore()` with no selector subscribes the component to the whole + * store, so it re-renders on every change anywhere in it. And on Zustand 5 the + * obvious repair, a selector that builds an object, loops forever without + * useShallow, because each call returns a new object. + */ +/** @type {import('eslint').Rule.RuleModule} */ +const storeWithoutSelector = { + meta: { + type: 'problem', + schema: [], + messages: { + whole: + '{{name}}() without a selector subscribes to the whole store and re-renders on every change in it. ' + + 'Select what you read: {{name}}(s => s.field), or for several fields {{name}}(useShallow(s => ({ a: s.a, b: s.b }))). ' + + 'On Zustand 5 an object selector without useShallow loops forever. In an effect or handler use {{name}}.getState().', + }, + }, + create(context) { + return { + CallExpression(node) { + if ( + node.callee.type === 'Identifier' && + /^use[A-Z]\w*Store$/.test(node.callee.name) && + node.arguments.length === 0 + ) + context.report({ node, messageId: 'whole', data: { name: node.callee.name } }); + }, + }; + }, +}; + +/** Events the browser itself fires on window; dispatching one of these is simulating the platform, not a bus. */ +const PLATFORM_EVENTS = new Set([ + 'resize', + 'scroll', + 'storage', + 'online', + 'offline', + 'focus', + 'blur', + 'popstate', + 'hashchange', + 'visibilitychange', + 'load', + 'beforeunload', + 'pagehide', + 'pageshow', + 'message', + 'keydown', + 'keyup', +]); + +/** + * Global mutable state and event buses on window: `window.__dragData`, + * `window.__addToast` and any other `window.__*`, and window.dispatchEvent + * with an event of the app's own. Nothing types them, nothing finds their + * listeners, and a test has to fake the global to reach them. + */ +/** @type {import('eslint').Rule.RuleModule} */ +const noWindowGlobals = { + meta: { + type: 'problem', + schema: [], + messages: { + global: + 'window.{{name}} is global mutable state. Keep it in a module (a store, a small session module) and import it.', + bus: + 'window.dispatchEvent with an event of our own is an untyped global bus. Call a store action, or bump a ' + + 'version counter in a slice that the listener selects.', + }, + }, + create(context) { + const ownEvent = (arg) => { + const event = unwrap(arg); + if (event?.type !== 'NewExpression' || event.callee.type !== 'Identifier') return false; + if (event.callee.name === 'CustomEvent') return true; + if (event.callee.name !== 'Event') return false; + const name = event.arguments[0]; + return !(name?.type === 'Literal' && typeof name.value === 'string' && PLATFORM_EVENTS.has(name.value)); + }; + return { + MemberExpression(node) { + const name = propertyName(node); + if (name?.startsWith('__') && isWindow(node.object)) + context.report({ node, messageId: 'global', data: { name } }); + }, + CallExpression(node) { + const callee = unwrap(node.callee); + if (callee?.type !== 'MemberExpression' || propertyName(callee) !== 'dispatchEvent') return; + if (isWindow(callee.object) && ownEvent(node.arguments[0])) context.report({ node, messageId: 'bus' }); + }, + }; + }, +}; + +export default { + meta: { name: 'trek' }, + rules: { + 'no-swallowed-catch': noSwallowedCatch, + 'store-without-selector': storeWithoutSelector, + 'no-window-globals': noWindowGlobals, + }, +}; diff --git a/client/tests/unit/scripts/eslint-rules.test.ts b/client/tests/unit/scripts/eslint-rules.test.ts new file mode 100644 index 000000000..20c18291c --- /dev/null +++ b/client/tests/unit/scripts/eslint-rules.test.ts @@ -0,0 +1,66 @@ +import { RuleTester } from 'eslint'; +import tseslint from 'typescript-eslint'; +import { describe, it } from 'vitest'; +import trek from '../../../scripts/lib/eslint-rules.mjs'; + +// RuleTester reports through the test framework's describe/it. +RuleTester.describe = describe; +RuleTester.it = it; + +const tester = new RuleTester({ + languageOptions: { + parser: tseslint.parser, + parserOptions: { ecmaFeatures: { jsx: true } }, + }, +}); + +describe('trek/no-swallowed-catch', () => { + tester.run('no-swallowed-catch', trek.rules['no-swallowed-catch'], { + valid: [ + 'load().catch((err) => toast.error(err.message))', + 'load().catch(() => setFailed(true))', + 'promise.then(() => {})', + 'try { a() } catch { b() }', + ], + invalid: [ + { code: 'load().catch(() => {})', errors: [{ messageId: 'swallowed' }] }, + { code: 'load().catch(() => undefined)', errors: [{ messageId: 'swallowed' }] }, + { code: 'load().catch(() => null)', errors: [{ messageId: 'swallowed' }] }, + { code: 'load().catch(function () {})', errors: [{ messageId: 'swallowed' }] }, + ], + }); +}); + +describe('trek/store-without-selector', () => { + tester.run('store-without-selector', trek.rules['store-without-selector'], { + valid: [ + 'const id = useTripStore((s) => s.trip.id)', + 'const { a, b } = useTripStore(useShallow((s) => ({ a: s.a, b: s.b })))', + 'useTripStore.getState().loadTrip(1)', + 'const t = useTranslation()', + 'const s = useStore()', + ], + invalid: [ + { code: 'const store = useTripStore()', errors: [{ messageId: 'whole', data: { name: 'useTripStore' } }] }, + { code: 'const { settings } = useSettingsStore()', errors: [{ messageId: 'whole' }] }, + ], + }); +}); + +describe('trek/no-window-globals', () => { + tester.run('no-window-globals', trek.rules['no-window-globals'], { + valid: [ + "window.dispatchEvent(new Event('resize'))", + "window.addEventListener('resize', onResize)", + 'window.location.reload()', + 'const data = dragSession.current', + ], + invalid: [ + { code: 'window.__dragData = { placeId: 1 }', errors: [{ messageId: 'global', data: { name: '__dragData' } }] }, + { code: "window.__addToast?.('saved', 'success')", errors: [{ messageId: 'global' }] }, + { code: '(window as any).__dragData = null', errors: [{ messageId: 'global' }] }, + { code: "window.dispatchEvent(new CustomEvent('accommodations:refresh'))", errors: [{ messageId: 'bus' }] }, + { code: "window.dispatchEvent(new Event('collab-files-changed'))", errors: [{ messageId: 'bus' }] }, + ], + }); +});