eslint: Change Request: Flat Config does not support FileEnumerator
ESLint version
v8
What problem do you want to solve?
eslint-plugin-import uses FileEnumerator in its no-unused-modules
rule, for the purpose of gathering a list of files are not eslintignored, or ignored by the user’s rule config, for checking if any exports or modules are unused.
It seems that in flat config, this capability does not exist.
What do you think is the correct solution?
Something that may work nicely is a new method on context
provided to rules, that can achieve the same goal, but I have no idea if this makes sense for eslint or not.
Participation
- I am willing to submit a pull request for this change.
Additional comments
This is the sole remaining blocker (afaik) to eslint-plugin-import supporting Flat Config, after which I plan to do a breaking change to drop older eslint versions, which is something quite a lot of users have been asking for.
About this issue
- Original URL
- State: open
- Created 5 months ago
- Reactions: 2
- Comments: 31 (30 by maintainers)
I had some time to look into this. Mainly good news and a bit of bad news.
Good news: I was able to get a working
no-unused-modules
rule based on a limited copy & paste ofFileEnumerator
that receivesisDirectoryIgnored
andisFileIgnored
instead of config logic. So I think that confirms the hope in this thread: that should be enough to support flat config (at the cost of some copy & paste). Reference PR here: https://github.com/import-js/eslint-plugin-import/pull/2967.Bad news: the specific
isFileIgnored
implementation in -> https://github.com/eslint/eslint/tree/issue18087-poc https://github.com/eslint/eslint/commit/4c4c1f4b973b3f2039813025d047a98801c95c59 seems to be giving inconsistent results for whether a file is ignored. So there’s some work to be done there. I didn’t have time to dig deeply yet - but @nzakas I’m guessing you have much more context than me anyway. I put a write up in https://github.com/JoshuaKGoldberg/repros/blob/0458e256d49b3887c2b5e7da27ee932d1a91ffa7/README.md.@ljharb Unfortunately, we’re not going to be able to ship anything until you’ve verified that this approach works. The ball is in your court now and it’s up to you as to whether or not this makes it in for the final v9.0.0.
@JoshuaKGoldberg My hunch is that it’s in the implementation, because we use
isFileIgnored()
frequently in the core and have a ton of tests validating its behavior. I’d suggest instead of trying to implement a newFileEnumerator
, that you just usefswalk
like we do: https://github.com/eslint/eslint/blob/15c143f96ef164943fd3d39b5ad79d9a4a40de8f/lib/eslint/eslint-helpers.js#L19. It does have a walkSync method that should work for this purpose.This might help as an example, @ljharb . I was updating the template I have for typescript-based projects
In the commit
20bfe75
I had set-up “import” in a flat config, and I was getting some errors from it, maybe some config like this might help you explore the issues?Let me know if I can help simplify this any further.
https://github.com/alexrecuenco/typescript-eslint-prettier-jest-example/pull/15/commits/20bfe7579ed88eb1030883a4f48dad97158b6019
Is there a reason you can’t just switch one of your existing repos to use flat config and try it out?
We are blocked on your feedback for this, and we are running out of time to get this into v9, so we’d appreciate you trying this out soon.
no, not just yet - I’ll try to make some time today or tomorrow!
--config
just changes the config file being used and that becomes the config array--ignore-pattern
is merged into the config array--no-ignore
is also used to calculate the config array.So
isDirectoryIgnored()
andisFileIgnored()
takes all of this into account.Here’s the branch: https://github.com/eslint/eslint/tree/issue18087-poc
The way I have this set up is as a
context.session
object that hasisDirectoryIgnored()
andisFileIgnored()
as methods. (This is only available when running in flat config mode.) Each method expects an absolute path to be passed in.Let me know how that works.
--ext
doesn’t exist in flat config – all that data is in the config file and therefore is respected inisDirectoryIgnored()
andisFileIgnored()
.Let me put together a quick POC that you can try out.
That is a risk, for sure, but given that we use the same
isDirectoryIgnored()
andisFileIgnored()
, those are the most likely things that would change and you’d get those passed through to you.Unfiltered would be disastrous because you’d end up getting everything in every
node_modules
(that’s part of the filtering).The glob results of the current run probably won’t work for you because that won’t give you all of the files in the project. For example, if someone runs
npx eslint foo.js
, then the result of the search will only be/path/to/foo.js
. We only search based on the pattern that was passed in, and if I’m understanding you correctly, you’d need more than that if you want to check if a module is unused anywhere in the project.As I said, this isn’t really functionality we want to encourage in rules, so we’re trying to make the surface area to get this working for you as small as possible.
(At some point, I’d like to look at a higher level at what it might look like for ESLint to understand the dependency graph of a project.)
Okay, I’m going to try asking this again: can you please explain exactly the approach/algorithm that you’re using right now to do this? We need to know:
root
?import
statement, or all of the lintable files?There isn’t going to be an exact replacement for
FileEnumerator
, so we need to understand the problem you’re trying to solve, plus the way you already solved it, in order to figure out the approach going forward.