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
|
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
I'd say it's helpful to have all the config objects in one place, let's keep as is. 👍🏻
| rules: { | ||
| 'react-hooks/rules-of-hooks': 'error', | ||
| 'react-hooks/exhaustive-deps': 'error', | ||
| }, |
There was a problem hiding this comment.
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
PyvesB
left a comment
PyvesB
left a comment
There was a problem hiding this comment.
Looks reasonable to me, thanks for tackling this!
chris48s
force-pushed
the
10731-eslint
branch
from
341d5b7 to
d0e314b
Compare
2 participants
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.