Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Support new ESLint flat config #2556

Open
Tracked by #18093 ...
TomerAberbach opened this issue Sep 25, 2022 · 37 comments
Open
Tracked by #18093 ...

[Feature Request] Support new ESLint flat config #2556

TomerAberbach opened this issue Sep 25, 2022 · 37 comments

Comments

@TomerAberbach
Copy link

ESLint has a new experimental config file format and this plugin doesn't work with it. The plugin fails at this line because the new config format doesn't pass parsers via paths. Instead the parser object itself is passed.

I was able to mostly fix this in my fork, but because the parserPath doesn't exist anymore for this config format some of the keysFromParser logic won't work anymore.

Anyway, figured I'd open this issue to start discussion on the new config file format!

@TomerAberbach TomerAberbach changed the title [Feature Request] Support new ESLint flag config [Feature Request] Support new ESLint flat config Sep 25, 2022
@ljharb
Copy link
Member

ljharb commented Oct 9, 2022

If there's a way to support both configs at the same time, a PR would be appreciated.

Look to jsx-eslint/eslint-plugin-jsx-a11y#891 and jsx-eslint/eslint-plugin-react#3429 for some preferred direction.

@TomerAberbach
Copy link
Author

Yup, my fork does support both configs. The only thing I'm unsure about in my fork is how to adapt the keysFromParser logic to work without knowing the paths.

But based on this comment though, it sounds like maybe it's fine for the new config format not to set visitorKeys to anything? Let me know!

@ljharb
Copy link
Member

ljharb commented Nov 1, 2022

As long as the rules and configs work in both config systems, we're good.

@b0o
Copy link

b0o commented Dec 22, 2022

The plugin fails at this line because the new config format doesn't pass parsers via paths.

We're experiencing this issue as well. @TomerAberbach it would be really cool if you could submit a PR with your fixes!

@TomerAberbach
Copy link
Author

Thanks for the message! Been a bit busy with other stuff and kind of forgot about this 😅

I'll try to pick this up soon!

@goosewobbler
Copy link

goosewobbler commented Jan 31, 2023

As long as the rules and configs work in both config systems, we're good.

I'll test this on my fork of @TomerAberbach's fork when I get a chance.

@TomerAberbach
Copy link
Author

Thanks @goosewobbler! A little while ago I tried making it so that all the tests get run twice (once with the legacy and once with the new config system), but got stuck trying to get stuff working and then got busy 🙃

Would be awesome if you're able to figure out how to test both systems! Happy to answer any questions about the fork if that would help as well

@CallMeLaNN
Copy link

CallMeLaNN commented Feb 6, 2023

I managed to make it work without changes in parser.js source. Since parserPath can be parser name and keysFromParser can accept it, we can use the settings.import/parsers to add the missing parser from getParserPath function.

So that can pass the parserPath is required! error but actually I don't see parserPath is useful to keysFromParser because on my env it always doesn't hit the line mentioned in keysFromParser logic won't work anymore. So "espree" below is the default I guess from the source.

This is my working test config on node and typescript env. (EDIT: I just remove from using FlatCompat and directly construct the config)

import importPlugin from "eslint-plugin-import";

export default [
  // ... "eslint:recommended"
  {
    // All eslint-plugin-import config is here
    languageOptions: {
      parserOptions: {
        // Eslint doesn't supply ecmaVersion in `parser.js` `context.parserOptions`
        // This is required to avoid ecmaVersion < 2015 error or 'import' / 'export' error
        ecmaVersion: "latest",
        sourceType: "module",
      },
    },
    plugins: { import: importPlugin },
    settings: {
      // This will do the trick
      "import/parsers": {
        espree: [".js", ".cjs", ".mjs", ".jsx"],
      },
      "import/resolver": {
        typescript: true,
        node: true,
      },
    },
    rules: {
      ...importPlugin.configs["recommended"].rules,
    },
  },
  // ... add other plugins like typescript-eslint etc
  {
    // All my custom config
    languageOptions: {
      // This default get replaced by plugins, so I added back. Not related probably.
      ecmaVersion: "latest",
      sourceType: "module",
     // ... globals etc
    },
  },
  // ... custom rules etc
]

It was discovered and tested on a module js file with import/namespace rule throw like this. I lint the eslint.config.js itself lol

Parse errors in imported module '@eslint/eslintrc': parserPath is required! (undefined:undefined) eslint(import/namespace)

@ziimakc
Copy link

ziimakc commented May 20, 2023

Any way to make it work in meantime with flat config?

UPD. espree did the trick:

