-
Notifications
You must be signed in to change notification settings - Fork 35
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 new ESLint config format and fix some lint errors #2412
Conversation
Fixed to avoid noise from semicolon changes when rewriting import statements. It turned out to be difficult to omit the React changes entirely so I left them in (it's easy to only do the React import changes and none of the other import changes if you'd prefer that, but they're order-dependent). |
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.
tested yesterday, LGTM
69f8511
to
367efd9
Compare
Rebased over main, resolving two minor merge conflicts |
367efd9
to
f118fbf
Compare
Rebased again 🙂 and since I see that this has been approved (thanks!) I'm gonna go ahead and merge it. |
I noticed that I wasn't able to run eslint on the project, either from the CLI or from within my text editor. Attempting either would spew parse errors (it seemed babel wasn't configured to understand JSX when invoked this way).
yarn build
appeared to run eslint successfully, but only on files that had been modified (compared to the current git HEAD).I think this had something to do with (1) using the old
.eslint.json
config syntax instead of the newereslint.config.js
, and (2) having a second (conflicting?) eslint configuration specified inpackage.json
.This PR ports the
.eslint.json
file in the repo to the new format, while maintaining (I think) functional equivalence, and removes theeslintConfig
section inpackage.json
. With these changes made, I'm now able to runeslint
from the command line.Once I made that change I noticed a ton of unused import errors, so I fixed those. Some were about
React
(as of React 17 there's a new JSX transform and it's no longer necessary forReact
to be in scope in order to use JSX). Others were about miscellaneous unused imports. I split them into two commits for clarity.I think it'd be nice later on to enable some of the recommended eslint rules that are currently suppressed, but this diff is already pretty big so I'll save that for later.