🐛 inconsistent behavior of unused exports with `--fix` flag

[yishuolin]

Prerequisites

• [X] I'm using the latest version
• [X] I've read the relevant documentation
• [X] I've searched for existing issues
• [X] I've checked the list of known issues
• [X] I've read the issue reproduction guide

Reproduction url

https://stackblitz.com/edit/knip-fix-example-y2fu4h2n?file=re-export.js&view=editor

Reproduction access

• [X] I've made sure the reproduction is publicly accessible

Description of the issue

Hi! I’ve encountered an issue related to the detection of unused exports.

Description

When running Knip on the following code without the --fix flag, no issues are reported:

import { A } from './export.js';
A;
export { A } from './export.js';

However, when running Knip with the --fix flag, it correctly detects that A is an unused export.

Expected Behavior

Knip should identify unused exports, regardless of whether the --fix flag is used.

Potential Fix

While reviewing the codebase, I noticed the following line in the collectUnusedExports function:

if (!isFix && exportedItem.isReExport) continue;

Link to the line of code

It seems that re-exports are being excluded unless the --fix flag is used. Maybe we need to adjust the logic to ensure that re-exports are not excluded during the collection of unused exports?

[webpro]

In this small example it may look a bit silly, but the logic is for larger reports where it's not convenient to have duplicates (i.e. re-exports) reported of essentially the same export.

The fix mode removes exports including re-exports, and that's what you're seeing in this minimal example. Note that the includeEntryExports flag is of influence here as well.

While I agree this might be confusing at times, I think the current behavior makes sense. Would you agree? Maybe there are ways we could improve? With better output, or in docs perhaps.

[yishuolin]

Thank you for the explanation. To simplify things, I’ve created a refined reproduction that focuses solely on the issue we discussed (without the includeEntryExports flag).

I agree that avoiding duplicate re-exports in reports is convenient for larger projects. However, IMHO, I still think the behavior should be consistent, and the detailed reports are acceptable—similar to how users expect many issues to be reported when running Knip for the first time.

That said, refining the documentation to clarify this behavior would also work for me.

Would it also make sense to introduce a flag to control whether re-exports are ignored?

[webpro]

Sorry this got neglected. I went ahead and like to see how this will play out.

the behavior should be consistent, and the detailed reports are acceptable

Agreed. In the latest version I've just published with pkg.pr.new that line is simply removed.

However, if an export is re-exported multiple times, each (re)export in the chain is reported. At this point I'm thinking it's better to report each of those, since reporting either only the leaf or the root would mean multiple runs are necessary to find all (e.g. in case --fix is not used).

similar to how users expect many issues to be reported when running Knip for the first time.

This was one of the reason I held off a bit. Only if the reported issues are valid, it's fine if there's a few extra lines for "duplicate" re-exports. For first-time users, if the report has false positives and the list then becomes even longer with unused re-exports (invalid or not) this isn't a great experience. Now that Knip becomes better and better we can be more slightly confident this happens less and less.

Any chance you still have a codebase around and see if it works as expected? If so, you can install version using something like npm i -D https://pkg.pr.new/knip@6c086e5

[yishuolin]

Thanks for the update! I just tested it with the new version and can confirm it works as expected.

[webpro]

Alright, let's roll with it. Thanks for the feedback!

[webpro]

:rocket: This issue has been resolved in v5.48.0. See Release 5.48.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.