mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
+1
-1
@@ -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 <rule>: ... */` 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 <rule>: ... */` 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)
|
||||
|
||||
@@ -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 }],
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
};
|
||||
@@ -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' }] },
|
||||
],
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user