cloudfour / cloudfour/lighthouse-parade

Adopt the lint rules deferred during the eslint-config 26 upgrade

Open
#379 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.