feat: support flat config format default for ESLint 9 #277

artus9033 · 2024-08-09T11:32:19Z

Summary

This PR solves #276 by adding flat configuration entrypoints that can be used with ESLint 9+, which deprecated the old eslintrc format and uses flat config by default.

Some plugins used under the hood by @callstack/eslint-config still were not ported to support flat config, thus partially the code performs proper changes either using @eslint/compat helpers where possible, or manually via manipulation of config / rules objects.

The changes involve:

adding new exports (*.flat.js) with flat config, and common configuration factories (*.factory.js) that are used both for new flat config entrypoints and old eslintrc-style entrypoints (unchanged file names, as they were); the actual code carrying rules / configuration was moved to *.factory.js files to share as much code without duplication as possible
updating the README with instructions for two variants: eslintrc-style config (ESLint < 9) & flat config (ESLint 9+)
reorganizing the structure of test project to a sub-monorepo inside test, carrying two workspace packages: eslint-v8 & eslint-v9, which are placeholders for scripts & dependency to ESLint v8 & v9; the yarn test command in the project performs a run of yarn test script for each of the workspaces of the monorepo in test/, causing both ESLint v8 & v9 to run on the same (untouched) files to test both for proper functioning of the flat config exports & the legacy, backwards-compatible eslintrc-style exports

The only pitfall is manual and/or @eslint/compat-assisted manipulation of imported configs / rules / plugins to adapt them to flat config format in a few places, which have been marked with // TODO: ... with proper information that these changes shall be removed in the future, as these packages provide support for the flat config format.

Test plan

yarn in the root directory
yarn test will run yarn in the test/ monorepo and will run both v8 & v9 ESLint on the test files

The test was run in this GH Action run.

Additionally, the new branch has been tested in a separate branch in the chartjs-plugin-dragdata project artus9033/chartjs-plugin-dragdata#142 & a functional ESLint run is presented in this CI run.

…on configuration code

…ig properly

… v8 & v9

retyui · 2024-08-09T12:48:26Z

I used the next command to compare configs on the next 2 versions

yarn eslint --print-config file.tsx

Do you know what is @ plugin ?
Can we keep prev. order of plugins ?
Why did you add @react-native plugin ?

also here is a diff of eslint rules https://www.diffchecker.com/HVG4ATyI/

+'@react-native/platform-colors': ['warn'],
+'no-empty-static-block': ['error'],
-'no-inner-declarations': ['error'],
-'no-new-symbol': ['error'],
+'no-new-native-nonconstructor': ['error'],
+'no-unused-private-class-members': ['error'],

did you want to introduce such changes ?

artus9033 · 2024-08-09T13:06:15Z

Do you know what is @ plugin ?

Can we keep prev. order of plugins ?

Thanks, I missed these changes - I will take a look at 1 & 2 to check if we can get this back.

also here is a diff of eslint rules https://www.diffchecker.com/HVG4ATyI/

+'@react-native/platform-colors': ['warn'],
+'no-empty-static-block': ['error'],
-'no-inner-declarations': ['error'],
-'no-new-symbol': ['error'],
+'no-new-native-nonconstructor': ['error'],
+'no-unused-private-class-members': ['error'],

Good catch, most likely some of the plugins we depend on must've changed their rules. I updated part of them since some introduced changes to support flat config. Do I assume properly that we want strictly the same rules in the final outcome that we had previously, or - in case these changes came from upgraded dependencies - would we want to e.g. keep the added rules and only restore the deletions?

Why did you add @react-native plugin ?

Would you tell, where do you see such change? It was already both in package.json and in the rules.

retyui · 2024-08-09T13:26:39Z

Would you tell, where do you see such change? It was already both in package.json and in the rules.

