maproulette / maproulette3

MapRoulette, the micro-tasking tool for OpenStreetMap
https://maproulette.org
MIT License
128 stars 35 forks source link

Migrate to new ESLint config format and fix some lint errors #2412

Closed jake-low closed 2 months ago

jake-low commented 3 months ago

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 newer eslint.config.js, and (2) having a second (conflicting?) eslint configuration specified in package.json.

This PR ports the .eslint.json file in the repo to the new format, while maintaining (I think) functional equivalence, and removes the eslintConfig section in package.json. With these changes made, I'm now able to run eslint 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 for React 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.

jake-low commented 3 months ago

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).

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 19.44444% with 29 lines in your changes missing coverage. Please review.

Project coverage is 23.95%. Comparing base (58b2708) to head (f118fbf). Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...ets/ProjectManagersWidget/ProjectManagersWidget.js 0.00% 3 Missing :warning:
src/components/ActivityListing/ActivityListing.js 0.00% 2 Missing :warning:
...ts/AdminPane/Manage/ChallengeCard/ChallengeCard.js 0.00% 2 Missing :warning:
...lengeOwnerLeaderboard/ChallengeOwnerLeaderboard.js 0.00% 2 Missing :warning:
.../components/ChallengeProgress/ChallengeProgress.js 50.00% 2 Missing :warning:
...ustom/RJSFFormFieldAdapter/RJSFFormFieldAdapter.js 0.00% 2 Missing :warning:
.../components/EnhancedMap/LayerToggle/LayerToggle.js 0.00% 2 Missing :warning:
.../components/ActivityListing/ActivityDescription.js 0.00% 1 Missing :warning:
...dminPane/Manage/ChallengeCard/ChallengeControls.js 0.00% 1 Missing :warning:
.../AdminPane/Manage/ProgressStatus/ProgressStatus.js 0.00% 1 Missing :warning:
... and 11 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2412 +/- ## ========================================== - Coverage 23.95% 23.95% -0.01% ========================================== Files 649 649 Lines 22482 22483 +1 Branches 6894 6908 +14 ========================================== Hits 5385 5385 - Misses 14311 14312 +1 Partials 2786 2786 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jake-low commented 2 months ago

Rebased over main, resolving two minor merge conflicts

jake-low commented 2 months ago

Rebased again 🙂 and since I see that this has been approved (thanks!) I'm gonna go ahead and merge it.