Update @typescript-eslint to v8 - #5853
Conversation
The catalog pinned 7.18.0, whose peer range is `eslint: ^8.56.0` with no typescript entry at all — this repo builds on TypeScript 5.9.3, which is why host's config carries `warnOnUnsupportedTypeScriptVersion: false`. 8.67.0 declares `typescript: >=4.8.4 <6.1.0`, so the version we compile with is a version the linter supports. It also makes type-aware linting possible for host. Host lints `.ts` through this parser and `.gts` through `ember-eslint-parser`'s own copy, which is on v8; while the two were different majors they fought over patching TypeScript, and enabling `parserOptions.project` on both left 974 files reporting that the tsconfig did not include them and took 533s. On one major it is 0 files and 53s. This bump raises 207 existing violations in host from typescript-eslint's own rules, left for a follow-up so this change stays a version bump and reverting it stays cheap. Around 56 are config drift rather than defects: v8 splits `ban-types` into `no-empty-object-type`, `no-unsafe-function-type` and `no-wrapper-object-types`, and the root config disables `ban-types`, so the successors need the same treatment. The rest are `no-unused-vars` and `no-unused-expressions`. Depends on eslint being declared in realm-server and software-factory: this bump changes how `@typescript-eslint/parser`'s `*` peer resolves an eslint, and without that declaration both packages pick up eslint 9 and fail demanding flat config. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16be9feced
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "@types/yargs": ^17.0.10 | ||
| "@typescript-eslint/eslint-plugin": ^7.18.0 | ||
| "@typescript-eslint/parser": ^7.18.0 | ||
| "@typescript-eslint/eslint-plugin": ^8.67.0 |
There was a problem hiding this comment.
In the checked .github/workflows/ci-lint.yaml, the Lint Host step invokes packages/host's pnpm run lint, and this catalog bump changes plugin:@typescript-eslint/recommended for every catalog consumer. The host config disables v7's removed ban-types rule but not v8's replacements (no-empty-object-type, no-unsafe-function-type, and no-wrapper-object-types), while existing host sources use both Function and {} types; together with the other newly reported v8 violations, this makes the required lint job exit nonzero. Disable or migrate the replacement rules and resolve the remaining new violations as part of this upgrade.
AGENTS.md reference: AGENTS.md:L160-L162
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Agreed, and the reasoning I'd used was wrong: Lint Host is a required job, so "mechanical bump now, violations in a follow-up" was never actually available — a branch that can't go green can't merge. AGENTS.md says to run pnpm lint in modified packages before committing, and I had run it, seen the failure, and shipped anyway.
Fixed in 1909a17. Host is now 0 problems, and the scope turned out wider than host: sweeping every linting package found 281 violations across eight packages, not 207 in one. realm-server had 86 and runtime-common 74, both through the monorepo root config — host is root: true and inherits nothing, which is why fixing host alone left them untouched. Four configs needed the change, each being root: true: the monorepo root, host, ai-bot and boxel-icons.
Almost all of it was the drift you identified, in two shapes rather than one:
ban-typessplit intono-empty-object-type/no-unsafe-function-type/no-wrapper-object-types, andno-var-requiresbecameno-require-imports. Both originals are disabled here, so the successors now are too.no-unused-varschanged itscaughtErrorsdefault to reporting. That was every one of host's 57 and all 86 in realm-server — unusedcatchbindings.caughtErrors: 'none'restores the prior behaviour.no-unused-expressionsis newly recommended and flags two deliberate idioms in host: a bare member read that registers a tracked dependency, andcond && doThing()as a guard. Disabled rather than rewriting 39 intentional sites inside a version bump.
Two were genuine and are fixed rather than silenced — cardApi and ThemeCard were bound as values but only consumed as types. Renamed with the _ prefix this repo already uses, which keeps the await loader.import(...) whose side effect the test depends on.
One correction to my own earlier sweep, in case it appears in the history: five packages first showed as failing with zero problems. That was my script calling a lint:js those packages don't define, not a real failure.
Preview deploymentsHost Test Results 1 files ±0 1 suites ±0 2h 30m 8s ⏱️ - 7m 43s Results for commit d664171. ± Comparison against earlier commit 1909a17. Realm Server Test Results 1 files ±0 1 suites ±0 15m 58s ⏱️ + 2m 12s Results for commit d664171. ± Comparison against earlier commit 1909a17. |
v8 renamed or split several rules this repo already turns off, so upgrading re-enabled them under their new names and raised 281 violations across eight packages. Almost all are that drift rather than defects. `ban-types` became `no-empty-object-type`, `no-unsafe-function-type` and `no-wrapper-object-types`; `no-var-requires` became `no-require-imports`. Both originals are disabled here, so the successors are too. `no-unused-vars` now reports unused `catch` bindings by default, which is every instance of the 57 in host and the 86 in realm-server; `caughtErrors: 'none'` keeps the previous behaviour. `no-unused-expressions` is newly recommended and flags two idioms used on purpose in host — a bare member read that registers a tracked dependency, and `cond && doThing()` as a guard. Four configs, because each is `root: true` and inherits nothing: the monorepo root, host, ai-bot and boxel-icons. Two violations were genuine and are fixed rather than silenced: `cardApi` and `ThemeCard` were bound as values but only ever consumed as types. Renamed with the `_` prefix this repo already uses for the deliberately unused, which keeps the `await loader.import(...)` whose side effect the test relies on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same drift as the other configs: `docs-app` is `root: true`, takes `@typescript-eslint` from the catalog, and already disables `ban-types` and `no-var-requires` — so v8's replacements need disabling in the same two override blocks. Missed in the first pass because the sweep that found the other eight packages ran `lint:js`, and boxel-ui's eslint work happens under `pnpm lint`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Claude explanation
The catalog pinned 7.18.0, whose peer range is `eslint: ^8.56.0` with no typescript entry at all — this repo builds on TypeScript 5.9.3, which is why host's config carries `warnOnUnsupportedTypeScriptVersion: false`. 8.67.0 declares `typescript: >=4.8.4 <6.1.0`, so the version we compile with is a version the linter supports.It also makes type-aware linting possible for host. Host lints
.tsthrough this parser and.gtsthroughember-eslint-parser's own copy, which is on v8; while the two were different majors they fought over patching TypeScript, and enablingparserOptions.projecton both left 974 files reporting that the tsconfig did not include them and took 533s. On one major it is 0 files and 53s.This bump raises 207 existing violations in host from typescript-eslint's own rules, left for a follow-up so this change stays a version bump and reverting it stays cheap. Around 56 are config drift rather than defects: v8 splits
ban-typesintono-empty-object-type,no-unsafe-function-typeandno-wrapper-object-types, and the root config disablesban-types, so the successors need the same treatment. The rest areno-unused-varsandno-unused-expressions.Depends on eslint being declared in realm-server and software-factory: this bump changes how
@typescript-eslint/parser's*peer resolves an eslint, and without that declaration both packages pick up eslint 9 and fail demanding flat config.