Re-exports with usage of barrel files isn't indexed correctly

[glemiron]

I found an interesting edge case that could be improved.

// orderFixtures folder
export * from './fixture1'
export * from './fixture2'

// orderFixtures/fixture1.ts
export const order1 = {
  id: 1
}

import * as orderFixtures from './orderFixtures'

export const allFixtures = {orderFixtures: orderFixtures}

Then it'll be used in test the following way:

import { allFixtures } from '../fixures'

allFixtures.order1

For this configuration I expect that knip will tell me that I have unused export in fixture2.ts, but instead I getting the following result:

Unused exports (2)
order1  unknown  fixures/orderFixtures/fixture1.ts:1:14
order2  unknown  fixures/orderFixtures/fixture2.ts:1:14

I'm considering to contribute to the project, but I don't sure what kind of test should be added here and how to name it. I believe that it related to modules resolution, but some guidance could help.

Thank you for the awesome work!

Here the minimal reproduction:
ReactPlayground.zip

[webpro]

This is an interesting case, could be a challenge to resolve it.

Assuming the intention of allFixtures.order1 is allFixtures.orderFixtures.order1.

And I'm afraid this is a "bridge too far" currently for Knip, the chain breaks here:

import * as orderFixtures from './orderFixtures';
export const allFixtures = { orderFixtures };

Interestingly, trying "Find all references" in my IDE on the order1 export does not lead me to src/App.jsx. Also TS itself has trouble here, this surprises me. the other way around is fine, from usage to implementation.

The issue here is that it's a property on a new export. It's not an explicit or even implicit re-export:

// explicit
export * from './orderFixtures';

// implicit
import * as orderFixtures from './orderFixtures';
export { orderFixtures };

This kind of works, would come close:

import * as orderFixtures from './orderFixtures';
export { orderFixtures as allFixtures };

Obviously not what you want, but just to show what Knip does support technically. In this last case all exports on the orderFixtures namespace are now considered used, which is something that could/should be improved on. In this scenario I also see that TS "Find all references" leads me to src/App.jsx so this scenario is something we could try and fix in Knip.

Your use case is totally valid JS/TS, it's just not something I see easily fixed in Knip.

If you're interested in working on improving this last case I'm happy to elaborate on the whereabouts of related things :)

[webpro]

Couldn't resist and went ahead on this myself.

This use case should be fixed in v5.14.0 and should properly handle exports individually (before, it assumed all members of the namespace as referenced):

import * as orderFixtures from './orderFixtures';
export { orderFixtures as allFixtures };

During the refactor I went ahead and tried whether supporting the original use case is feasible. I've added support in 5.15.0-keyedreexport.0. Not 100% sure yet I'll add it, but let's see where this goes. Any chance you could give the tagged keyedreexport version a shot on your workload? You can install using e.g.

npm install -D knip@keyedreexport

Happy to hear your feedback, @glemiron!

[webpro]

From TS compiler internals Discord:

I've just come across a case where I think findReferences might have a bug:

• ✅ https://codesandbox.io/p/devbox/github/webpro/knip/main/packages/knip/fixtures/re-exports-aliased-ns?file=%2F1-first.ts%3A1%2C16 - find references of first leads to index.ts
• 🔴 https://codesandbox.io/p/devbox/github/webpro/knip/feat/extend-ns-reexports/packages/knip/fixtures/re-exports-keyed-ns?file=%2F1-first.ts%3A1%2C14 - find references of first does not lead to index.ts
• the difference is how NS is "re-exported" from 4-collect.ts

[glemiron]

Unfortunately it seems that the fix doesn't work. In order to eliminate set up problems I've created a demo repository here.

I also found the second use case, I think it's related and you can also check out it in the repository.

And another observation: WebStorm is detects unused code correctly, so it seems as solvable problem.

[webpro]

Thanks for the demo repo! This is super helpful. Let's break it down a little bit.

1. First case:

import * as orderFixtures from './orderFixtures'
export const allFixtures = { orderFixtures: orderFixtures };  // NOT supported (yet?)
export const allFixtures = { orderFixtures };                 // Supported in @keyedreexport

To this I mentioned that I was "assuming allFixtures.orderFixtures.order1", and the last line in this example is the case that I think is fixed in the keyedreexport release.

2. Next case. This should be fixed and works as expected in main/v5.14.0 (and keyedreexport as well):

import * as orderFixtures from './orderFixtures';
export { orderFixtures as allFixtures };

3. The case that's brought in is this one:

import * as orderFixtures from './orderFixtures'
export const allFixtures = orderFixtures

While valid, but this could also (should?) be written like so:

import * as orderFixtures from './orderFixtures'
export { orderFixtures as allFixtures }

This case worked fine in main prior to opening this issue. Or maybe there's additional code/syntax missing that we've not seen/covered yet?

4. Last but not least you've introduced this case:

import { ValuesType } from 'utility-types'
import * as FEATURES from './constants'
export type Feature = ValuesType<typeof FEATURES>

This is a case that wasn't covered yet and is now added in v5.15.1.

And another observation: WebStorm is detects unused code correctly, so it seems as solvable problem.

Interesting, didn't know about this feature. Do you mean this Find unused code with coverage? That would be a bit different: Knip is a static analysis tool, and I think this feature is instrumenting code and then doing the analysis more dynamically in runtime.

That said, it's a solvable problem indeed. For now I'm going to keep it short here by saying that doing "Find all references" for each export takes too long in large projects (also see Slim down to speed up), but this can still be enabled by using --include-libs (which was added for a different feature and thus is unclear in this context and should be renamed).