eslint-plugin-import plugin error with workspaces

[davemarco]

In PR#1119, there is an issue when using workspaces with the eslint-plugin-import plugin.

Running npm run lint:check produces the following cryptic error:

ConfigError: Config "import/typescript": Key "plugins": Cannot redefine plugin "import".

After some investigation, the root cause is not entirely clear, but it appears related to the fact that node_modules are in a different location when using workspaces.

After testing, modifying CommonConfig.mjs to use importPlugin.flatConfigs.recommended appears to resolve the issue. This also aligns more closely with what the plugin recommends in its documentation.

For example, instead of:

import ImportPlugin from "eslint-plugin-import";

export default {
  plugins: {
    import: ImportPlugin,
  },
  rules: {
    // ...
  },
};

You can do:

import importPlugin from "eslint-plugin-import";

export default [
  importPlugin.flatConfigs.recommended,
  {
    rules: {
      // your additional rules
    },
  },
];

It's still unclear exactly why this works, but I did notice that the registered plugin name changes from:

import

to:

import:eslint-plugin-import@2.31.0.0/

when running npx eslint --print-config <config-path>.

Lastly, this fix requires converting CommonConfig into a CommonConfigArray, which I think is fine — but it will also require updates in the log viewer.

@junhaoliao, are okay with this change? If so i will make a PR to unblock PR#1119

[junhaoliao]

The issue is within ESLint where it performs an identity check on the plugin objects when it finds two plugins with the same name:
https://github.com/eslint/eslint/blob/0d17242b3c25c2ddf8363f4560641acd1ae82ca9/lib/config/flat-config-schema.js#L394-L397

My understanding is that ESLint does allow a plugin to be registered more than once so long their definitions are the same. However, their "redefinition" check is not carefully implemented. I believe our config should pass the check if ESLint was performing a deep comparison like loadash.isEqual. We can submit an issue to ESLint if you agree their existing behaviour is problematic.

For us, to avoid interface changes (so we achieve minimal changes in all our repos), I prefer keeping the CommonConfig. Instead, we can change line

eslint-config-yscope/CommonConfig.mjs

Line 16 in add6940

"import": ImportPlugin,

to

        "import": ImportPlugin.flatConfigs.recommended.plugins.import,

[junhaoliao]

it performs an identity check...
... We can submit an issue to ESLint if you agree their existing behaviour is problematic.

maybe they do that for performance reasons. so maybe instead we should file an issue to eslint-plugin-import to have them export this:
https://github.com/import-js/eslint-plugin-import/blob/01c9eb04331d2efa8d63f2d7f4bfec3bc44c94f3/src/index.js#L77-L78

[davemarco]

re:
"import": ImportPlugin.flatConfigs.recommended.plugins.import,
I guess this dosent import the starter config here, (i.e. only imports the plugin), but i guess okay since we already have a bunch of custom rules for the plugin?

[junhaoliao]

re:
"import": ImportPlugin.flatConfigs.recommended.plugins.import,
I guess this dosent import the starter config here, (i.e. only imports the plugin), but i guess okay since we already have a bunch of custom rules for the plugin?

Right, the effects should be the same because the current code and my proposal both only import a "rules" object which only contains definitions, rather than configurations, of the rules. It is intended because we want finer grained controls over the configurations.

[junhaoliao]

closed by #22