Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exports previously legitimately reported as unused are no longer reported #708

Open
neelmraman opened this issue Jun 27, 2024 · 1 comment
Labels
regression Something is no longer working

Comments

@neelmraman
Copy link

neelmraman commented Jun 27, 2024

Hi, after seeing that #697 was closed, I updated knip in the repro for that issue (here) to verify.

The behavior on v5.23.0 was that a, b, c, and d were reported as unused while the other two exports that used multiline exports were not reported (even though they were also unused). The new behavior on v5.23.1 is that none of the exports are reported as unused.

@neelmraman neelmraman added the regression Something is no longer working label Jun 27, 2024
@neelmraman neelmraman changed the title Exports previously reported as unused are no longer reported Exports previously legitimately reported as unused are no longer reported Jun 27, 2024
@webpro
Copy link
Collaborator

webpro commented Jun 28, 2024

There's a few things to this one. We're only talking about the combination of:

  • ignoreExportsUsedInFile: true
  • The export { id } notation

Unfortunately, internally when using the TypeScript AST nodes there's a big difference between these notations:

export const id = 1;
const id = 1;
export { id };

With the latter syntax, I couldn't find a reliable way to detect whether id was used more often or not within the same module. That's why I've opted for the stability of assuming export { id } is referenced (which technically could be argued it is, but it could be only the declaration, not other usage).

So this is the situation now, until we've found a reliable way to do it better.

Additional notes:

  • Usually ignoreExportsUsedInFile is used only for types, since values don't need to be exported. E.g. "ignoreExportsUsedInFile": { "interface": true, "type": true }
  • The former syntax doesn't have this issue, so purely from a Knip perspective that would be recommended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Something is no longer working
Projects
None yet
Development

No branches or pull requests

2 participants