knip icon indicating copy to clipboard operation
knip copied to clipboard

TypeScript Namespace - Unsed exported constants not detected

Open weilbith opened this issue 9 months ago • 6 comments

Aloha

I'm aware of the following unused/dead code in my legacy project, but knip does not detect it. I know version 5 added some "namespace" support, but this does not work unfortunately.

export namespace Foo {
  export const UNUSED_VARIABLE = "bar"
}

I can verify with other language tools, but even with a plain search that this UNUSED_VARIABLE is nowhere used in the whole repository. Unfortunately knip, as great as it is, does not detect this. Is that maybe already known? I couldn't find an related issue. Sorry, if there is one.

PS: Please don't ask me about the above code. 😑

weilbith avatar Apr 30 '24 10:04 weilbith

In the meanwhile, while trying to get rid of some namespaces, I realized this. If I convert the above code, it still does not recognize it as being unused. Which I assume is related on an abstract level?

export const Foo {
  UNUSED_VARIABLE: "bar"
}

I tried to use knip --include classMember, but no success. But maybe your logic to detect class member usage might be helpful for this?

weilbith avatar Apr 30 '24 10:04 weilbith

Only top-level exports are considered. I guess an exception should be made for namespaces indeed.

If by any chance you'd like to give it a shot you could start here: https://github.com/webpro/knip/blob/main/packages/knip/src/typescript/getImportsAndExports.ts#L279-L289

Now that I look at it, this looks oddly unfinished: https://github.com/webpro/knip/blob/main/packages/knip/fixtures/module-block/types.ts

Otherwise I'll pick it up at some point.

The second example is not a class, so there are no classMember either. It's just an exported named Foo object, and regular props are not considered named exports.

webpro avatar Apr 30 '24 11:04 webpro

I'm having a VERY similar problem which has nothing to do with TS namespaces. [email protected] is definitely broken, it's reporting wrong stuff as Unused exports in our CI. I'm gonna downgrade for now.

AndreaPontrandolfo avatar Apr 30 '24 15:04 AndreaPontrandolfo

@AndreaPontrandolfo To be quite honest, I'm not sure how that's helping out @weilbith and/or this project here? The title of this issue is "TypeScript Namespace". Feel free to open a new issue with more details, such as a reproducible test case. Or, indeed, downgrade.

webpro avatar Apr 30 '24 16:04 webpro

@weilbith Could you please create a minimal reproduction of the issue so I can see how things are imported and exported, including relevant TS config? I want to make sure the fixes are correct without introducing unexpected/breaking changes (or decide what needs to go into a next major if it is).

webpro avatar May 03 '24 09:05 webpro

Hmm. I struggle to do so. 🙈 My approach to provide a minimal TypeScript project with everything:

$ mkdir /tmp/knip-test-namespace
$ cd /tmp/knip-test-namespace
$ npm init
$ npm add --save-dev typescript knip
$ npx tsc --init

Creating at least two source files to create the relationship of an used and unused export:

other.ts:

export namespace Foo {
  export function bar() {}
  export function baz() {}
}

index.ts:

import { Foo } from "./namespace"

Foo.bar()

Running now npx knip unfortunately always tells me that other.ts is unused. I tried to just add something "else" (read - no namespace) to the file and import it as well. But still the same issue. So I guess I have a little setup issue here. Running npx tsc && node index.js works fine. 🤷🏾

weilbith avatar May 08 '24 15:05 weilbith

Going to close this issue, but feel free to (re)open with an actual reproduction.

webpro avatar Jun 15 '24 13:06 webpro