chore(lint): let the linter see tests/ and scripts/ - #1085
Merged
Conversation
`npm run lint` was `eslint src`, so two whole trees were never linted. Across
the fleet that hid roughly 3,900 errors, none of which any CI leg had shown.
Most of it was the config, not the code, and the same two defects were in
every app.
**scripts/ had no config block at all.** These are CommonJS Node CLI checkers,
and flat config defaults every `.js` to ESM with browser-ish globals, so eslint
read the CommonJS wrapper itself as undefined identifiers: `require`,
`process`, `__dirname`, `__filename`, `module`. A `scripts/**` block now
declares the environment. Declaring beats suppressing here: `no-undef` is the
rule that catches a genuinely misspelled identifier, and dozens of fake
findings would bury a real one. A second block covers `scripts/**/*.mjs`, which
is ESM and needs Node's globals without the CommonJS wrapper.
**The tests block applied a non-TypeScript-aware rule to TypeScript.** It named
`tests/**/*.ts` while setting the CORE `no-unused-vars`, which v9 deliberately
turns off for `.ts` in favour of the `@typescript-eslint` version. The core
rule reads the parameter names inside a function TYPE as bindings, so
t?: (app: string, key: string) => string
reports `app` and `key` as unused variables, and every unused `catch (e)` in a
`.ts` spec reports twice. The block is split now: `.js`/`.mjs` on the core
rule, `.ts`/`.tsx` on the TypeScript one, same patterns on both.
Also: stale `eslint-disable` comments naming plugins eslint 10 no longer
registers, which are themselves errors ("Definition for rule ... was not
found"), and a rule that must not parse shell scripts.
The genuinely real findings were the useful part: dead locals, unused imports,
dead helper functions, unused `catch` bindings, extensionless relative imports,
and a handful of `== null` comparisons spelled out so they still match null AND
undefined.
Verified per app: `npm run lint` 0 errors over src + tests + scripts,
`prettier --check` clean, and the unit suite still green.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
September 1, 2026 10:17
Contributor
Quality Report — ConductionNL/decidiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-l10n-js | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 537/537 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-01 10:42 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
npm run lintwaseslint src, sotests/andscripts/were never linted.Same two config defects as every fleet app:
scripts/had no config block — CommonJS Node CLI checkers read as ESM, sorequire/process/__dirnamereported as undefined identifiers. Ascripts/**block declares the environment; a second coversscripts/**/*.mjs, which is genuinely ESM. Declaring beats suppressingno-undef, the rule that catches a real typo.no-unused-varsto.ts— it reads parameter names inside a function type as bindings and double-reports every unusedcatch (e). Now split by file type.Verified:
npm run lint0 errors over src + tests + scripts,prettier --checkclean, unit suite green.Built in a git worktree so the checkout another session is actively using was never switched.
Here that hid 284 errors. Real findings: 48 extensionless relative imports, 5 unused
catchbindings, 4 unused args, an unused import, and two shapes worth naming:typeof import('@playwright/test')for a helper that takes the whole fixture. The rule bans inlineimport()even inside a type alias, so it is nowimport type * as PlaywrightModuleandtypeof PlaywrightModule.const { children: _, ...leaf } = nincheck-nav-ceiling.js, the omit-a-key idiom. Rewritten as an explicit copy-and-delete so there is no unused binding at all. The checker still prints the identical verdict (25 fragments, 6 primary / 2 footer / 9 settings, at the ADR-004 ceiling).Unit suite 378 passed.