cloudfour / cloudfour/lighthouse-parade
Adopt the lint rules deferred during the eslint-config 26 upgrade
- Dominant language
- TypeScript
- Stars
- 373
- Forks
- 15
- Avg merge
- 13h 36m
- Merged PRs (30d)
- 34
Description
## Background
`@cloudfour/eslint-config` 26 (landing with eslint 10 in #377) enables a set of rules that flag existing code. They're switched off in `eslint.config.js` so the toolchain upgrade stayed reviewable — 56 violations across 16 rules would have buried the actual dependency change.
Two reasons for deferring rather than fixing inline. None of the violations were found to be hiding defects when reviewed; they're style and strictness preferences. And the files they concentrate in — `cli.ts`, `crawl.ts`, `lighthouse.ts` — are the least tested in the repo (`cli.ts` was at 0% before #362), so a broad hand-edit there carries more risk than it appears to. Today's session already turned up two real bugs in exactly those files (#371, #373).
## The deferred rules
| Rule | Violations | Notes |
|---|---|---|
| `@typescript-eslint/strict-void-return` | 14 | Mostly `.on('event', () => emit(...))` in tests |
| `require-unicode-regexp` | 10 | The `v` flag changes escaping rules — needs care, not a blind fix |
| `@typescript-eslint/strict-boolean-expressions` | 8 | Several around genuinely nullable crawler fields |
| `@typescript-eslint/no-shadow` | 7 | All in `emitter.ts` generics |
| `n/prefer-global/process` | 3 | |
| `markdown/fenced-code-language` | 2 | README code fences |
| `@typescript-eslint/prefer-nullish-coalescing` | 2 | `\|\|=` → `??=`, equivalent here |
| `@typescript-eslint/restrict-plus-operands` | 2 | **See #380 — this one points at a real latent bug** |
| `import-x/no-anonymous-default-export` | 1 | |
| `@typescript-eslint/default-param-last` | 1 | |
| `unicorn/prefer-promise-try` | 1 | |
| `unicorn/prefer-promise-with-resolvers` | 1 | |
| `@eslint-community/eslint-comments/require-description` | 1 | |
| `@typescript-eslint/use-unknown-in-catch-callback-variable` | 1 | |
## Suggested approach
One rule per PR, smallest first, deleting its entry from the override block in `eslint.config.js` as it goes. `markdown/fenced-code-language` and the single-violation rules are near-free.
Two are worth doing deliberately rather than mechanically:
- **`require-unicode-regexp`** — the `v` flag is not a no-op. It changes character-class escaping, so each regex needs checking rather than a blanket `--fix`.
- **`@typescript-eslint/restrict-plus-operands`** — don't suppress this one; it's flagging a real problem. Details in #380.
## A caution from this upgrade
`eslint --fix` broke things three separate ways during this work, so it's worth running the tests after each rule rather than trusting the autofix:
- `unicorn/prefer-https` rewrote a URL in `test/utilities.test.ts` **inputs** while leaving the expected output alone, silently corrupting the assertion.
- `unicorn/prefer-string-replace-all` emitted `replaceAll`, which didn't compile under the then-current `target: es2020`.
- `unicorn/prefer-queue-microtask` swapped `process.nextTick` for `queueMicrotask` in a test that depends on microtask ordering.
Related: #377, #380
Contributor guide
Research direction
Start in eslint.config.js and run the lint checks to inspect the deferred override block. Work through one rule at a time, reading the affected cli.ts, crawl.ts, lighthouse.ts, emitter.ts, README fences, and related #380 details where mentioned. Done means the deferred overrides are removed, lint passes, and tests still pass after each rule.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- eslint, typescript
- Domain
- cli, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 62/100