-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 #10762
base: master
Are you sure you want to change the base?
Migrate to ESLint 9 #10762
Conversation
Fixes 'overrideLogoSize' is never reassigned. Use 'const' instead
These comments came from a swizzled upstream component but never did anything in our codebase. ESLint 9 does not allow disable comments for rules that are not registered.
These were here because in the past we were applying mocha lint rules to files which contained no tests ESLint 9 now flags eslint-disable comments that aren't doing anythings
ESLint 9 now flags eslint-disable comments that aren't doing anything
and update the handful of files violating it
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/eslint-config-standard-jsx@11.0.0, npm/eslint-config-standard-react@13.0.0, npm/eslint-config-standard@17.1.0, npm/eslint-plugin-cypress@3.6.0, npm/eslint-plugin-icedfrisby@0.1.0, npm/eslint-plugin-no-extension-in-require@0.2.0, npm/eslint-plugin-node@11.1.0, npm/eslint-plugin-promise@6.6.0, npm/eslint-plugin-react-hooks@4.6.2, npm/eslint@8.57.1 |
|
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 entire file is likely to need a rebase to resolve conflicts before we can merge. I'll come back to this once we have something approved
globalConfig, | ||
commonJsConfig, | ||
nodeEsmConfig, | ||
frontendConfig, | ||
servicesConfig, | ||
mochaConfig, | ||
cypressConfig, | ||
serviceTestsConfig, | ||
jsDocConfig, |
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.
The way I've structured this, I've written lots of small ESLint Config objects and then composed them all at the end here.
If we wanted, we could actually split these all out into individual files. I haven't done it here, but I am happy to do it if we think it would be useful.
rules: { | ||
'react-hooks/rules-of-hooks': 'error', | ||
'react-hooks/exhaustive-deps': '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.
Now that we have migrated to docusaurus, we don't really have much custom react code any more although there is a little bit.
Looking over our old config, the only rule from eslint-plugin-react we actually had enabled was react/jsx-sort-props
so I decided to just ditch eslint-plugin-react completely.
I think there is some value in keeping these two rules because mistakes with hooks are common causes of infinite loops
Closes #10731
This PR is a bit of a monster.
Normally I'd try and make small scoped commits so it is easier to understand what is going on at each stage of the process, but in this PR there were so many moving parts changing at once it wasn't really possible to do that.
Because so much stuff was all changing at once and we also wanted to simplify where possible, I've taken a holistic look at the config and rebuilt it from the ground up. My objective was to arrive at something that replicates the spirit of what we started with (you'll notice there are not major code changes being made in this PR to account for lots of lint rule changes), but I wasn't aiming to arrive at a line by line replica of what we had before. There are several reasons why I've chosen to make changes rather than migrating things exactly, including:
Anyway, the upshot of all this is that the new config is not a faithful recreation of the old one and the PR is probably quite hard to review, but the actual outcome should be an improvement 🤞
Another thing to note is: Our new ESLint config is in some ways even more verbose than the old one. There are certainly a lot more lines of code in it. That said, I'm hoping that one of the things I've managed to do here is make it clearer and more logically separated into logical units. So hopefully it is a bit easier to read and understand, even if it is longer.