Skip to content

Update to ESLint 9#7437

Closed
pjonsson wants to merge 1 commit intoTerriaJS:mainfrom
pjonsson:update-eslint
Closed

Update to ESLint 9#7437
pjonsson wants to merge 1 commit intoTerriaJS:mainfrom
pjonsson:update-eslint

Conversation

@pjonsson
Copy link
Copy Markdown
Contributor

@pjonsson pjonsson commented Jan 2, 2025

What this PR does

Depends on: #7411, #7382, #7381

Update to ESLint 9 since
ESLint 8 reached end-of-life
on 2024-10-05 and is no longer
maintained.

The main part of this PR is
the new flat configuration
file, which is quite close to
a 1:1 migration of the previous
non-flat config. The majority
of lines is the rules section,
which is the same as before
except for the now deprecated
rules that I removed.

Test me

Run yarn gulp lint.

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated relevant documentation in doc/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

Comment thread gulpfile.js
"0",
"--report-unused-disable-directives"
]);
runExternalModule(eslintExecutable, ["lib", "test", "--max-warnings", "0"]);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESLint 9 complains about unused directives even without the parameter, so drop the parameter.

The globs in the config decide what files are matched, so I dropped the --ext parameter as well. If we add an ESLint plugin that lints scss files in the future, that should work without modifications of the gulpfile.

Comment thread eslint.config.mjs
"react/no-danger": "warn",
"react/no-did-update-set-state": "error",
"react/no-will-update-set-state": "error",
"react/prop-types": "error",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The react/prop-types was disabled in #7382 (comment), but apparently doesn't trigger 1400+ warnings in ESLint 9, so re-enable the rule.

pjonsson added a commit to pjonsson/TerriaMap that referenced this pull request Feb 19, 2025
Update to ESLint 9 since
ESLint 8 reached end-of-life
on 2024-10-05 and is no longer
maintained.

This is the TerriaMap part of
TerriaJS/terriajs#7437
@pjonsson pjonsson changed the title Update to ESLint 9.17.0 Update to ESLint 9.20.1 Feb 19, 2025
pjonsson added a commit to pjonsson/TerriaMap that referenced this pull request Feb 20, 2025
Update to ESLint 9 since
ESLint 8 reached end-of-life
on 2024-10-05 and is no longer
maintained.

This is the TerriaMap part of
TerriaJS/terriajs#7437
@pjonsson
Copy link
Copy Markdown
Contributor Author

The corresponding TerriaMap PR is TerriaJS/TerriaMap#732

@pjonsson pjonsson force-pushed the update-eslint branch 2 times, most recently from 359744a to 09998cf Compare March 5, 2025 10:01
@pjonsson pjonsson mentioned this pull request Mar 5, 2025
4 tasks
@pjonsson pjonsson changed the title Update to ESLint 9.20.1 Update to ESLint 9 Mar 5, 2025
Update to ESLint 9 since
ESLint 8 reached end-of-life
on 2024-10-05 and is no longer
maintained.

The main part of this PR is
the new flat configuration
file, which is quite close to
a 1:1 migration of the previous
non-flat config. The majority
of lines is the rules section,
which is the same as before
except for the now deprecated
rules that I removed.
@zoran995
Copy link
Copy Markdown
Collaborator

@pjonsson, thanks for this, but I will close this PR and create a new one. There have been many changes to the codebase since this PR was created, and we need to follow a few different best practices.

@zoran995 zoran995 closed this Feb 25, 2026
@pjonsson
Copy link
Copy Markdown
Contributor Author

@zoran995 I converted the ESLint 8 configuration that was in the tree at the time of making this PR, and then I tracked the ESLint configuration updates that went into the main branch for 2 months and incorporated them into this PR. If the problem with this PR was that it implemented the same configuration as the existing ESLint 8 configuration, you could have saved yourself quite a bit of work by just making a comment on the PR about which parts you wanted changed and I would have happily made those changes.

@pjonsson
Copy link
Copy Markdown
Contributor Author

@zoran995 I realized I still have the local branch, here are the rules that changed in the ESLint 8 config since this PR was last rebased:

diff --git a/.eslintrc.js b/.eslintrc.js
index 86e1b9c93..6cce1c492 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -7,7 +7,6 @@ module.exports = {
     // https://github.com/typescript-eslint/typescript-eslint/blob/v6/packages/
eslint-plugin/src/configs/recommended.ts
     "plugin:@typescript-eslint/recommended",
     "plugin:react/recommended",
-    "plugin:react/jsx-runtime",
     "plugin:react-hooks/recommended",
     "prettier"
   ],
@@ -67,9 +66,6 @@ module.exports = {
     "react/self-closing-comp": "error",
     "react/jsx-no-undef": ["error", { allowGlobals: true }],

-    /* Ignore styled-components css property */
-    "react/no-unknown-property": ["error", { ignore: ["css"] }],
-
     "no-restricted-imports": [
       "error",
       {
@@ -131,7 +127,11 @@ module.exports = {
     "no-array-constructor": "error",
     "no-new-object": 1,
     "no-unneeded-ternary": 1 /* ECMAScript 6 */,
-    "prefer-const": "error"
+    "prefer-const": "error",
+    /* See https://stackoverflow.com/questions/64646248/eslintrc-js-for-react-17-and-jsx-without-import-react/64646593#64646593 */
+    "react/jsx-uses-react": "off",
+    "react/react-in-jsx-scope": "off",
+    "react/no-unknown-property": ["error", { ignore: ["css"] }]
   },
   overrides: [
     {

This looks like less than 30 minutes of work to fix, do you want me to rebase this PR to the latest main and update the changed rules?

@pjonsson pjonsson deleted the update-eslint branch February 26, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants