chore: use eslint-plugin-eslint-plugin flat configs #17204

mdjermanovic · 2023-05-20T22:37:53Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Upgrades eslint-plugin-eslint-plugin to the latest version and switches to using its flat config exports (eslint-community/eslint-plugin-eslint-plugin#347).

What changes did you make? (Give an overview)

Updated package.json to require ^5.1.0 of eslint-plugin-eslint-plugin, updated eslint.config.js to use the new flat configs of eslint-plugin-eslint-plugin and fixed a few new lint errors.

Is there anything you'd like reviewers to focus on?

netlify · 2023-05-20T22:38:00Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`21a6225`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/6478ab5ac9d0a10008667aac

mdjermanovic · 2023-05-20T22:40:44Z

eslint.config.js

-            "eslint-plugin/no-missing-message-ids": "error",
-            "eslint-plugin/no-unused-message-ids": "error",
-            "eslint-plugin/prefer-message-ids": "error",


These three rules are now already in the rules recommended config, so I removed them from here.

mdjermanovic · 2023-05-20T22:41:16Z

eslint.config.js

        rules: {
            "eslint-plugin/require-meta-docs-url": ["error", { pattern: "https://eslint.org/docs/latest/rules/{{name}}" }]
        }
    },
    {
        files: ["tests/lib/rules/*", "tests/tools/internal-rules/*"],
-        rules: {
-            ...eslintPlugin.configs["tests-recommended"].rules,
-            "eslint-plugin/prefer-output-null": "error",


This rule is now already in the tests recommended config, so I removed it from here.

mdjermanovic · 2023-05-20T22:41:52Z

lib/rules/logical-assignment-operators.js

@@ -188,7 +188,6 @@ module.exports = {
            }]
        },
        fixable: "code",
-        // eslint-disable-next-line eslint-plugin/require-meta-has-suggestions -- Does not detect conditional suggestions


This false positive has been fixed in the new version of eslint-plugin/require-meta-has-suggestions.

mdjermanovic · 2023-05-20T22:43:43Z

lib/rules/newline-after-var.js

