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

How to handle unused code in a shared package within a monorepo #472

Closed
what1s1ove opened this issue Jan 24, 2024 · 7 comments
Closed

How to handle unused code in a shared package within a monorepo #472

what1s1ove opened this issue Jan 24, 2024 · 7 comments
Labels
discussion Discussion

Comments

@what1s1ove
Copy link
Contributor

what1s1ove commented Jan 24, 2024

Hello, guys!
The question is as follows: currently, when adding any unused imports to the packages/shared module in the monorepo repository, knip does not raise any complaints about them (see the image below). Is there a way to make knip complain about bar?

Knip config:

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

export default config

Project structure:

apps/
  app/
    index.ts
    package.json

packages/
  shared/
    src/
      index.ts
    package.json

knip.config.js
package.json
@webpro
Copy link
Collaborator

webpro commented Jan 29, 2024

So you'd need to use --include-entry-exports, but the issue you linked to shows that you know that. Furthermore, I'd really need a minimal repro to look into (not screenshots or a scenario that I need to imagine).

@what1s1ove
Copy link
Contributor Author

So you'd need to use --include-entry-exports, but the issue you linked to shows that you know that. Furthermore, I'd really need a minimal repro to look into (not screenshots or a scenario that I need to imagine).

Hey @webpro !

I'll share the repository with you a bit later. I'll also share the solution I was able to come up with. It's a bit strange, but it works. Perhaps you can provide some insights on how to improve it.

@Faithfinder
Copy link
Contributor

I don't have a minimal repro currently, but enabling that flag in our repo gave me a billion false positives, unfortunately :(

I would say it doesn't follow source maps, is the problem? Entry file used to detect code usage in the package is src/index.ts, but entry file to the package is dist/index.js (and that's gitignored). I don't think knip understands the relationship between the two?

@what1s1ove
Copy link
Contributor Author

I don't have a minimal repro currently, but enabling that flag in our repo gave me a billion false positives, unfortunately :(

I would say it doesn't follow source maps, is the problem? Entry file used to detect code usage in the package is src/index.ts, but entry file to the package is dist/index.js (and that's gitignored). I don't think knip understands the relationship between the two?

Yes, I had the same issue. I had to use the --no-gitignore flag, add an entry for dist/index.js, and it worked with several exceptions.

@what1s1ove
Copy link
Contributor Author

what1s1ove commented Jan 31, 2024

Hey @webpro!
I have prepared a test repository - https://github.com/what1s1ove/knip-playground-ts

So, once again, the problem I want to solve is - when using a typescript monorepo, when there are unused things in packages/shared, knip should complained about them.

In the example repository, the packages/shared exports two functions, foo and bar. The function foo is used in apps/app, while the function bar is not used anywhere. I want knip to complain about the unused function bar.

As already mentioned, to detect unused items from packages/shared in other applications, the --include-entry-exports flag should be used. However, it's not as simple as that. And as I managed to find out, the issue is not with the flag itself but with how we import items from the packages/shared module.

Since npm cannot import non-js files, it means that we need to convert all files in packages/shared from .ts to .js. In other words, perform a regular build and make the NPM-entry for this package not src/index.ts but build/index.js.

// packages/shared/package.json

{
	"name": "@kp/shared",
	"main": "build/index.js",
	"types": "build/index.d.ts",
}

Accordingly, if knip has the entry set to src/index.ts, it will complain about all files as unused because other applications import items from packages/shared specifically through packages/shared/build/index.js as the entry.

It means we need to specify the entry for knip for packages/shared as build/index.js, and everything should work. But I couldn't make it work either, as knip continued to complain about unused files that were actually used in other applications.
The only thing that helped me solve the problem was to specify the entry point exactly as build/index.d.ts, while ignoring all other d.ts files and all files in the repository.

// knip.js

'packages/shared': {
	entry: ['build/index.d.ts'],
	ignore: ['src/**/*', 'build/helpers/**/*.d.ts'],
},

Only with this configuration, I was able to make knip complain about the unused function bar when using npx knip --include-entry-exports.

@webpro
Copy link
Collaborator

webpro commented Feb 4, 2024

The issue primarily stems from the fact that Knip is about source code, not build artifacts.

Adding build artifacts as entry files to the configuration does not "fix the chain", especially considering they're .gitignored (and also source maps would not get interpreted).

Feel free to bend and flex Knip any way you like, but I just can't support this use case atm for multiple reasons, and I'm not sure if that will ever be the case.

@webpro webpro closed this as completed Feb 4, 2024
@what1s1ove
Copy link
Contributor Author

what1s1ove commented Feb 7, 2024

Hey @webpro!

especially considering they're .gitignored

Exactly! That's why I had to use the --no-gitignore flag.

During my latest experiment with integrating knip into another project, I made a slight improvement to the solution. By specifying the main key in package.json as the path to the built file build/index.js, and the types key as the path to the source TypeScript file src/index.ts, everything works beautifully.

Shared folder packages.json - https://github.com/what1s1ove/knip-playground-ts/blob/main/packages/shared/package.json
Knip config - https://github.com/what1s1ove/knip-playground-ts/blob/main/knip.js

Once again, you rightly noted that this solution won't suit everyone, as I mentioned earlier in the post, since the builds and workflows of all monorepo projects vary, unfortunately. There are so many tools nowadays to achieve the same thing...

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

3 participants