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

Add note(s) to docs about flat configs and/or no-unused-modules' unusedExports not being supported in flat configs #2964

Open
nnmrts opened this issue Feb 18, 2024 · 5 comments

Comments

@nnmrts
Copy link

nnmrts commented Feb 18, 2024

I think for most users of this plugin it's not immediately obvious that flat configs are not or only partially supported. See for example the mentions of this issue in various commits, PRs and issues: #2556.

If I understand correctly after going through some of the issues here, the flat config format in general is not "officially" supported by this plugin, however it's not clear to me what exactly breaks and what not. I only found this (which aims to add support but doesn't have a lot of information) and this comment here:

@keijokapp going from "we do not support flat config" to "we support it except for one rule which is horrible broken" isn't an improvement imo, since it will discourage people from using the rule in a way that will last long after it's eventually made to work.
-- @ljharb

In my opinion it should be made clear somewhere in the readme what "we do not support flat config" actually means. For me personally at least everything except for setting unusedExports to true in no-unused-modules "works" though.

Which brings me to the main point of this issue: Can we document this fact in https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-unused-modules.md? Especially because both the "In the future, these options will be enabled by default (see #1324)" and the example below might encourage readers to set unusedExports to true. Maybe a link to #2907 should be included.

@ljharb
Copy link
Member

ljharb commented Feb 18, 2024

Given that eslint's FlatCompat allows people to use this plugin before it actually supports flat config, I agree adding a note in the readme (linked to from the no-unused-modules docs) here would be good.

@jpzwarte
Copy link

jpzwarte commented Mar 3, 2024

I'm not using the no-unused-modules rule directly, but I am attempting to bundle my ESLint flat config that includes eslint-plugin-import using rollup.

Of all the plugins & rules I'm using, no-unused-modules is the only one that's giving me trouble:

(!) Plugin node-resolve: Could not resolve import "eslint/lib/cli-engine/file-enumerator" in /Users/jzwartepoorte/Projects/components/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js using exports defined in /Users/jzwartepoorte/Projects/components/node_modules/eslint/package.json.
(!) Plugin node-resolve: Could not resolve import "eslint/lib/util/glob-utils" in /Users/jzwartepoorte/Projects/components/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js using exports defined in /Users/jzwartepoorte/Projects/components/node_modules/eslint/package.json.
(!) Plugin node-resolve: Could not resolve import "eslint/lib/util/glob-util" in /Users/jzwartepoorte/Projects/components/node_modules/eslint-plugin-import/lib/rules/no-unused-modules.js using exports defined in /Users/jzwartepoorte/Projects/components/node_modules/eslint/package.json.

My next step is trying to exclude this rule somehow from bundling.

@ljharb
Copy link
Member

ljharb commented Mar 3, 2024

I’m not sure why you’d ever want to bundle an eslint config - and either way, this plugin requires everything to exist on disk to work properly, so it’s not going to work with that approach.

@jpzwarte
Copy link

jpzwarte commented Mar 3, 2024

I'm trying to create a zero dependency ESLint config, which hopefully speeds up ESLint's running time significantly.

As far as I can tell, the paths that node-resolve complains about don't exist?!

@ljharb
Copy link
Member

ljharb commented Mar 3, 2024

That seems like a really bizarre way to solve it; I'd suggest using jest-eslint-runner instead, it does the job without violating the expectations of basically every package.

It seems like the error is coming from your rollup setup - the paths don't necessarily exist, which is why they're being conditionally required in the first place.

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

No branches or pull requests

3 participants