Skip to content

chore: make lint actually run - #26

Merged
cport1 merged 1 commit into
mainfrom
chore/eslint-flat-config
Aug 22, 2026
Merged

cport1 merged 1 commit into
mainfrom
chore/eslint-flat-config

Conversation

@cport1

@cport1 cport1 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Replaces #25, which GitHub auto-closed when its base branch was deleted on the squash-merge of #24. Same commit, rebased onto main.

The state it was in

All five packages declared eslint@^8.56 and @typescript-eslint@^6 as devDependencies and ran:

"lint": "eslint src/**/*.ts"

There was no config file anywhere in the tree. npm run lint exited 2 in every package, and had since the repo was created:

ESLint couldn't find a configuration file.

CI runs build, test and check:edge, so nothing was gating on it and nobody noticed. Five copies of a toolchain, zero lines linted.

(The glob was also wrong. In sh, src/**/*.ts expands as src/*/*.ts — it would have missed every file directly under src/ even with a config present.)

What this does

  • One flat config at the root (eslint.config.mjs), ESLint 9 + typescript-eslint 8, hoisted to the root the way prettier, tsup and typescript already are. The five duplicated devDependency sets are gone.
  • A deliberately narrow ruleset. Type checking is TypeScript's job and formatting is Prettier's, so what is left is the class of thing neither catches — an unused binding, a case that falls through, an expression statement that does nothing. Generated files (*.generated.ts) are ignored: findings there aren't actionable, since the fix belongs in the Go generator and the file is overwritten on regeneration.
  • no-explicit-any is a warning under a per-package budget (eslint src --max-warnings N), currently 9/25/14/2/5. CI fails if the count grows, so a new any needs a real type or a deliberate decision to raise the number. Same ratchet shape the frontend uses for stylelint. Verified: adding one stray any fails the build.
  • CI runs lint, so this can't rot again.
  • CONTRIBUTING.md documents the budget and says not to add per-package configs.

The 29 errors it surfaced

Mostly mechanical: 18 unused catch (e) bindings became optional catch, two dead const w = window as any assignments went away, two lets became const, and declare global { namespace Express { … } } is now allowed by rule option (allowDeclarations) rather than an inline disable, since augmenting a framework's types is exactly what that escape hatch is for.

Two are worth reading:

_measureJSExecution() had a loop that could be optimised away.

for (let i = 0; i < 1000; i++) {
  Math.sqrt(i) * Math.sin(i);      // result discarded
}
results.mathOps = performance.now() - start;

Nothing consumed the result, so an engine is free to eliminate the loop — which drives mathOps toward zero, and detectAutomation() flags jsTime < 0.1 as "JS execution unusually fast." The author already knew about this hazard one loop down (results.arrayLen = arr.length; // Ensure array is "used" to prevent optimization); the arithmetic and string loops just never got the same treatment. They now accumulate into recorded mathSink / stringLen values. Both are additive keys on a record whose only consumer reads mathOps by name through an open index signature.

The form.submit interception used arguments and a this alias. Rewritten with rest parameters and a closure — the replacement still has to be a function so it keeps the call site's this, but the collector instance now comes in through an arrow rather than const self = this. submit() takes no arguments, so behaviour is unchanged.

One rule I dropped while writing this: I had added no-void, then found it flags (e) => void this._handleClick(e) in widget.ts — which is the correct way to mark a deliberate fire-and-forget. The rule was fighting the idiom the codebase already uses properly.

Follow-up worth considering

Type-aware linting (@typescript-eslint/no-floating-promises and friends) is the highest-value ruleset for this codebase and needs parserOptions.projectService. Left out here to keep this reviewable — it will surface a real batch of findings and slow CI, and deserves its own PR.

Verification

npm run lint (0 errors, 55 warnings, all within budget), npm run build, npm test (306 tests), npm run check:edge — all pass.

Every package declared eslint and @typescript-eslint as devDependencies
and ran `eslint src/**/*.ts`, and no config file had ever existed
anywhere in the tree. So `npm run lint` exited 2 in all five packages,
and had since the repo was created -- CI never ran it, so nothing
noticed.

One flat config at the root (ESLint 9, typescript-eslint 8) replaces five
copies of a toolchain that was never wired up. The ruleset is narrow on
purpose: type checking is TypeScript's job and formatting is Prettier's,
so what is left is the class of thing neither catches. no-explicit-any is
a warning under a per-package --max-warnings budget, and CI now runs lint
so it cannot rot again.

Fixing the 29 errors this surfaced was mostly mechanical -- unused catch
bindings became optional catch, two dead `const w = window as any` went
away, two `let`s became `const`. Two are worth knowing about:

- _measureJSExecution() discarded the result of its arithmetic loop, so
  the loop could legally be optimised away entirely -- which would drive
  mathOps toward zero and trip the "JS execution unusually fast"
  automation signal on an ordinary browser. It now accumulates into a
  recorded mathSink, the way the array loop already used arrayLen, and
  reports stringLen for the same reason. Both are additive keys on a
  record the consumer reads by name.

- The HTMLFormElement.prototype.submit interception uses rest parameters
  and a closure instead of `arguments` and a `this` alias. submit() takes
  no arguments, so behaviour is unchanged.
@cport1
cport1 merged commit 15f1fd3 into main Aug 22, 2026
2 checks passed
@cport1
cport1 deleted the chore/eslint-flat-config branch August 22, 2026 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant