fix: Wrap plugins that are wrapped in extends (#37)

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

packages/migrate-config/src/migrate-config.js

@@ -151,6 +151,9 @@ function convertESLintIgnoreToMinimatch(pattern) {
 	return `${negatedPrefix}${matchEverywherePrefix}${patternWithoutLeadingSlash}${matchInsideSuffix}`;
 }
 
+// cache for plugins needing compat
+const pluginsNeedingCompatCache = new Set(pluginsNeedingCompat);
+
 /**
  * Determines if a plugin needs the compat utility.
  * @param {string} pluginName The name of the plugin.
@@ -161,7 +164,7 @@ function pluginNeedsCompat(pluginName) {
 		? pluginName.slice(0, pluginName.indexOf("/"))
 		: pluginName;
 
-	return pluginsNeedingCompat.includes(
+	return pluginsNeedingCompatCache.has(
 		naming.normalizePackageName(pluginNameToTest, "eslint-plugin"),
 	);
 }
@@ -643,17 +646,6 @@ function migrateConfigObject(migration, config) {
 		properties.push(b.property("init", b.identifier("ignores"), ignores));
 	}
 
-	// Copy over `plugins`
-	if (config.plugins) {
-		properties.push(
-			b.property(
-				"init",
-				b.identifier("plugins"),
-				createPlugins(config.plugins, migration),
-			),
-		);
-	}
-
 	// Handle `extends`
 	if (config.extends) {
 		let extendsCallExpression = b.callExpression(
@@ -675,6 +667,25 @@ function migrateConfigObject(migration, config) {
 		});
 
 		if (needsCompat) {
+			/*
+			 * When even one `extends` item needs compat, we need to mark every
+			 * plugin as needing compat. This is because the `fixupConfigRules`
+			 * function will be called on the entire object, and if any of those
+			 * plugins is also referenced in `plugins`, the user will get a
+			 * "can't redefine plugin" error.
+			 */
+			extendsArray.forEach(extend => {
+				if (extend.startsWith("plugin:")) {
+					const pluginName = extend.slice(7, extend.indexOf("/"));
+					const normalizedPluginName = naming.normalizePackageName(
+						pluginName,
+						"eslint-plugin",
+					);
+
+					pluginsNeedingCompatCache.add(normalizedPluginName);
+				}
+			});
+
 			if (!migration.imports.has("@eslint/compat")) {
 				migration.imports.set("@eslint/compat", {
 					bindings: ["fixupConfigRules"],
@@ -728,6 +739,20 @@ function migrateConfigObject(migration, config) {
 		configArrayElements.push(b.spreadElement(extendsCallExpression));
 	}
 
+	/*
+	 * Copy over plugins. This must happen after processing `extends` in order to
+	 * properly account for plugins that need the compat utility.
+	 */
+	if (config.plugins) {
+		properties.push(
+			b.property(
+				"init",
+				b.identifier("plugins"),
+				createPlugins(config.plugins, migration),
+			),
+		);
+	}
+
 	// Copy over `noInlineConfig` and `reportUnusedDisableDirectives`
 	const linterOptions = createLinterOptions(config);
 	if (linterOptions) {

packages/migrate-config/tests/fixtures/basic-eslintrc/expected.cjs

@@ -1,15 +1,14 @@
+const {
+    fixupConfigRules,
+    fixupPluginRules,
+} = require("@eslint/compat");
+
 const prettier = require("eslint-plugin-prettier");
 const _import = require("eslint-plugin-import");
 const node = require("eslint-plugin-node");
 const promise = require("eslint-plugin-promise");
 const standard = require("eslint-plugin-standard");
 const typescriptEslint = require("@typescript-eslint/eslint-plugin");
-
-const {
-    fixupPluginRules,
-    fixupConfigRules,
-} = require("@eslint/compat");
-
 const globals = require("globals");
 const tsParser = require("@typescript-eslint/parser");
 const js = require("@eslint/js");

packages/migrate-config/tests/fixtures/basic-eslintrc/expected.mjs

@@ -1,10 +1,10 @@
+import { fixupConfigRules, fixupPluginRules } from "@eslint/compat";
 import prettier from "eslint-plugin-prettier";
 import _import from "eslint-plugin-import";
 import node from "eslint-plugin-node";
 import promise from "eslint-plugin-promise";
 import standard from "eslint-plugin-standard";
 import typescriptEslint from "@typescript-eslint/eslint-plugin";
-import { fixupPluginRules, fixupConfigRules } from "@eslint/compat";
 import globals from "globals";
 import tsParser from "@typescript-eslint/parser";
 import path from "node:path";

packages/migrate-config/tests/fixtures/no-globals-for-env/expected.cjs

@@ -1,15 +1,14 @@
+const {
+    fixupConfigRules,
+    fixupPluginRules,
+} = require("@eslint/compat");
+
 const prettier = require("eslint-plugin-prettier");
 const _import = require("eslint-plugin-import");
 const node = require("eslint-plugin-node");
 const promise = require("eslint-plugin-promise");
 const standard = require("eslint-plugin-standard");
 const typescriptEslint = require("@typescript-eslint/eslint-plugin");
-
-const {
-    fixupPluginRules,
-    fixupConfigRules,
-} = require("@eslint/compat");
-
 const tsParser = require("@typescript-eslint/parser");
 const js = require("@eslint/js");
 

packages/migrate-config/tests/fixtures/no-globals-for-env/expected.mjs

@@ -1,10 +1,10 @@
+import { fixupConfigRules, fixupPluginRules } from "@eslint/compat";
 import prettier from "eslint-plugin-prettier";
 import _import from "eslint-plugin-import";
 import node from "eslint-plugin-node";
 import promise from "eslint-plugin-promise";
 import standard from "eslint-plugin-standard";
 import typescriptEslint from "@typescript-eslint/eslint-plugin";
-import { fixupPluginRules, fixupConfigRules } from "@eslint/compat";
 import tsParser from "@typescript-eslint/parser";
 import path from "node:path";
 import { fileURLToPath } from "node:url";

packages/migrate-config/tests/fixtures/plugins-dedupe/.eslintrc.yml

@@ -0,0 +1,7 @@
+root: true
+plugins:
+    - import
+    - n
+extends:
+    - plugin:import/errors
+    - plugin:n/recommended

packages/migrate-config/tests/fixtures/plugins-dedupe/expected.cjs

@@ -0,0 +1,28 @@
+const {
+    fixupConfigRules,
+    fixupPluginRules,
+} = require("@eslint/compat");
+
+const _import = require("eslint-plugin-import");
+const n = require("eslint-plugin-n");
+const js = require("@eslint/js");
+
+const {
+    FlatCompat,
+} = require("@eslint/eslintrc");
+
+const compat = new FlatCompat({
+    baseDirectory: __dirname,
+    recommendedConfig: js.configs.recommended,
+    allConfig: js.configs.all
+});
+
+module.exports = [
+    ...fixupConfigRules(compat.extends("plugin:import/errors", "plugin:n/recommended")),
+    {
+        plugins: {
+            import: fixupPluginRules(_import),
+            n: fixupPluginRules(n),
+        },
+    },
+];

packages/migrate-config/tests/fixtures/plugins-dedupe/expected.mjs

@@ -0,0 +1,26 @@
+import { fixupConfigRules, fixupPluginRules } from "@eslint/compat";
+import _import from "eslint-plugin-import";
+import n from "eslint-plugin-n";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import js from "@eslint/js";
+import { FlatCompat } from "@eslint/eslintrc";
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = path.dirname(__filename);
+
+
+const compat = new FlatCompat({
+    baseDirectory: __dirname,
+    recommendedConfig: js.configs.recommended,
+    allConfig: js.configs.all
+});
+
+export default [
+    ...fixupConfigRules(compat.extends("plugin:import/errors", "plugin:n/recommended")),
+    {
+        plugins: {
+            import: fixupPluginRules(_import),
+            n: fixupPluginRules(n),
+        },
+    },
+];

packages/migrate-config/tests/fixtures/release-it/expected.cjs

@@ -1,11 +1,10 @@
-const prettier = require("eslint-plugin-prettier");
-const _import = require("eslint-plugin-import");
-
 const {
-    fixupPluginRules,
     fixupConfigRules,
+    fixupPluginRules,
 } = require("@eslint/compat");
 
+const prettier = require("eslint-plugin-prettier");
+const _import = require("eslint-plugin-import");
 const globals = require("globals");
 const js = require("@eslint/js");
 

packages/migrate-config/tests/fixtures/release-it/expected.mjs

@@ -1,6 +1,6 @@
+import { fixupConfigRules, fixupPluginRules } from "@eslint/compat";
 import prettier from "eslint-plugin-prettier";
 import _import from "eslint-plugin-import";
-import { fixupPluginRules, fixupConfigRules } from "@eslint/compat";
 import globals from "globals";
 import path from "node:path";
 import { fileURLToPath } from "node:url";

packages/migrate-config/tests/migrate-config-cli.test.js

@@ -21,6 +21,8 @@ const filePaths = [
 	"reveal-md/.eslintrc",
 	"release-it/.eslintrc.json",
 	"no-globals-for-env/.eslintrc.yml",
+	"overrides-extends/.eslintrc.json",
+	"plugins-dedupe/.eslintrc.yml",
 ].map(file => `tests/fixtures/${file}`);
 
 //-----------------------------------------------------------------------------