"import/parsers": {
    espree: [".js", ".cjs", ".mjs", ".jsx"],
    "@typescript-eslint/parser": [".ts"],
},

@500-internal-server-error

Hello, I'm still having problems with mixing this plugin with my ESLint flat config. The fork mentioned in the OP seems to be a bit old. Is there any way I can use this plugin with the new flat config?

$ npx eslint src/index.ts

Oops! Something went wrong! :(

ESLint: 8.42.0

Configuration for rule "plugins" is invalid. Expected severity of "off", 0, "warn", 1, "error", or 2.

You passed '"import"'.

See https://eslint.org/docs/latest/use/configure/rules#using-configuration-files for configuring rules.

Thanks in advance.

@ljharb
Copy link
Member

ljharb commented Jun 8, 2023

Not until that support is released.

500-internal-server-error added a commit to binusgdc/VCPA that referenced this issue Jun 9, 2023
Downgraded ESLint config file to use the old cascading config instead of the new flat config. This allows eslint-plugin-import to be used, as it seems to only support the old config.

See:

- import-js/eslint-plugin-import#2556

- eslint/eslint#13481
500-internal-server-error added a commit to binusgdc/VCPA that referenced this issue Jun 9, 2023
Downgraded ESLint config file to use the old cascading config instead of the new flat config. This allows eslint-plugin-import to be used, as it seems to only support the old config.

See:
- import-js/eslint-plugin-import#2556
- eslint/eslint#13481
@OlivierZal
Copy link

OlivierZal commented Aug 2, 2023

Hi @ljharb and @timmywil,

I guess my issue is related to this one but I'm not sure: I've recently migrated my .eslintrc to the new eslint.config.js eslint flat config:

import js from '@eslint/js'
/* eslint-disable import/no-unresolved */
import tsParser from '@typescript-eslint/parser'
import tsPlugin from '@typescript-eslint/eslint-plugin'
/* eslint-enable import/no-unresolved */
import prettierConfig from 'eslint-config-prettier'
import importPlugin from 'eslint-plugin-import'
import globals from 'globals'

export default [
  {
    ignores: ['.homeybuild/'],
  },
  js.configs.recommended,
  {
    languageOptions: {
      ecmaVersion: 'latest',
      globals: {
        ...globals.browser,
        ...globals.es2024,
        ...globals.node,
      },
      sourceType: 'module',
    },
    linterOptions: {
      reportUnusedDisableDirectives: true,
    },
    plugins: {
      import: importPlugin,
    },
    rules: importPlugin.configs.recommended.rules,
  },
  {
    files: ['**/*.ts', '**/*.tsx', '**/*.mts', '**/*.cts'],
    languageOptions: {
      parser: tsParser,
      parserOptions: {
        project: './tsconfig.json',
      },
    },
    plugins: {
      '@typescript-eslint': tsPlugin,
    },
    rules: {
      ...tsPlugin.configs['eslint-recommended'].overrides[0].rules,
      ...tsPlugin.configs['strict-type-checked'].rules,
      ...tsPlugin.configs['stylistic-type-checked'].rules,
      ...importPlugin.configs.typescript.rules,
      '@typescript-eslint/no-unsafe-argument': 'off',
      '@typescript-eslint/no-unsafe-assignment': 'off',
      '@typescript-eslint/no-unsafe-call': 'off',
      '@typescript-eslint/no-unsafe-member-access': 'off',
      '@typescript-eslint/no-unsafe-return': 'off',
      '@typescript-eslint/no-unused-vars': [
        'error',
        {
          varsIgnorePattern: 'onHomeyReady',
        },
      ],
    },
    settings: {
      ...importPlugin.configs.typescript.settings,
      'import/resolver': {
        ...importPlugin.configs.typescript.settings['import/resolver'],
        typescript: {
          alwaysTryTypes: true,
        },
      },
    },
  },
  prettierConfig,
]

and I got the following errors:

  2:19  error    Parse errors in imported module 'axios': parserPath or languageOptions.parser is required! (undefined:undefined)  import/namespace
  2:19  error    Parse errors in imported module 'axios': parserPath or languageOptions.parser is required! (undefined:undefined)  import/default
  2:19  warning  Parse errors in imported module 'axios': parserPath or languageOptions.parser is required! (undefined:undefined)  import/no-named-as-default
  2:19  warning  Parse errors in imported module 'axios': parserPath or languageOptions.parser is required! (undefined:undefined)  import/no-named-as-default-member

Should it be solved with #2829, or is it another issue?

I'm also wondering why I'm getting this error (only for @typescript-eslint/* imports):

  2:22  error  Unable to resolve path to module '@typescript-eslint/parser'         import/no-unresolved
  3:22  error  Unable to resolve path to module '@typescript-eslint/eslint-plugin'  import/no-unresolved

@ljharb
Copy link
Member

ljharb commented Aug 4, 2023

@OlivierZal yes, i suspect #2829 will solve your issue.

For the other one, are those packages installed on disk and in package.json?

@OlivierZal
Copy link

OlivierZal commented Aug 4, 2023

Thanks @ljharb for your answer.

Yes, they are installed and work very well: @typescript-eslint errors are correctly raised when the linter is run.

So do you think it is worth raising a separate issue on this one?

@ljharb
Copy link
Member

ljharb commented Aug 5, 2023

Yes, I think so, thanks

@simlu
Copy link

simlu commented Aug 14, 2023

Running into the same error here parserPath or languageOptions.parser is required! when trying to use FlatESLint.

I'll keep an eye on #2829 for sure 👍

@ljharb
Copy link
Member

ljharb commented Apr 8, 2024

@trivikr see #2948. One simply shouldn't upgrade to v9 until all of the configs and plugins one uses support it, just like every other time eslint has bumped the major.

@JavaScriptBach
Copy link

Has anyone managed to get this plugin to work on a TypeScript project using FlatCompat?

@ljharb
Copy link
Member

ljharb commented May 7, 2024

@JavaScriptBach in theory, all of our rules except one should work with FlatCompat, but one of them can't work yet because flat config lacks the capacity to support it.

@arslivinski
Copy link

@ljharb which one?

@ljharb
Copy link
Member

ljharb commented May 8, 2024

@arslivinski no-unused-modules

@arslivinski
Copy link

Every time that I enable the no-unused-modules rule, I end up disabling it not long after because it always breaks my flow when splitting a large PR into smaller ones, as I always end with unused modules. I understand that it is MY use case and doesn't reflect how this plugin is used at large.

That being said, supporting flat configs and ESLint >= 9 could be considered a breaking change? If so, couldn't we also remove this rule if it is the only reason holding this back?

Or, as another alternative, would be too much effort to have two exports? The default one using the previous config engine and an alternative one using the flat config engine? I think that this could be possible if we target a minimum version of ESLint 8 that has support for the new flat config, and to use the flat config we have to explicitly import the plugin. I would be pleased to submit a PR for this, if it is something that the maintainers see as a possible solution.

@guillaumebrunerie
Copy link

Every time that I enable the no-unused-modules rule, I end up disabling it not long after because it always breaks my flow when splitting a large PR into smaller ones, as I always end with unused modules. I understand that it is MY use case and doesn't reflect how this plugin is used at large.

Interesting, I have the opposite experience. Personally I find no-unused-modules to be by far the most useful rule this plugin offers, as it allows finding dead code that would be very hard to find otherwise.

However, I've also found that it often doesn't work very well in my editor, it regularly reports some exports as unused even though "find all references" finds several references in other files. Linting from the command line works fine.

I wonder if there could be a compromise consisting of activating this rule only when linting the whole project and not when linting a specific file? From what I understand, requiring access to other files than the one being linted is what causes problems in ESLint 9, so maybe that would be easier?

@ljharb
Copy link
Member

ljharb commented May 10, 2024

@arslivinski no, it needn't be a breaking change, it will be semver-minor. but regardless, i wouldn't want to claim flat config support and not have 100% of it.

@arslivinski
Copy link

@ljharb completely understand your point of view. However, do you know if there's some change planned for ESLint that would make this rule viable again? If not, what's the plan, just wait? I really want to contribute. 😄

Off-topic

Every time that I enable the no-unused-modules rule, I end up disabling it not long after because it always breaks my flow when splitting a large PR into smaller ones, as I always end with unused modules. I understand that it is MY use case and doesn't reflect how this plugin is used at large.

Interesting, I have the opposite experience. Personally I find no-unused-modules to be by far the most useful rule this plugin offers, as it allows finding dead code that would be very hard to find otherwise.

@guillaumebrunerie in the past, when I was in need to do a spring cleanup on codebase, I always used Madge for the task of finding unused modules. My conclusion is that this method of doing a cleanup occasionally is more effective (and faster!) than running no-unused-modules.

However, I've also found that it often doesn't work very well in my editor, it regularly reports some exports as unused even though "find all references" finds several references in other files. Linting from the command line works fine.

I wonder if there could be a compromise consisting of activating this rule only when linting the whole project and not when linting a specific file? From what I understand, requiring access to other files than the one being linted is what causes problems in ESLint 9, so maybe that would be easier?

You can disable the rule on the default config and enable it only when running on the CI or a pre-commit/push hook.

@ljharb
Copy link
Member

ljharb commented May 10, 2024

@arslivinski yes, if we can affirm to the eslint team that their proposed solution will work for us, they'll ship something in v9 we can use (and polyfill for v8 and earlier v9s) moving forward. The work hasn't been done to verify that yet, though.

@glarget
Copy link

glarget commented May 14, 2024

Hi, I tried to migrate to eslint v9 and understand that eslint-plugin-import is still in WIP to support it.

I have a question related to "import/order" rules.

Does this rule supposed to work with the flat config ?

I tried it but the format on save with doesn't work as expected.

I guess that I have to wait until migrating to V9 or maybe I do something wrong ?

Here is a code sample. Thank you very much for your help !

import js from '@eslint/js';
import importPlugin from 'eslint-plugin-import';
import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended';

export default [
  js.configs.recommended,
  ...tseslint.configs.recommended,
 plugins: {
      react,
      'react-hooks': reactHooks,
      import: importPlugin,
  },
rules: { 
  'import/order': [
        'error',
        {
          groups: ['builtin', 'external', 'internal', 'parent', 'sibling', 'index', 'object'],
          'newlines-between': 'always',
          alphabetize: {
            order: 'asc',
            caseInsensitive: true,
          },
          pathGroups: [
            {
              pattern: 'react',
              group: 'external',
              position: 'before',
            },
          ],
          pathGroupsExcludedImportTypes: ['react'],
          warnOnUnassignedImports: true,
        },
      ],
  },
  eslintPluginPrettierRecommended
]

@ljharb
Copy link
Member

ljharb commented May 14, 2024

@glarget no, this plugin does not yet work with flat config or with v9.

gajus added a commit to gajus/eslint-config-canonical that referenced this issue May 17, 2024
@lgenzelis
Copy link

lgenzelis commented May 20, 2024

For anyone else landing here looking for a solution, here's my eslint.config.cjs, which works for me :)

const pluginImport = require('eslint-plugin-import');

module.exports = [
  {
    plugins: {
      import: { rules: pluginImport.rules },
    },
    rules: {
      'import/order': 'error',
      'import/group-exports': 'error',
      'import/exports-last': 'error',
    },
  },
  // ... all your other other configurations
];

@csvan
Copy link

csvan commented May 20, 2024

@lgenzelis are you sure all the rules actually work in the above? The fact that you can successfully run eslint with the above config is not an indication that the plugin actually works, it just means you get a lot of false positives.

@OlivierZal
Copy link

OlivierZal commented May 20, 2024

On my end, the following flat config (extract) worked with ESlint 8 but raises an error with ESlint 9:

    plugins: {
      import: importPlugin,
    },
    rules: {
      ...importPlugin.configs.recommended.rules, // flat config does not support the old `plugins` format
      <custom rules>
    }

Error raised:

TypeError: context.getAncestors is not a function
Occurred while linting /Users/.../src/index.ts:66
Rule: "import/no-named-as-default"
    at importDeclaration (/Users/.../node_modules/eslint-plugin-import/lib/importDeclaration.js:2:27)
    at checkDefault (/Users/.../node_modules/eslint-plugin-import/lib/rules/no-named-as-default.js:21:62)
    at ruleErrorHandler (/Users/.../node_modules/eslint/lib/linter/linter.js:1115:48)
    at /Users/.../node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/.../node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/.../node_modules/eslint/lib/linter/node-event-generator.js:297:26)
    at NodeEventGenerator.applySelectors (/Users/.../node_modules/eslint/lib/linter/node-event-generator.js:326:22)
    at NodeEventGenerator.enterNode (/Users/.../node_modules/eslint/lib/linter/node-event-generator.js:340:14)
    at runRules (/Users/.../node_modules/eslint/lib/linter/linter.js:1154:40)

Not all the import plugin rules fail, but some of the recommended config do.

I think that such a compatibility issue with ESlint 9 needs to be addressed before to think about flat config.

@ljharb
Copy link
Member

ljharb commented May 20, 2024

@OlivierZal flat config works on eslint 8, so that's what i'd want to support first.

@lgenzelis
Copy link

@lgenzelis are you sure all the rules actually work in the above? The fact that you can successfully run eslint with the above config is not an indication that the plugin actually works, it just means you get a lot of false positives.

@csvan not saying that every rule works, but those specific 3 do (I validated it). Oh, and I've should've mentioned: I'm using eslint 8, not 9.

@trivikr

This comment was marked as spam.

@trivikr

This comment was marked as off-topic.

@ljharb

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests