-
Notifications
You must be signed in to change notification settings - Fork 197
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
Migrate to ESLint 9 and ESLint flat config #6615
Conversation
d70ee5f
to
5095b22
Compare
b8441d4
to
a7c40f2
Compare
'**/coverage/**/*', | ||
'docs/_build/*', | ||
'dev-server/static/**/*.js', | ||
], |
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.
Many of these files are already ignored by .gitignore
. Does ESLint not automatically respect that?
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, doesn't seem to. I even had to add a few more there that were not part of .eslintignore
and were getting scanned now (.tox
, .yarn
and .yalc
specifically).
|
||
// Scripts and configuration files | ||
{ | ||
files: ['**/*.js'], |
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.
I think I would prefer to use an allow-list rather than making these rules apply to all files and then attempting to exclude src/
. It depends how cumbersome maintaining the allow-list is.
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.
Agree, but for now let's just migrate the config as-is (as much as possible). We can adjust that later.
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.
OK
package.json
Outdated
"eslint-plugin-react": "^7.12.4", | ||
"eslint-plugin-react-hooks": "^4.0.4", | ||
"eslint": "^9.12.0", | ||
"eslint-config-hypothesis": "file:.yalc/eslint-config-hypothesis", |
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.
A .yalc
reference crept into the package.json file.
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.
Yeah, I'll change that once eslint-config-hypothesis
v3 is released with support for flat config.
'@typescript-eslint/no-non-null-assertion': 'off', | ||
'@typescript-eslint/no-this-alias': 'off', | ||
'@typescript-eslint/consistent-type-assertions': 'error', | ||
'@typescript-eslint/consistent-type-imports': '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.
I think we use the same rules in other TS-enabled projects. It would be good to centralize them in eslint-config-hypothesis where appropriate.
a7c40f2
to
068fb17
Compare
068fb17
to
d0df1ca
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6615 +/- ##
=======================================
Coverage 99.42% 99.42%
=======================================
Files 270 270
Lines 10175 10175
Branches 2419 2419
=======================================
Hits 10117 10117
Misses 58 58 ☔ View full report in Codecov by Sentry. |
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.
Thanks a lot for this 👍
Depends on hypothesis/frontend-toolkit#50
Update project to ESLint 9 and migrate ESlint config to flat format.
This migration is introducing a few side effects:
// eslint-disable
comments which do not disable anything have been removed, as they are now reported as warning.@typescript-eslint/...
packages replaced bytypescript-eslint
and updated to v8eslint-plugin-react-hooks
updated to v5eslint-plugin-mocha
updated to v10.5eslint-plugin-react
updated to v7.37eslint-config-hypothesis
updated to v3