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 report files not used by other workspaces in a private monorepo? #140

Closed
zyulyaev opened this issue May 26, 2023 · 11 comments
Closed
Labels
discussion Discussion

Comments

@zyulyaev
Copy link

zyulyaev commented May 26, 2023

Consider the following monorepo structure:

/apps
- /frontend
- /backend
/libs
- /shared
- - index.ts
- - foo.ts
- - bar.ts

And the content of the /libs/shared/index.ts is:

export * from './foo'
export * from './bar'

Let's imagine neither frontend nor backend app uses the symbols exported by the /libs/shared/bar.ts file. The package is not published either, so there is absolutely no way for the bar file to be used. However, it seems like knip considers it to be used since it is re-exported by the entry file (index.ts).

Is there a way for me to configure knip to report the bar file as unused?

@zyulyaev zyulyaev added the discussion Discussion label May 26, 2023
@zyulyaev zyulyaev changed the title How to report files not used by other workspaces in a private monorepo How to report files not used by other workspaces in a private monorepo? May 26, 2023
@webpro
Copy link
Owner

webpro commented May 26, 2023

Strictly spoken, the file is used, since it's imported from another file.

What you're probably after (from the perspective of how Knip currently works) is two things:

  1. the option to mark re-exports from non-entry files as unused (an early version of Knip actually had this)
  2. to mark a file as unused when all exports are unused

Will keep this issue open to track interest.

@zyulyaev
Copy link
Author

Oh, I must have put it wrong. It's not so much about the file, but unused code. I can see that in the following example foo is reported as unused:

file: foo.js
=======

export const foo = () => 'foo'

file: bar.js
=======

export * from './foo'
export const bar = () => 'bar'

file: index.js
=======
import {bar} from './bar'

console.log(bar())

So I'm looking for a similar behavior even when you import another package in the monorepo. Like if instead of import {bar} from './bar' I had import {bar} from '@libs/bar'

@webpro
Copy link
Owner

webpro commented May 28, 2023

In that case you can use --exclude nsExports to ignore all star imports. nsExports stands for namespaced exports which includes both * and * as MyNamespace exports.

@zyulyaev
Copy link
Author

@webpro this does not seem to help... I created a repo with reproduction: https://github.com/zyulyaev/knip-playground
If I run npx knip in the root of this repo (both with and without --exclude nsExports) it reports no issues. However, the bar function defined in /packages/shared/bar.mjs is never used by any code in the monorepo. So I wish I could somehow configure knip to report it as being unused.

@webpro
Copy link
Owner

webpro commented May 31, 2023

Yes, that would come back to my first point of #140 (comment)

@zyulyaev
Copy link
Author

zyulyaev commented May 31, 2023

I'm not so familiar with knip, but this does not sound relevant to my issue:

the option to mark re-exports from non-entry files as unused (an early version of Knip actually had this)

If you mean that in this example:

file: foo.js

export foo = () => 'foo'

=====
file: index.js

export * from './foo'
export bar = () => 'bar'

since foo is re-exported from a non-entry file it should be treated as unused - I don't think that's the behavior I'm looking for. First of all if we follow this logic, bar will still be treated as used even if it is not used by any other code in the monorepo. And secondly, foo may actually be used by other code in the monorepo.

As far as I understand the core issue here is that packages in a monorepo do not share any information about individual symbols/files being used by each other, right? I concluded that because when I removed index.js from the list of entry files - knip reported everything as being unused, even though most of the symbols/files were used by other packages in the monorepo.
I totally get that for packages published to the npm registry, it makes total sense to treat everything exported from entry files as being used since you have no way to track actual usages.

But I think being able to track usages between packages in a monorepo could be very useful for cases when the packages are private. I obviously can't speak for the whole industry, but I feel like the pattern of having a single monorepo for everything a company creates has become pretty common.

@webpro
Copy link
Owner

webpro commented May 31, 2023

I think we mean the same thing, but now that I look it up I think I was mixing up the --include-entry-exports option (the thing that Knip had before, but wasn't widely used, also see #73) which is ultimately what I meant, and what I think you want here. It's another way of saying "do not exclude entry files from reporting unused exports".

As far as I understand the core issue here is that packages in a monorepo do not share any information about individual symbols/files being used by each other, right?

Knip should see usage across packages in a monorepo.

And I agree with the overall sentiment of "self-contained repos". Even just for finding "everything that's exported" it could be a useful flag.

@webpro
Copy link
Owner

webpro commented Jun 15, 2023

I've re-introduced the --include-entry-exports flag in v2.14.0-next.0.

Any chance you could try it out on your project(s)?

You can install with e.g. npm install -D knip@next

@webpro webpro closed this as completed in f911062 Jun 17, 2023
@webpro
Copy link
Owner

webpro commented Jun 17, 2023

🚀 This issue has been resolved in v2.14.0. See Release 2.14.0 for release notes.

@webpro
Copy link
Owner

webpro commented Jun 17, 2023

Went ahead for this one and #142 and I think it's working as expected now :)

Thanks again for reporting this.

@harrisoff
Copy link

harrisoff commented Jul 21, 2023

I run npx knip --include-entry-exports with knip@2.16.0 in the repo https://github.com/zyulyaev/knip-playground and here is the report:

Unused exports in namespaces (2)
bar  packages/shared/bar.mjs
foo  packages/shared/foo.mjs

I think foo is not supposed to be reported because it is imported in packages/app/index.mjs via import { foo } from '@kp/shared'?

Am I missing something? Is there any extra config should be set? 🤔

update: Yes, paths should be set and everything works fine now

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