you right, (I haven't used the latest v8 config) that's why this plugin was missed

artus9033 · 2024-08-12T17:11:57Z

@retyui I tried to reproduce the diffs you reported, but on my side they work pretty much the same...

The following is a diff (full config object, not just rules, with stripped disk paths) between:

yarn eslint --print-config index.tsx > old-config.json run inside /test/ in a clone of the project before my changes
node node_modules/eslint8/bin/eslint.js --config=./eslint-v8/.eslintrc.js --print-config index.tsx > config-new-v8.json run in the changed repo inside /test/

https://www.diffchecker.com/6lpZreHP/

The only difference here seems to be "unicorn/template-indent": [0],, which most likely comes from an upgraded dependency. There don't seem to be the differences you posted back then and also the order of plugins is unchanged; ignored files difference comes straight from the changed config inside test/, so it is expected. Do you think we want to override this one to its default value?

The following is a diff between:

(as previously) yarn eslint --print-config index.tsx > old-config.json run inside /test/ in a clone of the project before my changes
node node_modules/eslint9/bin/eslint.js --config=./eslint-v9/eslint.config.mjs --print-config index.tsx > config-new-v9.json run in the changed repo inside /test/

https://www.diffchecker.com/3eMWAhwO/

The difference in rules is same as with ESLint v8 "unicorn/template-indent": [0], and also jest version is set to latest (which is expected as this comes straight from the config inside test/eslint-v9). Of course, the globals differ due to changes in ESLint 9.

The "@" plugin entry seems to come from here: https://github.com/eslint/eslint/blob/0dd38631b8dd60f93807e9cee1df3e752b86ab51/lib/config/default-config.js#L21
and is apparently what they call "root plugin" and I assume it should be there. It seems to me that this is now the default config shape for ESLint.

As to the order of other modules, I will inspect & update the code soon to maintain it.

retyui

Thank you, excellent work

artus9033 · 2024-08-12T17:50:43Z

Now (1dd27ea) the order of plugins is maintained:

diff for old vs new @ ESLint v8: https://www.diffchecker.com/P9h13ZZj/
diff for old vs new @ ESLint v9: https://www.diffchecker.com/O46wz5eC/

Thanks @retyui, can you just take a look & elaborate on what do we want to do with this new "unicorn/template-indent": [0]? Personally I would just leave it as-is as it seems to be introduced by an update to our dependencies.

Also, would you like me to bump up the version in package.json, I guess to a new minor, or will you do it later?

retyui · 2024-08-13T07:47:44Z

@thymikee Do you agree to bump minor version or it's a major release ?

thymikee · 2024-08-19T09:26:13Z

node.flat.js

@@ -0,0 +1,3 @@
+const createNodeConfig = require('./node.factory');
+
+module.exports = createNodeConfig(true);


instead of a boolean flag in an argument that controls whether this is a flat config or not, can we have two specialized functions instead of one? createFlatNodeConfig and createLegacyNodeConfig or something that makes sense semantically?

Sure, these names make sense - refactored in 10889e9

thymikee · 2024-08-19T09:27:48Z

react-native.flat.js

@@ -0,0 +1,3 @@
+const createRNConfig = require('./react-native.factory');
+
+module.exports = createRNConfig(true);


same boolean as argument situation here

Sure, refactored in 10889e9

thymikee · 2024-08-19T09:28:38Z

package.json

-    "eslint-plugin-prettier": "^5.0.0",
-    "eslint-plugin-promise": "^6.1.1",
-    "eslint-plugin-react": "^7.33.2",
+    "eslint-plugin-jest": "^28.7.0",


FYI, we usually we treat upgrading major version of deps as a major change

thymikee · 2024-08-19T09:29:16Z

test/package.json

    "@callstack/eslint-config": "link:..",
-    "eslint": "*",
-    "jest": "*",
+    "jest": "^29",


what's the reason for not being compatible with lower versions? If that was just for the version bump in the lockfile, you can invalidate the lockfile entry itself

Right, I must've changed it while testing to see if eslint v9 picks the version automatically when specified explicitly, but it didn't (that's why I placed an explicit settings object for jest in eslint.config.mjs). Reverted in 0bf92a6

thymikee

Let's roll with this change as a major version, as it mixes both new eslint functionality and bumps some deps' major versions :)

artus9033 · 2024-08-19T11:59:59Z

@retyui will you handle the version bump & publish?

artus9033 added 5 commits August 8, 2024 15:26

feat: export FlatConfig to support ESLint v9

09d92e5

refactor: add backwards compatibility for ESLint < 9 and extract comm…

ce0b74a

…on configuration code

chore: updated README.md

f2f7693

fix: adjust plugin flowtype in node.factory.js to work with flat conf…

f1a033e

…ig properly

fix(test): created configuration for tests to be run both with ESLint…

45e6945

… v8 & v9

artus9033 requested review from jaworek and retyui August 9, 2024 11:45

artus9033 changed the title ~~feat: support ESLint 9~~ feat: support flat config format default for ESLint 9 Aug 9, 2024

retyui approved these changes Aug 12, 2024

View reviewed changes

fix: maintain previous order of plugins in final configuration

1dd27ea

thymikee reviewed Aug 19, 2024

View reviewed changes

thymikee requested changes Aug 19, 2024

View reviewed changes

artus9033 added 2 commits August 19, 2024 11:42

fix: bring back compatibility with any jest version in test package

0bf92a6

refactor: export named functions from .factory.js source files

10889e9

thymikee approved these changes Aug 19, 2024

View reviewed changes

thymikee merged commit 100c778 into callstack:main Aug 19, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support flat config format default for ESLint 9 #277

feat: support flat config format default for ESLint 9 #277

artus9033 commented Aug 9, 2024 •

edited

Loading

retyui commented Aug 9, 2024

artus9033 commented Aug 9, 2024

retyui commented Aug 9, 2024

artus9033 commented Aug 12, 2024 •

edited

Loading

retyui left a comment

artus9033 commented Aug 12, 2024 •

edited

Loading

retyui commented Aug 13, 2024

thymikee Aug 19, 2024

artus9033 Aug 19, 2024

thymikee Aug 19, 2024

artus9033 Aug 19, 2024

thymikee Aug 19, 2024

thymikee Aug 19, 2024 •

edited

Loading

artus9033 Aug 19, 2024

thymikee left a comment

artus9033 commented Aug 19, 2024

		@@ -0,0 +1,3 @@
		const createNodeConfig = require('./node.factory');

		module.exports = createNodeConfig(true);

		@@ -0,0 +1,3 @@
		const createRNConfig = require('./react-native.factory');

		module.exports = createRNConfig(true);

feat: support flat config format default for ESLint 9 #277

feat: support flat config format default for ESLint 9 #277

Conversation

artus9033 commented Aug 9, 2024 • edited Loading

Summary

Test plan

retyui commented Aug 9, 2024

artus9033 commented Aug 9, 2024

retyui commented Aug 9, 2024

artus9033 commented Aug 12, 2024 • edited Loading

retyui left a comment

Choose a reason for hiding this comment

artus9033 commented Aug 12, 2024 • edited Loading

retyui commented Aug 13, 2024

thymikee Aug 19, 2024

Choose a reason for hiding this comment

artus9033 Aug 19, 2024

Choose a reason for hiding this comment

thymikee Aug 19, 2024

Choose a reason for hiding this comment

artus9033 Aug 19, 2024

Choose a reason for hiding this comment

thymikee Aug 19, 2024

Choose a reason for hiding this comment

thymikee Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

artus9033 Aug 19, 2024

Choose a reason for hiding this comment

thymikee left a comment

Choose a reason for hiding this comment

artus9033 commented Aug 19, 2024

artus9033 commented Aug 9, 2024 •

edited

Loading

artus9033 commented Aug 12, 2024 •

edited

Loading

artus9033 commented Aug 12, 2024 •

edited

Loading

thymikee Aug 19, 2024 •

edited

Loading