-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[core] Upgrade eslint-config-airbnb-typescript #34642
Conversation
|
If the Having more consistency in how we create components with |
Just noticed this PR: #34637 Which is also present in the eslint output:
There's several more of these caught by the upgraded setup |
Signed-off-by: Jan Potoms <[email protected]>
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.
It looks like we're finally close to merging this :)
Signed-off-by: Jan Potoms <[email protected]>
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 it's good to go! Thanks for working on this!
Signed-off-by: Jan Potoms <[email protected]>
Signed-off-by: Jan Potoms <[email protected]>
Tried to upgrade
eslint-config-airbnb-typescript
so we could use it in Toolpad.It needed a few updates to the setup. Mainly it now requires
eslint-config-airbnb
to be set up separately.The new version generates a lot of new warnings, I already ignored some of them that seem low priority to cut down on the noise, but we may want to review the remaining ones. Some of them seem legit problems. Running
yarn eslint --fix
gets a lot of them sorted, but I didn't push that yet to keep the noise down.The main problems seem to center around:
react/function-component-definition
: This seems to discourage declaring components in an anonymous way. It improves the debugging experience and in some setups it may improve the hot reloading. (most of them are auto-fixable)react/jsx-no-useless-fragment
: Some examples seem to have a top-level fragment for no reason. (most of them are auto-fixable)react/jsx-no-constructed-context-values
: Warns for patters in React context that may cause performance issues.Worth investing more time in this? The
react/jsx-no-constructed-context-values
ones seem interesting ones to solve IMOTo do before we can marge this:
react/jsx-no-constructed-context-values
for TreeView? It seems to require a lot of refactoring to stabilize it..test.ts
and.spec.ts
and fix issues [core] ESLint fixes for tests #34924