knip icon indicating copy to clipboard operation
knip copied to clipboard

🐛 Fails to parse TypeScript config that comes from npm

Open Zamiell opened this issue 11 months ago • 5 comments

Prerequisites

Reproduction access

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

Description of the issue

Hello again, and thanks for the great tool.

I have a GitHub action called get-package-names. When I use Knip, it says that the "dist/main.js" file is unused, but that is a bug, because it is clearly listed in the "action.yml" file:

name: get-package-names
description: Gets the packages inside of a monorepo with optional filters

branding:
  icon: package
  color: gray-dark

inputs:
  script-name:
    description: The name of a "package.json" script. If specified, only monorepo packages that contain a script with the specified name will be returned.
    required: false
    default: ""

outputs:
  package-names:
    description: 'The names of the directories that were found in the monorepo "packages" directory. For example: ["foo", "bar"]'

runs:
  using: node20
  main: dist/main.js

Steps To Reproduce

git clone [email protected]:complete-ts/get-package-names.git
cd get-package-names
git checkout 7e955cf48cae33280c54f14add6aaf8079f022c3
npm ci
npx knip

Zamiell avatar Mar 02 '25 13:03 Zamiell

From the looks of it, there's a few related things at play here:

  • Knip tries explicitly to not analyze built/compiled artifacts, and if a tsconfig.json exists that helps Knip to redirect e.g. dist/main.js to src/main.ts then only the latter will be part of the analysis (also see https://knip.dev/features/monorepos-and-workspaces#source-mapping)
  • Knip respects .gitignore files, which often contains items like dist to ignore build artifacts from the report, so if this item or the file is missing then the file will be reported as unused (assuming it's part of project files)

webpro avatar Mar 02 '25 16:03 webpro

  • For bullet point 1, it looks like that is not happening, even though "./src/main.ts" exists and "./dist/main.js" exists.
  • For bullet point 2, GitHub actions should not have "dist" inside the ".gitignore", because the build artifacts need to be committed to the repository. Thus, in my project, "dist" is not in the ".gitignore", and this is both intended & idiomatically correct.

Zamiell avatar Mar 02 '25 16:03 Zamiell

With regards to the source mapping, you can see this in --debug mode (there should be something with "rewiring"). If both files exist, only the src version is included in the analysis, and the other one is then unused (also assuming it's part of project files).

There are a few options, though:

  • remove dist files from the project files, rough examples look like project: ["src/**"] or project: ["**", "!dist/**"] (recommended)
  • exclude from the report by using something like ignore: ["dist/**"] (not recommended)

webpro avatar Mar 02 '25 17:03 webpro

Oh, I think the issue is actually just that Knip fails to read my TypeScript configuration properly:

Unable to resolve config:complete-tsconfig/tsconfig.base.json

(The full output is here.)

As you can see, there is nothing in the logs about rewiring. My guess is because the "include" prop of the config is in the upstream config.

Does Knip support TypeScript configs that come from npm packages? (That's something I would expect it to support, since it is common for people to use e.g. @tsconfig/strictest.)

Edit - I'll change the title of this PR, since you helped me find the real problem.

Zamiell avatar Mar 02 '25 17:03 Zamiell

Does Knip support TypeScript configs that come from npm packages?

Yes.

Knip should not log "unable to resolve", but add complete-tsconfig to the "Unlisted dependencies" instead (as it's not listed in package.json). I'll see what I can do to fix up this output.

webpro avatar Mar 03 '25 05:03 webpro

Closing this issue as part of a general cleanup to keep this project sustainable and optimize my time working on it. If you think this is inappropriate or if there is no workaround and you feel stuck, feel free to open a new issue. Thanks for your understanding.

webpro avatar Aug 10 '25 07:08 webpro