Linting refactor and enable GitHub actions #3179

nellh · 2024-10-22T23:35:34Z

The linter hasn't been run on CI in a while and some of the rules in use do not make sense for the current version of the project. This PR replaces the linting config and includes all the required fixes for the new ruleset.

This also adds the new react-compiler beta linter, which can do a better job of catching misuse of React and in the future may be useful for automatic memoization of component rendering.

TODO:

About 100 errors left
GitHub action

Annotated or removed as needed.

codecov · 2024-10-22T23:40:22Z

Codecov Report

Attention: Patch coverage is 56.55577% with 222 lines in your changes missing coverage. Please review.

Project coverage is 43.09%. Comparing base (0bf65b0) to head (dc70912).
Report is 39 commits behind head on master.

Files with missing lines	Patch %	Lines
packages/openneuro-cli/src/download.js	3.70%	26 Missing ⚠️
...euro-app/src/scripts/pages/admin/flagged-files.jsx	0.00%	18 Missing ⚠️
packages/openneuro-client/src/client.js	11.76%	15 Missing ⚠️
...ts/dataset/mutations/delete-anonymous-reviewer.tsx	0.00%	8 Missing and 1 partial ⚠️
packages/openneuro-server/src/handlers/comments.ts	0.00%	7 Missing ⚠️
...cripts/dataset/files/viewers/file-viewer-nifti.tsx	0.00%	5 Missing ⚠️
...src/scripts/dataset/fragments/copyable-tooltip.jsx	0.00%	4 Missing and 1 partial ⚠️
.../openneuro-server/src/graphql/resolvers/dataset.ts	16.66%	5 Missing ⚠️
packages/openneuro-app/src/scripts/app.tsx	0.00%	3 Missing and 1 partial ⚠️
...-components/src/activity-slider/ActivitySlider.tsx	0.00%	4 Missing ⚠️
... and 78 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3179      +/-   ##
==========================================
+ Coverage   42.84%   43.09%   +0.24%     
==========================================
  Files         549      547       -2     
  Lines       36187    36163      -24     
  Branches     1132     1128       -4     
==========================================
+ Hits        15506    15584      +78     
+ Misses      20479    20379     -100     
+ Partials      202      200       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

effigies

Partial review. Will finish tomorrow.

effigies · 2024-10-24T02:26:33Z

eslint.config.mjs

+    "packages/openneuro-app/pluralize-esm.js",
+    "packages/openneuro-app/src/scripts/utils/schema-validator.js",


Might be helpful to future readers to know why we're ignoring these. I think this is the reason?

Suggested change

"packages/openneuro-app/pluralize-esm.js",

"packages/openneuro-app/src/scripts/utils/schema-validator.js",

// Generated from external sources

"packages/openneuro-app/pluralize-esm.js",

"packages/openneuro-app/src/scripts/utils/schema-validator.js",

effigies · 2024-10-24T02:27:48Z

eslint.config.mjs

+    "packages/**/*.ts",
+    "packages/**/*.tsx",
+    "packages/**/*.jsx",
+    "packages/**/*.js",


Is it reasonable for this thing to lint itself?

Suggested change

"packages/**/*.js",

"packages/**/*.js",

"eslint.config.mjs",

effigies · 2024-10-24T02:31:15Z

packages/openneuro-app/src/scripts/dataset/fragments/dataset-history.jsx

@@ -80,7 +79,6 @@ const DatasetHistory = ({ datasetId }) => {
                </div>
                <div className="col-lg col col-2 grid actions">
                  <Revalidate datasetId={datasetId} revision={commit.id} />
-                  <UpdateRef datasetId={datasetId} revision={commit.id} />


Why are we removing this feature? I have occasionally used it.

The mutation it uses hasn't worked since 2022. It would move the HEAD reference in MongoDB but we no longer do that, always following the git repo HEAD reference instead. The git version would be more destructive and I think it's better handled via the git interface itself.

Do we allow force-pushes to revert? If not, then it's just logging into the worker and manually doing the job.

Anyway, if it hasn't worked, then this is no loss, so no further objections.

effigies · 2024-10-24T12:57:22Z

Finished review. Main concern is dropping the UpdateRef mutation, but it's not an indispensable feature.

nellh added 20 commits October 22, 2024 13:59

lint: Upgrade to eslint 9 and refactor to enable simpler linting config

d5c0338

fix(server): Use Object.hasOwn over hasOwnProperty

79389b9

fix(app): Disable linting for react.d.ts type helper in tests

e6b3060

fix(server): Prefix unreferenced validateUrl vars with _

58eb99b

chore(server): Use mongoose types as a type import

7d996c7

chore(lint): Fix object prototype hasOwnProperty usage

0928d94

chore(lint): Use object over any for model definitions

a0e36c9

feat(graphql-client): Only warn once on mismatched versions

00b718e

chore(lint): Don't suppress console from the CLI upload steps

7fbda3e

chore(lint): Remove cjs import for node-mailjet

7e1eda7

chore(lint): Indexer should print console messages

feb1eaa

chore(lint): Document issues with Readable interface types

27af188

chore(lint): Enable checking for schema.ts

97d130e

chore(lint): Fix linter configuration for console and unused vars

81240a6

chore(lint): Fix types imported as values

beada40

fix(app): Error handling for FlaggedFileContainer query errors

ccf90e1

chore(lint): Cleanup unused variables

f0ab199

Annotated or removed as needed.

refactor(app): Drop deprecated updateRef

9097564

chore(lint): React type import fixes

34a07ca

chore(lint): Linting and error handling fixes

508a1dc

nellh added 2 commits October 23, 2024 10:41

fix(app): Remove no-op value mutation TwoHandleRange change handlers

cf5956d

chore(lint): Remaining eslint refactor fixes

eabf4dc

nellh force-pushed the eslint-fixes branch from f24aa96 to eabf4dc Compare October 23, 2024 21:57

ci: Add ESLint step

dc70912

nellh marked this pull request as ready for review October 23, 2024 22:09

effigies reviewed Oct 24, 2024

View reviewed changes

nellh merged commit 1573f51 into master Oct 24, 2024
13 of 15 checks passed

nellh deleted the eslint-fixes branch October 24, 2024 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linting refactor and enable GitHub actions #3179

Linting refactor and enable GitHub actions #3179

nellh commented Oct 22, 2024 •

edited

Loading

codecov bot commented Oct 22, 2024 •

edited

Loading

effigies left a comment

effigies Oct 24, 2024

effigies Oct 24, 2024

effigies Oct 24, 2024

nellh Oct 24, 2024

effigies Oct 24, 2024

effigies commented Oct 24, 2024

		"packages/openneuro-app/pluralize-esm.js",
		"packages/openneuro-app/src/scripts/utils/schema-validator.js",

Linting refactor and enable GitHub actions #3179

Linting refactor and enable GitHub actions #3179

Conversation

nellh commented Oct 22, 2024 • edited Loading

codecov bot commented Oct 22, 2024 • edited Loading

Codecov Report

effigies left a comment

Choose a reason for hiding this comment

effigies Oct 24, 2024

Choose a reason for hiding this comment

effigies Oct 24, 2024

Choose a reason for hiding this comment

effigies Oct 24, 2024

Choose a reason for hiding this comment

nellh Oct 24, 2024

Choose a reason for hiding this comment

effigies Oct 24, 2024

Choose a reason for hiding this comment

effigies commented Oct 24, 2024

nellh commented Oct 22, 2024 •

edited

Loading

codecov bot commented Oct 22, 2024 •

edited

Loading