Repository navigation
chore(devops): move lint coverage to oxlint - #6076
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe PR adds Oxlint plugins and rules, including selected restrictions on ChangesOxlint configuration and codebase updates
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Possibly related issues
Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #6071 requires removing typescript-eslint and fully replacing ESLint with Oxlint. At the reviewed head, package.json still lists eslint and typescript-eslint, retains ESLint scripts, and… |
Remove the remaining ESLint and typescript-eslint dependencies, configuration, scripts, and lint-staged usage. Make Oxlint the complete lint implementation required by issue #6071. |
| Docstring Coverage | |
Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 45 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title "chore(devops): move lint coverage to oxlint" matches the main change. The PR moves retained ESLint rule coverage to Oxlint, enables the correctness category and import/no-duplicates, and ad… Description check The description states the PR moves retained lint coverage from ESLint to Oxlint. It lists the Oxlint configuration changes, the import deduplication work, the six optional-chaining fixes, and the ver… Out of Scope Changes check The Oxlint configuration, import deduplication changes, documented suppressions, and optional-chaining fixes all support the lint migration described for issue #6071. The reviewed summary shows no dem…Full details: Title check
-
Explanation
The title "chore(devops): move lint coverage to oxlint" matches the main change. The PR moves retained ESLint rule coverage to Oxlint, enables the correctness category and import/no-duplicates, and adds ignore patterns and engine overrides. The title is short, specific, and uses a conventional commit prefix.
Full details: Description check
-
Explanation
The description states the PR moves retained lint coverage from ESLint to Oxlint. It lists the Oxlint configuration changes, the import deduplication work, the six optional-chaining fixes, and the verification results. It is directly related to the changeset.
Full details: Out of Scope Changes check
-
Explanation
The Oxlint configuration, import deduplication changes, documented suppressions, and optional-chaining fixes all support the lint migration described for issue
#6071. The reviewed summary shows no demonstrated unrelated changes.
-
Full details: Linked Issues check
-
Explanation
Issue
#6071requires removingtypescript-eslintand fully replacing ESLint with Oxlint. At the reviewed head,package.jsonstill listseslintandtypescript-eslint, retains ESLint scripts, and runs ESLint from the lint and lint-staged workflows. The PR description also says ESLint remains for a later PR. The Oxlint migration in this PR does not complete the linked issue.Resolution
Remove the remaining ESLint and
typescript-eslintdependencies, configuration, scripts, and lint-staged usage. Make Oxlint the complete lint implementation required by issue#6071.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
New rules arrive to check the code
Imports take a clearer road
Custom tags still register
Tests assert what must be there
Oxlint joins the linting flow
Comment @coderabbitai help to get the list of available commands.
🤖 Claude Code ReviewVerdict: No blocking issues. Findings: 0 high, 0 medium, 0 low. Code review No issues found. Checked for bugs and CLAUDE.md compliance. What I checked:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🤖 Claude Code ReviewReviewed range: Verdict: No issues found. Checked for bugs and CLAUDE.md compliance. (0 critical / 0 major / 0 minor) This range changes two things:
Since the result is identical to |
|
/ready |
cd5e51f to
7e25fc8
Compare
🤖 Claude Code ReviewVerdict: No issues found. Checked for bugs and CLAUDE.md compliance. (0 critical / 0 major / 0 minor) Full re-review of the PR at
|
🤖 Claude Code ReviewReviewed range: Verdict: No issues found. Checked for bugs and CLAUDE.md compliance. (0 critical / 0 major / 0 minor) Most of this range is a merge of the base branch: the nation AI split, server join/roster changes, tests and Oxfmt reformatting of The PR's own change in this range:
|
Enable the correctness category and import/no-duplicates while keeping the four type-aware rules that ESLint never enforced off. Move the engine determinism restrictions to oxlint and its official ESLint-rules JS plugin, then fix the resulting lint findings. This includes six unsafe optional-chain cases that oxlint catches but ESLint misses, plus the import-deduplication gap from #6025. ESLint remains as a safety net for the removal follow-up.
Keep no-misused-spread, require-array-sort-compare, no-redundant-type-constituents, await-thenable, and no-useless-default-assignment explicitly disabled. These checks were not enabled by ESLint and are outside this migration. Remove their associated code and test changes, retaining the six unsafe optional-chain fixes and import deduplication.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolves #6073 > [!IMPORTANT] > Stacked PR 1 of 3. They need to be merged in order (1 first), not per se at the same time but is preferred. Part of the TypeScript 7 preparation tracked in #4982: update the OXC tooling before moving ESLint's remaining lint coverage to Oxlint and removing the TypeScript-ESLint dependency. | Package | Before | After | | --- | --- | --- | | `oxlint` | 1.75.0 | **1.88.0** | | `oxlint-tsgolint` | 7.0.2001 | **7.0.2004** (required by oxlint 1.88.0: `>=7.0.2003`) | | `oxfmt` | 0.48.0 | **0.73.0** | | `oxlint-plugin-eslint` | — | **1.88.0** (new) | `oxlint` 1.89.0 and `oxfmt` 0.74.0 were published on 2026-10-19, three days ago, so npm skips them per `min-release-age=7`. The versions above are the newest npm is allowed to install. ### oxfmt 0.48.0 → 0.73.0. Files changes would also have been needded on upgrading Prettier 3.8.3 to 3.9.0 Formatting-only change: **150 files** reformatted (148 `.ts`, 2 `.md`). These changes would eventually have been needed when upgrading Prettier to 3.9.0 or above, even if this repo would not have migrated to oxfmt. The TypeScript changes are embedded Lit template expressions. The layout change was introduced by Oxfmt 0.61.0 through Oxc PR [#24972](oxc-project/oxc#24972), “stop preserving indent for embedded template expressions.” It was released in Oxfmt 0.61.0. The old Prettier output in this repository is explained by its locked version, **3.8.3** at the time of #3871. Prettier 3.9.0 formats this expression with the newer layout too. There was no config disabling it: the pre-migration `.prettierrc` did not set `embeddedLanguageFormatting`, so Prettier used its default `auto` behavior. ```diff - ${gamePortal.isOnGamePortal() - ? this.renderPortalSignIn() - : this.renderLoginOptions()} + ${ + gamePortal.isOnGamePortal() + ? this.renderPortalSignIn() + : this.renderLoginOptions() + } ``` No logic changes. The import-sorting config in `.oxfmtrc.json` is unaffected: `sortImports` produces byte-identical output between 0.48.0 and 0.73.0 with this repo's config, so the ordering tuned in #6026 is unchanged. oxfmt is now about twice as fast: `oxfmt --check` over the repo went from **4.26s** to **2.09s**. ### oxlint-plugin-eslint `oxlint-plugin-eslint` is part of the official `oxc-project/oxc` monorepo. It packages ESLint's own rule implementations as an Oxlint JS plugin, which is the documented way to use rules Oxlint does not implement natively yet. The follow-up #6076 uses it to replace `eslint/no-restricted-syntax`. Its version tracks Oxlint, so the packages are bumped together. ## Verification - `oxfmt --check` — clean - Oxlint with the existing configuration — clean ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory ## Please put your Discord username so you can be contacted if a bug or regression is found: tryout33 --------- Co-authored-by: Josh Harris <josh@openfront.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
59ab090 to
37c89f8
Compare
🤖 Claude Code ReviewVerdict: No issues found. Checked for bugs and CLAUDE.md compliance. (0 critical / 0 major / 0 minor) Full re-review of the PR at
|
Celant
left a comment
Celant
left a comment
There was a problem hiding this comment.
Looks good. I checked that the DetMath restrictions still fire through the eslint plugin, and every dropped side-effect import still has a value import that registers the element.
github-project-automation
Bot
moved this from Triage
to Final Review
in OpenFront Release Management
github-project-automation
Bot
moved this from Final Review
to Complete
in OpenFront Release Management
Resolves #6071
Refs #6025
Important
Stacked PR 2 of 3. They need to be merged in order (1 first), not per se at the same time but is preferred.
Follow-up to #3866 and the OXC toolchain upgrade in #6074. This moves the lint coverage we want to retain from ESLint to Oxlint. ESLint remains in place for the next PR, which removes its remaining dependencies and configuration.
Oxlint configuration
correctnesscategory and theimportplugin'sno-duplicatesrule.typescript/no-explicit-anyoff, withtypescript/prefer-nullish-coalescing,eqeqeq,no-case-declarations, and the existing unused-variable settings enabled.typescript/no-floating-promisestypescript/restrict-template-expressionstypescript/unbound-methodtypescript/no-base-to-stringtypescript/no-misused-spreadtypescript/require-array-sort-comparetypescript/no-redundant-type-constituentstypescript/await-thenabletypescript/no-useless-default-assignment.claude/**to Oxlint's ignore patterns to match ESLint's existing ignore.eslint/no-restricted-propertiesis native to Oxlint and preserves theMath.*restrictions. The non-nativeno-restricted-syntaxrule runs through the officialoxlint-plugin-eslintpackage added in #6074. Both guards retain their existing paths and behavior.Import deduplication (#6025)
Follow-up to #6026. In this PR because it is part of oxlint. Enable
import/no-duplicatesto restore import deduplication previously supplied byprettier-plugin-organize-imports. Oxlint's rule has a working auto-fix and correctly handles type imports. Also consideredeslint/no-duplicate-imports, but that one has no auto-fix here and reports intentional type/value import pairs as duplicates.Safe fixes merge duplicate imports. A small number of side-effect imports must remain separate from type-only imports: TypeScript erases the named type import at runtime, while the side-effect import registers a custom element. Commented those cases with Oxlint suppressions with explanations, so these imports aren't removed.
The one remaining
prettier-plugin-organize-importsbehavior gap is sorting named specifiers inside one import declaration (for example,import { zeta, alpha }). Neither Oxfmt'ssortImportsnor Oxlint currently does this. Existing imports are already sorted from the TypeScript organize-imports era, but they will not be automatically re-sorted going forward.Findings fixed
Enabling
correctnesssurfaced sixeslint/no-unsafe-optional-chainingcases. These are genuine runtime risks that ESLint 10 missed because TypeScriptasassertions interrupt the AST pattern its rule checks. They are fixed rather than suppressed.Verification
oxlintandeslint— cleanoxfmt --check— cleannpm run typecheck— passesnpm test— 9,416 passed, 1 skipped. Five assertions expecten-USnumber grouping, while this environment defaults tonl-NL(e.g.1.150vs1,150).Local lint timing with this configuration: ESLint 45.3s, Oxlint 4.4s (about 10× faster).
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
tryout33