webpro-nl / knip

✂️ Find unused files, dependencies and exports in your JavaScript and TypeScript projects. Knip it before you ship it!
https://knip.dev
ISC License
6.47k stars 149 forks source link

Support reading workspace config from `pnpm-workspace.yaml` #50

Closed mcmxcdev closed 1 year ago

mcmxcdev commented 1 year ago

First of all, knip is an amazing tool and I can see this becoming really useful!

We use https://pnpm.io/ as package manager and it took me a while to go through all error messages when executing knip until I was able to make it work. In the end, I had to copy over our workspaces config from pnpm-workspace.yaml to package.json and update the format.

It would be cool to support pnpm out of the box, or at least put a note into the readme for now, so these users are not excluded and can get started with knip.

webpro commented 1 year ago

Sorry you had to go through this. I'm happy to look into supporting pnpm-workspace.yaml as well. Do you happen to have such a repo or know about one?

mcmxcdev commented 1 year ago

I found this through a quick search online: https://github.com/DavidWells/pnpm-workspaces-example

Note that the lockfile format in this project is outdated though, not sure if it matters. I assume a simple pnpm i with the latest version of the package manager would update it accordingly.

You would also need to delete the workspaces config from the projects package.json, I assume it was a mistake by the repo owner, its not needed.

webpro commented 1 year ago

Thanks! Got it.

and it took me a while to go through all error messages when executing knip until I was able to make it work

What kind of errors do you mean here? Anything I could use to reproduce issues?

mcmxcdev commented 1 year ago

Hmm, I think the issue was all on my side actually, my bad. It does work now without any config at all.

What I did was copy over the knip.json content from https://github.com/webpro/knip#usage and only further realized that there is a guide for monorepos further down below.

So although it's clear that it can't work then, it should display a useful error message. This is what I got repeatedly:

$ npx knip
  92% of files processed (195 of 210)
   95 Unlisted or unresolved dependencies

Processing: libs/shared/ui/src/lib/MultiSelectWithAdd/MultiSelectWithAdd.spec.tsx
file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:123
                if (referencedDependencies?.has(dependency))
                ^

RangeError: Maximum call stack size exceeded
    at isUnreferencedDependency (file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:123:17)
    at file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:138:70
    at Array.find (<anonymous>)
    at isUnreferencedDependency (file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:138:46)
    at file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:138:70
    at Array.find (<anonymous>)
    at isUnreferencedDependency (file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:138:46)
    at file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:138:70
    at Array.find (<anonymous>)
    at isUnreferencedDependency (file:///home/user/Webdevelopment/Freelancing/org/repo/node_modules/.pnpm/knip@1.7.0/node_modules/knip/dist/dependency-deputy.js:138:46)

Node.js v18.12.1
webpro commented 1 year ago

I have released v1.8.0 that reads pnpm-workspace.yaml (and updates the docs). Note that only workspaces in the Knip config are part of the analysis.

Without any configuration, this is the default: https://github.com/webpro/knip/blob/06220bc336e912cda3f9589004f97280b4b7e1e9/src/constants.ts#L8-L13

When the default project glob is applied that might be the issue causing the RangeError, although 210 files isn't that much. After configuring the workspace packages in knip.json, do you still see this error?

mcmxcdev commented 1 year ago

Thanks for the update, it works really well now without any config for us!

I don't see the error at all anymore, it was just due to custom bad config. So the only thing you can do is provide a custom error message instead of it running into out of range.

webpro commented 1 year ago

Thanks for reporting back.

it works really well now without any config for us!

Just saying, Knip does not take workspaces into account if there is no workspaces: {} in a knip.json file, so it analyzes the codebase as a single workspace/package.