[no-unused-modules] Feature Request: Option to ignore exports with usage inside the module

[Ephem]

Both the original issues for this rule (#186, #361) mention finding dead code as the main motivation for implementing the rule. It is useful for that today, but I think this aspect can be improved further.

A common pattern I see is something like the following. The example is borrowed from Redux, but I see this in a lot of other contexts as well:

export const ACTION = 'I_AM_CONSTANT';

export default reducer(state, action) {
  if (action.type === ACTION) { ... }
}

In the above case, ACTION is often not imported and used in another file, but is still exported to signal to other developers that it is fine to do so if they need it. ACTION is not a dead variable, but it is a dead export, but what I want is to determine if it is dead code.

What if this rule could be combined with something like the functionality from no-unused-vars to verify that an export is neither used in another module or in the same one? This would find actual dead code.

The current use of the rule is something like "find out where you can remove the export statement, and possibly dead code", which is a fine use case in itself, so I propose adding this functionality behind an option, which also avoids a breaking change.

Does this seem useful and like something that would fit into eslint-plugin-import in general and no-unused-modules specifically?

[Ephem]

I did a very hacky proof of concept-version of this as a new rule (no-dead-vars) which basically replaced the export-check in no-unused-vars with a more sophisticated one based on no-unused-modules. The original check counts a variable as used as long as it is exported, this version made sure the export was actually used somewhere. That rule entirely replaced no-unused-vars and seemed to work fairly well, but turned into a behemoth of a rule..

I think trying to approach it by using no-unused-modules as the basis would probably make more sense.

[ljharb]

I'm not entirely clear on your goal. In this case (for no-unused-modules), the export of ACTION should be considered unused if a) it's not listed as an entrypoint, ie, for external consumers of a package, and b) no other considered files in the project import it. In other words, it will tell you when it's safe to delete the export token.

After doing so, no-unused-vars would tell you if it's safe to delete the const.

What's the value of a rule that tells you both in one pass?

[Ephem]

The value would be for people who like my team don't consider extra export-tokens as a problem per se to lint for. We use export as a way to express intent, "This is intended to be fine to import and use from elsewhere, even if it isn't currently".

A separate case is where you export internals from a module for testing purposes. While we don't do this heavily, there are definitely cases where we rely on it, so we can't remove the export-token. I guess this specific case could be solved by including the test-files for linting, but then I'm not sure if we would find dead exports that have remaining tests?

For us, turning on no-unused-vars with the intent to use it for dead code removal (as opposed to "dead export removal") gave us so many false positives it wasn't useful which is why I started tinkering. 😄

As an aside, I think having this as a separate rule in eslint-plugin-import would be fine, but possibly weird since it doesn't really directly deal with imports. Having it entirely outside this project as I've implemented it currently incurs a pretty hefty performance penalty however since we can't reuse the existing ExportMap.

[ljharb]

Generally, exporting things just for tests is considered an antipattern, so if something is only imported in tests, it's dead code.

If you get false positives with no-unused-vars, i'd file a bug on eslint - i've not noticed any in years of using it.

The original check counts a variable as used as long as it is exported, this version made sure the export was actually used somewhere.

The existing rule should be checking if it's actually imported somewhere - merely exporting something shouldn't be sufficient with the unusedExports option enabled (which i'd recommend)

[Ephem]

Thanks for engaging and I'm sorry for not expressing this more clearly, let me try to clear things up! 😄

If you get false positives with no-unused-vars, i'd file a bug on eslint - i've not noticed any in years of using it.

It's rock stable! I was talking about no-unused-modules and should have put quotes around "false positives", since it's working as currently intended. What I meant was that we were trying to use no-unused-modules to find dead code which it doesn't currently support, as opposed to finding unused exports which it does.

The original check counts a variable as used as long as it is exported, this version made sure the export was actually used somewhere.

The existing rule should be checking if it's actually imported somewhere

In this part I was talking about no-unused-vars which doesn't have such a check. My hacky tinkering took the approach of adding it, but isn't really important to the bigger discussion.

Generally, exporting things just for tests is considered an antipattern, so if something is only imported in tests, it's dead code.

While I agree (which is why we don't use it very often), there are definitely cases where this is useful, and differing opinions on this being an antipattern. So while I agree, this seems a bit prescriptive for this rule?

To take a step back and reformulate. I want a rule that can find dead code, but no-unused-vars is not useful with exports. no-unused-modules currently does not find dead code, it finds exports that are not imported elsewhere. The intersection of finding exports that are dead code is not currently supported by any (single) lint rule known to me, so if you don't want to remove unused export-tokens for any reason you are out of luck. What I argue is that there are valid reasons to not use the current no-unused-modules, but still want better dead code detection.

To make sure I understand you correctly, are you expressing two arguments against this option?

• This rule and library is only, and should only be, concerned with imports and exports directly and that the proposed option and use case is outside of that scope
• There are no valid use cases for exporting unused things, so no-unused-modules should always be on, which lets no-unused-vars find the dead code

I'm very much interpreting here, so please correct me if I'm putting words in your mouth!

These are both sensible and kind of expected arguments. I guess what I'm arguing is part that there are valid use cases for not wanting to have no-unused-modules turned on and still want to find dead code, and that the performance overhead of implementing this rule outside of this library would be so high that the scope creep of adding that functionality to this rule could still be worth it. Those arguments both come down to for how many this would be valuable, which I can't judge.

Anecdotally I almost never see unused exports in open source, but in my experience from a bunch of commercial projects, it's a common pattern to signal parts of a module that are considered "public" for other developers of the same project.

[ljharb]

I think your analysis is correct (there's rules to remove unused vars, and unused exports, but not dead code entirely). However, if you remove all unused vars and all unused exports and all un-imported files, how would you have any dead code left?

This rule and library is only, and should only be, concerned with imports and exports directly and that the proposed option and use case is outside of that scope

Yes, objectively.

There are no valid use cases for exporting unused things, so no-unused-modules should always be on, which lets no-unused-vars find the dead code

This is a bolder stance for me to take, but yes, that's also my opinion.

[Ephem]

Thanks for explaining! While we do things a bit differently I think this makes sense from the library point of view, supporting every usecase quickly gets a library diluted and unnecessarily complex.

This issue has been open a while with no further interest expressed from others, so I'll go ahead and close it, thanks again for the discussion and your hard work! 💯