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

Support eslint flat config #29

Closed
silverwind opened this issue Mar 9, 2024 · 27 comments · Fixed by #122
Closed

Support eslint flat config #29

silverwind opened this issue Mar 9, 2024 · 27 comments · Fixed by #122

Comments

@silverwind
Copy link

Likely depends on import-js#2556 but it's good to track nonetheless.

Ref: eslint/eslint#18093

@JounQin
Copy link
Member

JounQin commented Mar 13, 2024

eslint-plugin-i is now eslint-plugin-import-x.

I'm focusing on TypeScript migration first, see also #41.

After that, I'll start working on new features like supporting flat config.

@meteorlxy
Copy link

meteorlxy commented Mar 18, 2024

Just to mention, some rules will break when using flat config. See this comment

In brief, eslint will fail to use eslint.config.js as config file after enabling import/no-unused-modules with unusedExports: true

Related issues:

@karlhorky
Copy link

Interesting, we are using eslint-plugin-import-x now with the Flat Config:

This seems to be working for us so far:

Simplified version of the code:

import eslintImportX from 'eslint-plugin-import-x';

/** @type {import('@typescript-eslint/utils/ts-eslint').FlatConfig.ConfigArray} */
const configArray = [
  {
    plugins: {
     'import-x': eslintImportX,
    },
    settings: {
      'import-x/parsers': {
        '@typescript-eslint/parser': ['.ts', '.tsx'],
      },
      'import-x/resolver': {
        // Load <rootdir>/tsconfig.json
        typescript: true,
        node: true,
      },
    },
    rules: {
      // Error on imports that don't match the underlying file system
      // https://github.com/un-ts/eslint-plugin-import-x/blob/master/docs/rules/no-unresolved.md
      'import-x/no-unresolved': 'error',
  },
];

@Zamiell
Copy link

Zamiell commented May 30, 2024

@karlhorky I am trying out this plugin with the flat config. I copy pasted the config from your post, but I get these errors:

image

(I enabled more rules than simply import-x/no-unresolved.)

Any idea on how to fix?

@n0099
Copy link

n0099 commented Jun 5, 2024

module.exports = {
    extends: [
        'plugin:import-x/recommended',
        'plugin:import-x/typescript',
    ],
}

can only be translate into

export default [
    ...compat.config(pluginImportX.configs.recommended), // https://github.com/un-ts/eslint-plugin-import-x/issues/29#issuecomment-2148843214
    pluginImportX.configs.typescript, // no need of wrapping in compat.config() since there's no pluginImportX.configs.typescript.plugins
];

since


is still passing string as plugin.

@onx2
Copy link

onx2 commented Jun 8, 2024

@n0099 where is compat coming from?

Is there an example of a simple flat config for eslint v9 that uses this plugin?

My super simple set up works for everything but the import sorting.

import globals from "globals";
import pluginJs from "@eslint/js";
import tseslint from "typescript-eslint";
import pluginImportX from "eslint-plugin-import-x";

export default [
  {
    languageOptions: { globals: globals.browser },
    ignores: ["./dist/"],
  },
  pluginJs.configs.recommended,
  ...tseslint.configs.recommended,
  pluginImportX.configs.typescript,
];

@n0099
Copy link

n0099 commented Jun 8, 2024

@n0099 where is compat coming from?

@eslint/eslintrc ota-meshi/typescript-eslint-parser-for-extra-files#95 (comment)

import { FlatCompat } from '@eslint/eslintrc';

My super simple set up works for everything but the import sorting.

If you only want to extends from typescript rules, the plugin is not defined in the rule:

export = {
settings: {
'import-x/extensions': allExtensions,
'import-x/external-module-folders': ['node_modules', 'node_modules/@types'],
'import-x/parsers': {
'@typescript-eslint/parser': [...typeScriptExtensions, '.cts', '.mts'],
},
'import-x/resolver': {
node: {
extensions: allExtensions,
},
},
},
rules: {
// analysis/correctness
// TypeScript compilation already ensures that named imports exist in the referenced module
'import-x/named': 'off',
},
} satisfies PluginConfig

so manually add plugin define to let eslint know where to find define for rules with prefix import-x/

+  { plugins: { 'import-x': pluginImportX } },
  pluginImportX.configs.typescript,

or just also extending recommended rules:

+  ...compat.config(pluginImportX.configs.recommended),
  pluginImportX.configs.typescript,

@muuvmuuv
Copy link

Where do you import compat from?

@GalindoSVQ
Copy link

Where do you import compat from?

#29 (comment)

@KristjanTammekivi
Copy link

Just to mention, some rules will break when using flat config. See this comment

In brief, eslint will fail to use eslint.config.js as config file after enabling import/no-unused-modules with unusedExports: true

Related issues:

* [Bug: can not find flat file config eslint/eslint#16485](https://github.com/eslint/eslint/issues/16485)

* [import/no-unused-modules breaks `--no-eslintrc` and eslint fails import-js/eslint-plugin-import#2907](https://github.com/import-js/eslint-plugin-import/issues/2907)

* [Add note(s) to docs about flat configs and/or no-unused-modules' `unusedExports` not being supported in flat configs import-js/eslint-plugin-import#2964](https://github.com/import-js/eslint-plugin-import/issues/2964)

Also this eslint/eslint#18087

@Zamiell
Copy link

Zamiell commented Jul 2, 2024

Chiming in here to say that I don't think the ecosystem uses import/no-unused-modles anymore in favor of knip. Since ESLint is designed to parse individual files (as the linked GitHub issue above shows), it makes more sense to do this kind of project-level checking in a separate tool.

Are any other rules than import/no-unused-modles broken by the flat config?

@SukkaW
Copy link
Collaborator

SukkaW commented Jul 7, 2024

Let's deprecate no-unused-modles then~!

@SukkaW
Copy link
Collaborator

SukkaW commented Jul 8, 2024

@Zamiell The discussion about deprecating no-unused-modules rule can be found here: #90

@silverwind
Copy link
Author

silverwind commented Jul 8, 2024

Never heard about knip, but I'm generally in favor of at least disabling rules that are duplicate with what typescript already validates, if the linted file is typescript, of course. But I guess this is more of a config topic than a plugin topic.

@GerroDen
Copy link

It works great in flat config 👍🏻 but no-unused-modules leads to an eslint error that the eslint config is missing.

@SukkaW
Copy link
Collaborator

SukkaW commented Jul 25, 2024

It works great in flat config 👍🏻 but no-unused-modules leads to an eslint error that the eslint config is missing.

Yes. This is because no-unused-modules uses an ESLint internal private API that was designed to be used for crawling eslintrc files.

Currently, I am working on a more performant no-cycle rule alternative.

@silverwind would you like to replace the FileEnumerator w/ a globbing function?

@silverwind
Copy link
Author

silverwind commented Jul 25, 2024

In import-js#3018 they used @nodelib/fs.walk to replace it, if I understand correctly. Seems like a popular enough dependency to use.

@silverwind
Copy link
Author

Maybe @michaelfaith would be interested in porting their work here in case that PR stalls because of those "node 4 concerns" which we do not have with this fork 😆.

@SukkaW
Copy link
Collaborator

SukkaW commented Jul 25, 2024

Maybe @michaelfaith would be interested in porting their work here in case that PR stalls because of those "node 4 concerns" which we do not have with this fork 😆.

Yeah. @michaelfaith if you are interested, you can port that PR to eslint-plugin-import-x as well. We use TypeScript and we are not limited to Node.js 4.

@michaelfaith
Copy link

Sure, I don't mind helping out, when I have some time.

@controversial
Copy link

controversial commented Aug 3, 2024

In import-js#3018 they used @nodelib/fs.walk to replace it

note the fs.walk approach discussed in import-js#3018 for replacing FileEnumerator depends on a proposed eslint API that’s currently only hypothetical'

I’d support deprecating no-unused-modules and skipping support for using that rule with flat config

@SukkaW
Copy link
Collaborator

SukkaW commented Aug 4, 2024

that’s currently only hypothetical

Did you mean context.session?

I’d support deprecating no-unused-modules and skipping support for using that rule with flat config

I've considered deprecating no-unused-modules (#90) but I changed my idea.

And as long as we don't include this rule in the flat config, everything would be fine.

@silverwind
Copy link
Author

I'd definitely keep it at this point, it's too valuable and not everyone wants to install another dependency and fight its config just to replace one rule.

@michaelfaith
Copy link

michaelfaith commented Aug 4, 2024

Yeah

that’s currently only hypothetical

Did you mean context.session?

Yeah, I wouldn't really say "hypothetical". Proposed is accurate though. The two functions that I used in that implementation were added in a POC branch of the eslint repo to prove the proposed api. The idea is that the code would use those functions if they were on the context, and if not, fall back to the old implementation. And if, between now and when the API is finalized, those functions get introduced with a slightly different shape, then the implementation would need to be adjusted. It'd be the same here, if that's the route you want to go. I did use the POC branch in the eslint repo to work against, though. So it did prove the solution out.

@michaelfaith
Copy link

Oh, and to add a little more context: that api very likely would have already been added to ESLint in v9, if it weren't for the feet-dragging from the eslint-plugin-import project in validating the approach. If the API is ever going to be added, there needs to be an expressed demand for it. So if you want it, this could be a good way to express that demand. Food for thought.

@michaelfaith
Copy link

I started a draft PR, with everything except the changes to no-unused-module. I wanted to confirm that that's the direction you want to go before implementing it.

@controversial
Copy link

controversial commented Aug 5, 2024

that’s currently only hypothetical

Did you mean context.session?

Yes, I’m talking about context.session.isDirectoryIgnored() and context.session.isFileIgnored()! These were proposed by nzakas as a way to enable fs.walk approaches to replicate the functionality of the FileEnumerator API. There’s a POC in a branch of eslint, but hasn’t been merged into eslint proper, as @michaelfaith explained!

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

Successfully merging a pull request may close this issue.