Eslint-plugin-import: rule: no-unused

Created on 22 Feb 2016  路  16Comments  路  Source: benmosher/eslint-plugin-import

On first execution, build an index of imports from some source root(s) pattern (i.e. /src/.*\.js) and then flag all exports that are not used.

Subsequent executions using the same base would benefit from a cache.

Could try using a makefile to build the import cache in a file to maintain it across executions + build it fast? Might be a useful strategy for caching exports between executions, too.

FS cache should live in the node_modules/eslint-plugin-import folder? or just the system tmp dir?

Probably should configure base as a rule parameter? until it benefits some other rule, anyway.

Could also mark full modules unused if nothing imports a given file.

rule proposal

Most helpful comment

FWIW, the no-cycle rule introduced some infrastructure that should make this straightforward.

The na茂ve implementation could just crawl the defined entry points' dependency graphs for all used exports, cache it using the existing cache rules, and work off of that when linting.

The real question is how time-intensive this will be:

just crawl the graphs for all used exports

Feel free to take a stab at it! (again, using no-cycle as a reference for crawling the export map graph)

All 16 comments

Definitely 鉂わ笍 this idea.

I've implemented something like this for @github but its pretty app specific. I was researching a way it could be extracted into a proper eslint plugin. It'd be rad if it was was part of eslint-plugin-import.

My original thought was that _this_ plugin wouldn't have to be responsible for building the index per se. If you spec'd out the format of the index file, an application specific Makefile could be responsible for generating it. Currently we have a mix of ES6 imports and older CJS stuff. You'll also want to special case "entry points" somehow since those won't directly be imported, but maybe referenced by some html script tag in the app. Our could see our indexer looking for ES imports, CJS and these custom html templating references.

It'd still be great for a standard index builder to be part of eslint-plugin-import itself to all all the easy cases. But I'd interested in having a custom hook into that point.

My original thought was that this plugin wouldn't have to be responsible for building the index per se

I would be totally up for that if you or someone else can point me to something that would generate an index on the FS I could write the rule against. Less is more. 馃槑

You'll also want to special case "entry points" somehow

I'm imagining this could be a new responsibility of the resolvers, i.e. Node resolver exposes either the package.json main field(s) or root index.js, and the Webpack resolver would read the entry points out of the relevant config file(s). Could be implemented in v3 of resolvers as getEntryPoints(context|filename) => String[] or something similar.

Could also allow explicit config of entry points in a setting or rule param, but IMO the less of that, the better.

Idea: the resolvers can provide a list of entry points. Node resolver uses package.json and defaults to index.js. Webpack resolver uses "entry" field.

Using the memo parser, it shouldn't technically add much time, either.

Any news or updates on this rule? Having this rule would be super beneficial (yay, deleting code).

Let me know how I can help out!

FWIW, the no-cycle rule introduced some infrastructure that should make this straightforward.

The na茂ve implementation could just crawl the defined entry points' dependency graphs for all used exports, cache it using the existing cache rules, and work off of that when linting.

The real question is how time-intensive this will be:

just crawl the graphs for all used exports

Feel free to take a stab at it! (again, using no-cycle as a reference for crawling the export map graph)

Is this rule meant to report the modules themselves not being used somewhere? Or should it also/instead report individual exports (in case that there are multiple exports within a single module) without any usage? (just to make sure I don't misunderstand the aim of this rule... )

@rfermann every export - default or named - in every file.

The rule would also need an option so that for published packages, entry points could be exempted.

Well...too bad. I only have a prototype solution for checking the usage of the modules themselves, just as it is mentioned in #361, although created independent of this issue.

What about reopening #361 and creating 2 different rules e.g. "no-unused-modules" and "no-unused-exports"?

I鈥檓 not sure what you mean - you have a rule that just checks if the file paths are referenced?

I鈥檓 not sure i see the value of checking that a file is used or not if we already had a way to check if every export is used or not - iow, the former seems like a subset of the latter.

@ljharb: I have a first draft of a rule checking for referenced paths, yes.

In my opinion, these rules are close to each other. But I don't think, that one is a subset of the other.

Let's say, there is a module with some code in it. This module lacks any export statements (because someone deleted all export statements by accident, or for any other reason). It would pass the "no-unused-exports" rule without being reported.

That's a really good point.

I still think it should be the same rule tho - maybe no-unused-modules, with an "ignore" option for entry points, and separate options to control checking for specific exports, and checking files that have no exports.

That means, we will have a rule that reports:

  • unused exports (indicating, that these exports can possibly be removed from the module; option "unusedExports")
  • modules without any exports (indicating unused modules; option "missingExports")

This rule might work different than my existing solution, but I will give it a try. Although I didn't get yet, why there needs to be an "ignore" option for the entry points. Isn't that handled by the resolvers?

@rfermann no - published packages may have files that are never imported, but are correct and need to remain, for external consumers. Not every project being linted is an app :-)

Hm...as it turns out, I misunderstood this point. The term "entry point" led me in the wrong direction.

Closed with #1142.

Was this page helpful?
0 / 5 - 0 ratings