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

--include-entry-exports ignores workspaces in knip config #479

Closed
what1s1ove opened this issue Jan 26, 2024 · 6 comments
Closed

--include-entry-exports ignores workspaces in knip config #479

what1s1ove opened this issue Jan 26, 2024 · 6 comments
Labels
discussion Discussion

Comments

@what1s1ove
Copy link
Contributor

what1s1ove commented Jan 26, 2024

Hey guys!

During the investigation of issue #472 , I encountered a problem. I'm not sure if it's a bug or not.

To make knip complain about unused items in the shared module, a suggestion was made to use the --include-entry-exports flag in the #140. This flag works if workspaces are specified in the root package.json. However, if they are specified in the knip config and not in the root package.json, this flag stops working correctly and complains about all files in the shared folder.

For me, this looks like a bug because according to the documentation (https://knip.dev/features/monorepos-and-workspaces), if there are no workspaces in the root package.json, knip should use the workspaces paths specified in the knip config.

The knip config looks like this:

/** @type {import('knip').KnipConfig} */
const config = {
	workspaces: {
		'packages/app': {
			entry: ['index.js'],
		},
		'packages/shared': {
			entry: ['index.js'],
		},
	},
}

export default config

You can see an example repository here - https://github.com/what1s1ove/knip-playground

Steps to reproduce:

  1. Run npx knip --include-entry-exports and see in the console that knip complains only about packages/shared/bar.js.
  2. Remove workspaces from the root package.json.
  3. Reinstall packages (npm i) to regenerate package-lock.json.
  4. Run npx knip --include-entry-exports and see in the console that now knip complains about all modules in the shared folder.
@what1s1ove what1s1ove added the discussion Discussion label Jan 26, 2024
@webpro
Copy link
Collaborator

webpro commented Jan 29, 2024

Not sure I understand the issue. Why would you remove workspaces from package.json in the first place? The repo url provided gives a 404.

@what1s1ove
Copy link
Contributor Author

what1s1ove commented Jan 29, 2024

Not sure I understand the issue. Why would you remove workspaces from package.json in the first place? The repo url provided gives a 404.

Hey @webpro !

Why would you remove workspaces from package.json in the first place?

because I want to setup the knip package in a repository where there are no NPM-workspaces, and they will not appear there in the near future.

The repo url provided gives a 404.

Sorry, I just opened the repo. Could you take a look.

@webpro
Copy link
Collaborator

webpro commented Jan 29, 2024

If workspaces are removed from package.json, the internal packages are also removed from node_modules with an npm install. And TypeScript module resolution can no longer find them.

I guess you'll want to npm install the packages separately again after this? Then running Knip may or may not work as you'd expect, but I'm not sure whether this is still within the scope of Knip's --include-entry-exports, since all packages are now explicitly external to each other.

@what1s1ove
Copy link
Contributor Author

what1s1ove commented Jan 30, 2024

If workspaces are removed from package.json, the internal packages are also removed from node_modules with an npm install. And TypeScript module resolution can no longer find them.

I guess you'll want to npm install the packages separately again after this? Then running Knip may or may not work as you'd expect, but I'm not sure whether this is still within the scope of Knip's --include-entry-exports, since all packages are now explicitly external to each other.

I understand you. However, if I use knip-workspaces instead of NPM-workspaces, shouldn't it continue to work? Or does the --include-entry-exports flag only work with NPM-workspaces?

@webpro
Copy link
Collaborator

webpro commented Jan 30, 2024

if I use knip-workspaces instead of NPM-workspaces, shouldn't it continue to work?

No, Knip workspaces isn't anything like the npm feature. npm does all the heavy lifting like installing dependencies and making those accessible to the runtime (e.g. Node.js), so e.g. your code or Knip can do its thing. So with/out the installation and symlinking of directories in node_modules etc. this is totally different.

The Knip workspaces config is meant to set up entry/project files and connect them with package.json dependencies etc. workspaces. And you can configure extra workspaces, but that has nothing to do with package management/installations.

Or does the --include-entry-exports flag only work with NPM-workspaces?

Yeah that's something you should just try, not sure about it. The symlinks are important here, because that makes Knip think internal packages/workspaces are not in node_modules. But feel free to try and bug me about it, might be easy to fix if it doesn't already, no idea yet.

@what1s1ove
Copy link
Contributor Author

if I use knip-workspaces instead of NPM-workspaces, shouldn't it continue to work?

No, Knip workspaces isn't anything like the npm feature. npm does all the heavy lifting like installing dependencies and making those accessible to the runtime (e.g. Node.js), so e.g. your code or Knip can do its thing. So with/out the installation and symlinking of directories in node_modules etc. this is totally different.

The Knip workspaces config is meant to set up entry/project files and connect them with package.json dependencies etc. workspaces. And you can configure extra workspaces, but that has nothing to do with package management/installations.

Or does the --include-entry-exports flag only work with NPM-workspaces?

Yeah that's something you should just try, not sure about it. The symlinks are important here, because that makes Knip think internal packages/workspaces are not in node_modules. But feel free to try and bug me about it, might be easy to fix if it doesn't already, no idea yet.

Now I know more, thanks! You can close this issue. I'll try to focus on #472.

@webpro webpro closed this as completed Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion
Projects
None yet
Development

No branches or pull requests

2 participants