@@ -212,7 +212,6 @@ module.exports = {
                context.report({
                    node,
                    messageId: "unexpected",
-                    data: { identifier: node.name },


This was caught by the new version of eslint-plugin/no-unused-placeholders.

mdjermanovic · 2023-05-20T22:43:51Z

lib/rules/newline-after-var.js

@@ -231,7 +230,6 @@ module.exports = {
                context.report({
                    node,
                    messageId: "expected",
-                    data: { identifier: node.name },


This was caught by the new version of eslint-plugin/no-unused-placeholders.

mdjermanovic · 2023-05-20T22:50:40Z

Looks like some tests that use this project's eslint config files are failing because eslint-plugin-eslint-plugin does not work on Node 12. I'll see if I can update the tests.

aladdin-add · 2023-05-22T12:09:12Z

can we just use eslint-plugin-eslint-plugin v4, not eslint-plugin-eslint-plugin/configs/*? something like:

const eslintPlugin = require("eslint-plugin-eslint-plugin");
module.exports = [{
files: ["lib/rules/*", "tools/internal-rules/*"],
plugins: {'eslint-plugin': eslintPlugin},
rules: {...eslintPlugin.configs.recommended.rules},
}];

The only difference eslintrc vs. flat is: the plugins has to be an object in the flat config, so it's possible to use the new config in v4.

mdjermanovic · 2023-05-23T12:17:30Z

can we just use eslint-plugin-eslint-plugin v4, not eslint-plugin-eslint-plugin/configs/*? something like:
const eslintPlugin = require("eslint-plugin-eslint-plugin");
module.exports = [{
files: ["lib/rules/*", "tools/internal-rules/*"],
plugins: {'eslint-plugin': eslintPlugin},
rules: {...eslintPlugin.configs.recommended.rules},
}];
The only difference eslintrc vs. flat is: the plugins has to be an object in the flat config, so it's possible to use the new config in v4.

We can certainly do that. I think that's how we're actually using eslint-plugin-eslint-plugin now in this project's eslint.config.js, so we would just leave everything as is:

eslint/eslint.config.js

Line 133 in 5b68d51

...eslintPlugin.configs["rules-recommended"].rules,

I just wanted to change this to use the flat config exports for testing purposes, as that would be a production example of a flat config that uses a shared flat config. Now, that alone doesn't justify the effort of updating 50 tests, but this might be a good opportunity to update those tests to use fixture configs so that we're able to change this project's lint configs without worrying about it affecting the tests in any way.

mdjermanovic · 2023-06-05T18:17:12Z

All tests are passing now.

bmish · 2023-06-05T19:23:50Z

eslint.config.js

+const eslintPluginRulesRecommendedConfig = require("eslint-plugin-eslint-plugin/configs/rules-recommended");
+const eslintPluginTestsRecommendedConfig = require("eslint-plugin-eslint-plugin/configs/tests-recommended");


Can you destructure these configs from require('eslint-plugin-eslint-plugin')? That will be more future-proof than reaching directly into specific files in the event that the plugin more-strictly defines its Node API.

I believe this is the only way to use flat configs from eslint-plugin-eslint-plugin at the moment.

https://github.com/eslint-community/eslint-plugin-eslint-plugin#eslintconfigjs-requires-eslintv8230

Configs in require('eslint-plugin-eslint-plugin').configs are in the eslintrc format.

@aladdin-add is that correct?

(not blocking) @aladdin-add shouldn't we export the flat configs too? Reaching into specific files is fragile especially if we were to move things around.

they are exported. The default predefined configs are eslintrc, and to not affect the use of eslintrc, I added a new directory to place the flat configs: https://github.com/eslint-community/eslint-plugin-eslint-plugin/tree/main/configs. Any better suggestions?

I'm going to merge this as it correctly uses the current interface of eslint-plugin-eslint-plugin. We can update the usage if and when the interface changes.

So require('eslint-plugin-eslint-plugin').configs will always have to be in the eslintrc format for backwards compatibility, but flat configs have to be accessed by file path e.g. require("eslint-plugin-eslint-plugin/configs/rules-recommended")? Is there a cleaner, long-term plan or recommendation for supporting both config formats (instead of just having to access the newer one by file path)?

So require('eslint-plugin-eslint-plugin').configs will always have to be in the eslintrc format for backwards compatibility, but flat configs have to be accessed by file path e.g. require("eslint-plugin-eslint-plugin/configs/rules-recommended")? Is there a cleaner, long-term plan or recommendation for supporting both config formats (instead of just having to access the newer one by file path)?

I think we could discuss that in #17242.

aladdin-add

LGTM.

mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing labels May 20, 2023

mdjermanovic requested a review from a team as a code owner May 20, 2023 22:37

mdjermanovic requested a review from aladdin-add May 20, 2023 22:39

mdjermanovic commented May 20, 2023

View reviewed changes

mdjermanovic marked this pull request as draft May 22, 2023 09:50

mdjermanovic added 4 commits June 1, 2023 14:27

chore: use eslint-plugin-eslint-plugin flat configs

3a30c6e

fix bin tests

ee52efc

fix cli tests

cc9019a

fix flat-eslint tests

21a6225

mdjermanovic force-pushed the eslintplugineslintplugin-flatconfig branch from 64b3388 to 21a6225 Compare June 1, 2023 14:29

mdjermanovic added 2 commits June 4, 2023 22:15

fix eslint tests

3f8b593

fix cli-engine tests

39f5786

mdjermanovic marked this pull request as ready for review June 5, 2023 18:10

bmish approved these changes Jun 5, 2023

View reviewed changes

aladdin-add approved these changes Jun 6, 2023

View reviewed changes

mdjermanovic merged commit 6a0196c into main Jun 7, 2023

mdjermanovic deleted the eslintplugineslintplugin-flatconfig branch June 7, 2023 07:25

mdjermanovic mentioned this pull request Jun 7, 2023

chore: upgrade eslint-plugin-jsdoc v46 #17245

Merged

1 task

eslint-github-bot bot locked and limited conversation to collaborators Dec 5, 2023

eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: use eslint-plugin-eslint-plugin flat configs #17204

chore: use eslint-plugin-eslint-plugin flat configs #17204

mdjermanovic commented May 20, 2023

netlify bot commented May 20, 2023 •

edited

Loading

mdjermanovic May 20, 2023

mdjermanovic May 20, 2023

mdjermanovic May 20, 2023

mdjermanovic May 20, 2023

mdjermanovic May 20, 2023

mdjermanovic commented May 20, 2023

aladdin-add commented May 22, 2023

mdjermanovic commented May 23, 2023

mdjermanovic commented Jun 5, 2023

bmish Jun 5, 2023

mdjermanovic Jun 5, 2023

bmish Jun 6, 2023

aladdin-add Jun 6, 2023

mdjermanovic Jun 7, 2023

bmish Jun 7, 2023

mdjermanovic Jun 7, 2023

aladdin-add left a comment

		const eslintPluginRulesRecommendedConfig = require("eslint-plugin-eslint-plugin/configs/rules-recommended");
		const eslintPluginTestsRecommendedConfig = require("eslint-plugin-eslint-plugin/configs/tests-recommended");

chore: use eslint-plugin-eslint-plugin flat configs #17204

chore: use eslint-plugin-eslint-plugin flat configs #17204

Conversation

mdjermanovic commented May 20, 2023

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented May 20, 2023 • edited Loading

✅ Deploy Preview for docs-eslint canceled.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdjermanovic commented May 20, 2023

aladdin-add commented May 22, 2023

mdjermanovic commented May 23, 2023

mdjermanovic commented Jun 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aladdin-add left a comment

Choose a reason for hiding this comment

netlify bot commented May 20, 2023 •

edited

Loading