chore: replace eslint with oxlint - #431
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp8HUuTkriy2qNUP2GNgcs
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good — this is a mechanical lint-toolchain swap (eslint → oxlint) plus the small source fixes it surfaced. Reviewed the .oxlintrc.json config, package.json/lint-staged script changes, the void/as const/import fixes in src/, and the yarn.lock diff (removed eslint packages, added the legitimate oxc-project oxlint/oxlint-tsgolint native-binding packages). Checked the log-util.ts message-join change specifically: `${message}` on an array already calls Array.prototype.join(',') under the hood, so .join(',') is behaviorally identical, not a functional change. The nock.isDone() assertion fix in spec/index.spec.ts is a genuine test-correctness improvement (previously asserted nothing) and the PR author reports all 93 tests still pass.
Extended reasoning...
Overview
This PR replaces the project's (unused) eslint setup with oxlint: deletes .eslintrc.json, adds .oxlintrc.json, updates package.json scripts/devDependencies/lint-staged config, regenerates yarn.lock, and applies the small source-level fixes the new linter surfaced across src/Queue.ts, src/index.ts, src/utils.ts, src/utils/checks-util.ts, src/utils/log-util.ts, typings/ambient.d.ts, and two spec files, plus a trivial vitest.config.ts cleanup.
Security risks
No security-sensitive code paths (auth, crypto, permissions) are touched. The only dependency-security angle is the new oxlint/oxlint-tsgolint devDependencies and their platform-native binding packages in yarn.lock; these match the well-known oxc-project naming/versioning scheme and are devDependencies only (not shipped to production), so risk is low. A socket-security bot comment appeared on the PR timeline (its content wasn't available to this review), but nothing in the lockfile diff looked anomalous.
Level of scrutiny
Config/tooling-only PR with mechanical, easily verifiable source changes (fire-and-forget void, as const literal-type narrowing, dropping unused catch bindings, a require → import swap with an added ambient module declaration, and a template-literal-to-.join change that I confirmed is behavior-preserving for arrays). This warrants a lighter level of scrutiny than a feature/logic PR, and I was able to reason through every functional-looking diff line directly.
Other factors
The test-suite fix (making the dangling nock.isDone() expectation actually assert .toBe(true)) is a real improvement to test rigor, and the PR author states all 93 tests still pass locally, which is consistent with the low-risk nature of the surrounding fixes. No outstanding CHANGES_REQUESTED or unaddressed reviewer objections are indicated in the timeline metadata provided.
Resolves the conflict with #432/#433 (stacked-PR backports) and brings main's new code under the oxlint rules: - src/index.ts: keep main's single-PR/stack dispatch in the manual backport command and mark both fire-and-forget calls with `void` (typescript/no-floating-promises), as the branch did for the pre-existing call. Same for the new backportStackToLabel loop. - src/index.ts: stringify caught errors explicitly in the new log templates (typescript/restrict-template-expressions). - spec/index.spec.ts: drop the unused backportStackToBranch import. - spec/index.spec.ts: the branch turned the previously inert `expect(nock.isDone(), ...)` into a real assertion. Capture nock.pendingMocks() before nock.cleanAll() so a failing test cannot leak persisted interceptors into later tests, and remove the labels interceptor of the stacked-label test, which no code path requests for a `labeled` event on a PR targeting the default branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dp8HUuTkriy2qNUP2GNgcs
3 participants
Requested by Samuel Attard · Slack thread
Before: eslint 8 +
standard-with-typescriptwere declared as devDependencies with a legacy.eslintrc.json, but nothing ever ran them;yarn lintwas prettier-only.After:
yarn lintrunsoxlint --type-aware(viaoxlint-tsgolint) and then the existing prettier check, so CI and the pre-commit hook actually lint; eslint and its five plugins/configs are gone.eslint,@typescript-eslint/eslint-plugin,eslint-config-standard-with-typescript,eslint-plugin-import,eslint-plugin-n,eslint-plugin-promise; addedoxlint@^1.81.0,oxlint-tsgolint@^7.0.2001. Deleted.eslintrc.json..oxlintrc.jsonmirroring chore: replace eslint with oxlint github-app-auth-action#174 (correctness category,typescript/import/node/promise/vitestplugins,no-floating-promises, etc.).typescript/no-require-importsis off forspec/**(JSON fixtures) andscripts/**(CommonJS postinstall script);no-non-null-assertioniswarn(9 existing sites).lint= oxlint + prettier check, newlint:fix; lint-staged now runs oxlint and prettier on staged files (the old entry ranprettier --write **/*.ts, ignoring the staged list).void), 7'x' as 'x'->as const, unused import/catch bindings,String->string,require('what-the-diff')->importwith an ambient declaration, a redundant triple-slash reference, anunknown[]template interpolation, and a conditionalexpectinoperations.spec.ts.afterEachcalledexpect(nock.isDone(), msg)with no matcher, so the "all interceptors used" check never asserted anything. It now.toBe(true)(all 93 tests still pass), andbeforeEachawaitsrobot.load(trop).Verified locally with
yarn lint,yarn build, andyarn test(8 files, 93 tests passing).Drops vs.
standard-with-typescriptnot carried over: the style-only rules (explicit-function-return-type,strict-boolean-expressions,naming-convention,member-delimiter-style, etc.) since prettier owns formatting here and these were never enforced.🤖 Generated with Claude Code
https://claude.ai/code/session_01Dp8HUuTkriy2qNUP2GNgcs
Generated by Claude Code