-
Notifications
You must be signed in to change notification settings - Fork 2.9k
refactor: Migrate from ESLint to oxlint and biome #40731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor: Migrate from ESLint to oxlint and biome #40731
Conversation
9695b13 to
fc89dac
Compare
fc89dac to
13533f7
Compare
|
Oof just a few changes 😅 I'm gonna say let's hold off until after v43 (unless we need it for the ESM migration)? |
|
Hey, I haven't done double-check yet, but feel free to comment anyways |
Don't think we need for ESM. |
| @@ -1,10 +1,10 @@ | |||
| .md-header .md-header__button.md-logo { | |||
| padding: 0; | |||
| padding: 0; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this change, looks wrong to me
| */ | ||
| function re2() { | ||
| return require('re2'); | ||
| return require("re2"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no indention change and single quote usage!
| if (typeof e === 'string') { | ||
| return e; | ||
| } | ||
| return JSON.stringify(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be done separately
| ...err, | ||
| }; | ||
| const response: Record<string, unknown> = {}; | ||
| for (const key of Object.keys(err)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are these changes necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file violates "no conditionals in tests", I just asked to reorganize it somehow.
- Restore vi.unmock for mutex in race condition tests - Use early exit pattern in docker versioning isCompatible
13533f7 to
4df1296
Compare
viceice
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quotes are wrongly changed from single to double 😕
| @@ -1,9 +1,9 @@ | |||
| import type { HostRule } from '../types/index.ts'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs fix 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should extract a package from it, like i've done for the eslint rules 😁
| "jsPlugins": ["./tools/lint/rules.js"], | ||
| "categories": {}, | ||
| "rules": { | ||
| "constructor-super": "error", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't there some preset support? so we don't need to configure them all manually 🤔
|
@zharinov |
@marklai1998 We don't need just decent set of rules, we want to supersede all the existing eslint rules. If there is at least one rule covered by biome and not covered by oxlint, it's worth to have both. And we have such rule: it's Another benefit is imports ordering which seems to be more stable with biome at the moment. I could be wrong on this one, though. Anyways, compared to the |
Changes
Replaces ESLint with faster native linters:
Removed (~10 packages)
eslintand all plugins (@typescript-eslint/*,eslint-plugin-import-x,eslint-plugin-promise,@vitest/eslint-plugin,@containerbase/eslint-plugin)eslint.config.mjsAdded
.oxlintrc.jsonwith comprehensive rule configurationno-tools-importandtest-root-describerulesbiome.jsonfornoUndeclaredDependencies,useNamingConvention,useConsistentObjectDefinitions, andorganizeImportsScripts
oxlint/oxlint-fix/oxlint-ci- oxlint commandsbiome/biome-fix/biome-ci- biome commandsContext
AI assistance disclosure
Documentation (please check one with an [x])
How I've tested my work (please select one)