Skip to content

Actually run eslint in CI - #295

Merged
Sbussiso merged 1 commit into
masterfrom
deps/lint-in-ci
Sep 12, 2026
Merged

Sbussiso merged 1 commit into
masterfrom
deps/lint-in-ci

Conversation

@Sbussiso

Copy link
Copy Markdown
Contributor

The gap

frontend/eslint.config.js exists because npm run lint used to fail with
"couldn't find a config" — its own header comment says the lint command
"had been a no-op for however long. This file fixes that."

It didn't, quite. No CI job has ever run the linter. Frontend audit + build runs npm ci, npm audit, npm test and the production build, and
stops there. So the config landed and nothing invoked it.

Consequences, both concrete:

  • 13 eslint errors have been sitting in master with nothing reporting them.
  • The eslint 9 → 10 major (Bump eslint and @eslint/js to 10, together #294) had to be evaluated by running the linter
    by hand locally, because CI had no opinion to offer about a linter bump.

Changes

1. deploy.yml gains a Lint (eslint) step in the frontend job, before
the tests.

2. Two rules join the existing warn list in eslint.config.js. They
were the only thing keeping lint at a non-zero exit, and they are the same
family as the four React Compiler advisories this config already demotes,
for the same documented reason:

rule count what it reports
react-hooks/immutability 12 useEffect(() => { loadThings() }, [...]) where loadThings is a const declared further down the component
react-hooks/use-memo 1 first argument is not an inline function expression

The immutability one is correct at runtime — effects run after render,
by which point the binding is initialized. It is the React Compiler saying
it cannot track the value, not a live bug. The fix (hoist each loader, or
useCallback) touches six files and belongs with the next edit to each,
not bundled into a CI change.

eslint exits non-zero on errors only, so the 47 deliberate warnings stay
visible in the log without blocking deploys.

Verification

The gate is not vacuous:

$ npm run lint                      # current tree
✖ 47 problems (0 errors, 47 warnings)     → exit 0

$ echo 'export const probe = () => { thisIsNotDefined() }' > src/__lintprobe.js
$ npm run lint
  1:30  error  'thisIsNotDefined' is not defined  no-undef
✖ 48 problems (1 error, 47 warnings)      → exit 1

🤖 Generated with Claude Code

`eslint.config.js` was added specifically because `npm run lint` had
been silently erroring with "couldn't find a config" and the lint
command was a no-op. The config fixed that — and then nothing ever ran
it. The frontend CI job does `npm ci`, `npm audit`, `npm test` and the
build, and never the linter.

So the no-op survived the fix. Thirteen errors accumulated in master
unnoticed, and the eslint 9 -> 10 major in #294 had to be reviewed by
running the linter by hand, because CI could say nothing about it.

Two changes:

- deploy.yml gains a `Lint (eslint)` step in the frontend job.
- The two React Compiler advisories that were holding lint at a
  non-zero exit join the four already demoted to `warn` by the policy
  written into this config. `react-hooks/immutability` (12) fires on
  `useEffect(() => { loadThings() }, [...])` where `loadThings` is a
  `const` declared further down — correct at runtime, since effects run
  after render, so it is the compiler reporting it cannot track the
  value rather than a live bug. `react-hooks/use-memo` (1) fires on a
  non-inline first argument. Hoisting the six files' loaders is a real
  change that belongs with the next edit to each, not bundled here.

Warnings stay visible in the CI log; only errors fail the build. That
split is the point.

Verified the gate is not vacuous: lint exits 0 on the current tree (47
warnings, 0 errors), and exits 1 with a deliberate `no-undef` planted
in src/.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Sbussiso
Sbussiso merged commit 84c94ad into master Sep 12, 2026
12 checks passed
@Sbussiso
Sbussiso deleted the deps/lint-in-ci branch September 13, 2026 05:04
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