311-data icon indicating copy to clipboard operation
311-data copied to clipboard

Enable ESLint on all js files

Open nichhk opened this issue 2 years ago • 6 comments

Overview

Many of our javascript files say /* eslint-disable */ at the top, preventing the linter from running on them. This is a bad idea since the linter helps to identify readability issues and bugs.

Find the full list of places where we're disabling ESLint here: https://github.com/hackforla/311-data/search?q=%22%2F*+eslint-disable+*%2F%22. Note that we do NOT care about code in client/v1!

After we enable ESLint on all the files, we should enable simple-import-sort to sort our imports.

Action Items

  • [ ] Remove /* eslint-disable */ from all files and fix the resulting errors. We should break this up into multiple PRs--one per file, or one per directory.
  • [ ] Enable simple-import-sort and fix resulting errors

nichhk avatar Jul 07 '22 01:07 nichhk

@funbunch and @ardada2468, feel free to work on this issue if you have extra time. Please note the files that you'll be fixing before starting on them here, so that we can avoid duplicate work!

nichhk avatar Aug 02 '22 01:08 nichhk

ran the following to find places where /* eslint-disable */ is located (excluding v1, node_modules directories and the compiled code bunde.js):

grep -Rin --exclude-dir={v1,node_modules} --exclude=bundle.js eslint-disable

here are the locations and line numbers. hope this helps

client/components/main/Reports.jsx:1:/* eslint-disable react/self-closing-comp */

client/components/main/Reports.jsx:31: /* eslint-disable consistent-return */

client/components/main/CookieNotice.jsx:1:/* eslint-disable */

client/components/main/Desktop/TypeSelector/index.js:1:/* eslint-disable */

client/components/common/MultiSelect/GroupedMultiSelect.jsx:39: // eslint-disable-next-line

client/components/Map/Map.jsx:1:/* eslint-disable */

client/components/Map/mapColors.js:1:/* eslint-disable */

client/components/Map/constants.js:1:/* eslint-disable */

client/components/Map/districts.js:1:/* eslint-disable */

client/components/Map/index.js:1:/* eslint-disable */

client/components/Map/layers/BoundaryLayer.js:1:/* eslint-disable */

client/components/Map/layers/RequestsLayer.js:1:/* eslint-disable */

client/components/Map/layers/AddressLayer.js:1:/* eslint-disable */

client/components/Map/controls/MapOverview.jsx:1:/* eslint-disable */

client/components/Map/controls/RequestsBarChart.jsx:1:/* eslint-disable */

client/components/Map/controls/MapRegion.jsx:1:/* eslint-disable */

client/components/Map/controls/MapSearch.jsx:1:/* eslint-disable */

client/components/Map/controls/MapLayers.jsx:1:/* eslint-disable */

client/components/Map/controls/MapMeta.jsx:1:/* eslint-disable */

client/components/Map/controls/RequestsDonut.jsx:1:/* eslint-disable */

client/components/Map/RequestDetail.jsx:1:/* eslint-disable react/prop-types */

client/components/Map/geoUtils.js:1:/* eslint-disable */

client/App.jsx:42: {/* eslint-disable-next-line react/jsx-props-no-spreading */}

client/index.js:1:/* eslint-disable react/jsx-filename-extension */

client/redux/tempTypes.js:1:/* eslint-disable */

client/redux/sagas/data.js:1:/* eslint-disable */

client/utils/checkEnv.js:1:/* eslint-disable no-console */

edwinjue avatar Aug 18 '22 00:08 edwinjue

Thanks for this list Edwin! I will claim everything in "client/components/Map/controls/".

nichhk avatar Aug 21 '22 17:08 nichhk

Glad I could help! However, I notice branch 1270-enable-eslint-on-all-js-files is 100 commits behind dev. Should we still checkout this branch to make the changes or should we go off a new branch?

edwinjue avatar Aug 23 '22 04:08 edwinjue

Since I'm already working on the contact form, I'll lint everything in client/components/main/

edwinjue avatar Aug 23 '22 21:08 edwinjue

Linted everything except the following:

client/components/Map/* client/redux/tempTypes.js:1:/* eslint-disable */ client/components/common/MultiSelect/GroupedMultiSelect.jsx:39: // eslint-disable-next-line client/utils/checkEnv.js:1:/* eslint-disable no-console */

Changes have been pushed up to this branch. Will try to take a look at some of the other Map files when I find more free time

edwinjue avatar Aug 27 '22 04:08 edwinjue

Hey @edwinjue Do you have an update for us on this issue?

Please update:

  • Progress:
  • Blockers:
  • Availability:
  • ETA:

Thanks!

mc759 avatar Dec 13 '22 03:12 mc759