🐛 Knip does not resolve package.json exports quite correctly

[maastrich]

Prerequisites

• [X] I'm using the latest version
• [X] I've read the relevant documentation
• [X] I've searched for existing issues
• [X] I've checked the list of known issues
• [X] I've read the issue reproduction guide

Reproduction url

https://github.com/maastrich/repro/pull/2

Reproduction access

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

Description of the issue

First, thanks for the amazing tool, it's really impressive how quick and effective knip is 👏

When setting up exports using glob syntax in the package.json, typescript can resolve * to be either a path part or multiple path part:

"./*": "./src/public/*/index.ts" // can resolve to
   -> "./src/public/foo/bar/index.ts"
   -> "./src/public/baz/index.ts"

but knip doesn't

For the moment I've found a workaround where I add this in my package.json so knip can resolve le path (leaving both as typescript resolve the first one and knip the second)

"./*": "./src/public/*/index.ts",
+ "./**": "./src/public/**/index.ts",

[webpro]

Thanks for report. That's correct according to the Node.js docs and honestly I'm a bit baffled by it:

* maps expose nested subpaths as it is a string replacement syntax only.

All instances of * on the right hand side will then be replaced with this value, including if it contains any / separators.

From a Knip perspective, the issues I have with this:

• a lot more files might become entry files and this isn't great in terms of finding unused exports (which aren't reported by default in entry files)
• unsure about the performance penalty, Knip has to traverse the fs and find all matching files (a runtime does string replacement first and then starts resolution).
• people who'd be baffled like me might unintentionally use * thinking it actually means * in glob pattern matching, with subtly unexpected results both in exposed exported paths as in Knip analysis.

What about this workaround, would this work:

"./*": "./src/public/**/index.ts",

Just curious, not saying it's a solution.

[maastrich]

Unfortunately the workaround you proposed is not working with rollup (but typescript handles it) I was more thinking about an option in the knip config as a "quick fix" saying

pkgJsonExport: "glob" | "node";

where glob is the default (to remain non-breaking) and would become node in a future major ?

[webpro]

Just saying, the generic (and intended) solution to this is to add entry patterns to your knip config manually:

{
  "entry": ["./src/public/{a,b}/index.ts"]
}

At this point this feels better than rewriting * to globstar ** and marking "everything" as an entry file (the node option), even though it goes 100% against the "zero-config aim".

[maastrich]

I'll have to try it, but I think that next major i there is one should handle exports as specified in node.js docs

[webpro]

Understood, but this is a good example of static analysis versus runtime behavior and there are tradeoffs to be considered.

[acidoxee]

I can confirm Knip does seem to have issues when subpath exports are used. Unused files/exports detection on our side is quite wrong (by hundreds of items), and I haven't found a solution yet.

Configuring paths in Knip to emulate those subpath exports seems to yield worse results, and (if I understand correctly) adding all files in entry isn't a solution as we use a very broad "./*": "./src/*.ts" subpath export for some packages ("core" packages that any other package can import from). We've added some of the files in there in entry as they are the "main" files expected to be imported from other packages, but other files in there just serve as utils/types/other stuff for those main files. If they're not useful anymore, we'd like to know.

Do you think I'm misconfiguring/misusing the tool somehow? Or is that a fact that subpath exports are a known issue and must be worked on before going further? Thanks 🙏

[webpro]

There's a known issue as described above. Also, feel free to create a reproduction I could look into.

[acidoxee]

Sorry for not providing a reproduction, I tried one here that best resembles my project setup with minimum complexity.

Workspace b is using only a subset of workspace a. In the current state, packages/a/src/utils/unused.ts should be flagged as an unused file/all of its exports should be flagged as unused, and in packages/a/src/utils/used.ts, only the aFoo variable is being used, so the aBar export/variable should be flagged as unused.

The repo uses TS project references and workspaces (here npm, although mine is Yarn, but I don't believe it makes a huge difference), so the dependency between workspaces is seen both through the project references' setup (packages/b/tsconfig.json contains a project reference to packages/a/tsconfig.json) and through the regular dependencies (packages/b/package.json has a "@internal/a": "*" dev dependency). You can make sure that everything works properly by running npm run b (I've made this script readily available in the root), it should simply display aFoo.

Both workspaces define a "exports": { "./*": "./src/*.ts" } subpath export, which basically makes every file available from the outside. So technically, I guess this warrants an entry: ['./src/**'] config in Knip as every file could be accessed, so that's what I've done here. But despite that, we still want to know whether some of those files (and the exported variables inside) are unused, hence adding includeEntryExports: true to Knip's config.

Despite all this, if you run npm run knip (which I've setup to just target the packages/a workspace), the current result is the following, which isn't what I expected:

Unused exports (3)
unusedVar  unknown  packages/a/src/utils/unused.ts:1:14
aFoo       unknown  packages/a/src/utils/used.ts:1:14  
aBar       unknown  packages/a/src/utils/used.ts:2:14

Do you reckon my usage makes sense for Knip to analyze? Am I misconfiguring or misunderstanding something? Can you confirm the current configuration is correct (or not) and would work if this current issue were to be tackled?

[webpro]

@acidoxee Feel free to create a separate issue. The reproduction is incomplete and then still I can't reproduce incorrect behavior.

[acidoxee]

@webpro Sorry if this is incomplete, I thought everything was laid out properly. I'm fairly new to Knip, so do you see a config issue in the reproduction? Because I did my best to put in there exactly what's broken (at least from what I can tell), nothing more, nothing less.

Why, to you, is it incomplete? Why do you say you can't reproduce, when running npm run knip in my reproduction shows exactly what (I think) is not working correctly, and to me seems to be exactly what this current issue and the duplicate one you linked are about? Is it just missing some proper config on Knip's side until package exports are fully resolved? Am I misunderstanding some here? 🙏

[fregante]

• a lot more files might become entry files and this isn't great in terms of finding unused exports (which aren't reported by default in entry files)

I think that this configuration essentially kills knip exports detection because it means:

• all exports are public

However, it does make sense in monorepos thanks to includeEntryExports

• every export is public
• every import is "public" too (within the repo), so "usage" can be detected

• unsure about the performance penalty, Knip has to traverse the fs and find all matching files

It is what is is in monorepos, 😅 my config has to look like this anyway:

  entry: ["src/**!", ...libEntryFilesExclusions],

• people who'd be baffled like me might unintentionally use * thinking it actually means * in glob pattern matching

Yeah here it's not a glob, but rather:

• key: 1 or more characters, until the end of the string or whatever comes after *
• value: use the matched string (as you would with $1)

---

So yeah there are other solutions (entry overrides in knip.js) but it would be best to have knip resolve exports as per spec: https://nodejs.org/api/packages.html#package-entry-points

[webpro]

For this to happen a few things had to come together, in short:

• Introduction of inputs with Options so we can start specifying when exports of entry files should be included in the analysis in a more fine-grained manner. For instance, default exports of Astro Pages or Next.js exports like getServerSideProps are consumed by the framework so we can't just include all exports of all entry files without loads of false positives.
• Globs not "leaking" into descendant workspaces (handled as part of #1015)

Anyway, I've started implementing this and added some tests and fixtures. Would be great if you could try it out on your repo(s) to verify before it goes out:

npm i -D https://pkg.pr.new/knip@606bce7

The --debug logging now has output like to verify glob patters, matching files, and the files that have their exports included in the analysis or not, look for messages like this:

Finding entry paths from plugins (skip exports analysis)

[...]

Finding entry paths from plugins

EDIT: For the record, to include exports of entry files the analysis it's still to required to add isIncludeEntryExports: true or --include-entry-exports. What has happened in the today's changesets is:

• translate * to ** in package.json#exports to fix the original issue of OP
• enable --include-entry-exports for entry files from package.json

[fregante]

Context: my repo has approximately this config:

// To be excluded in production mode. Do not add `!` here, it's automatic.
const devFiles = [
  "**/testUtils/**",
  "**/storybookUtils/**",
  "**/testHelpers.*",
  "**/*.test.*",
  "**/*.stories.*",
  "scripts/**",
];

/**
 * Blocks cross-package import of dev files.
 * If they're meant to be shared, they should be in libs/util-test for example
 */
const libEntryFilesExclusions = devFiles.map((pattern) => `!${pattern}!`);

/**
 * Detect usage of all files. During production mode, exclude devFiles
 */
const projectDefault = ["**!", ...devFiles.map((pattern) => `!${pattern}!`)];

return {
  workspaces: {
    "libs/*": {
      entry: ["src/**!", ...libEntryFilesExclusions],
      project: projectDefault,
    }
  }
};

and libs have this in package.json

  "exports": {
    "./package.json": "./package.json",
    "./*": "./dist/*"
  },

If I drop entry or just "src/**!", knip starts reporting hundreds of unused files and dependencies, which suggests that it's not picking up the correct entry files at all.

Note that I do not have --include-entry-exports yet.

This probably has something to do with https://github.com/webpro-nl/knip/issues/1007#issuecomment-2766749291: if I drop the src/** from entry, then knip will use the exports map that contains ./dist/*, which already appears not to work in my case, as suggested in the linked comment.

Positive note: I did not see any regressions after installing the new pre-release 👌

[webpro]

@fregante Let's keep focus on the OP in this issue. Feel free to open a separate issue with a reproduction, because this isn't actionable.

[fregante]

I don't understand your comments. You asked us to test it. I tested it and it does not change it. I included context instead of just saying "no luck."

Let's keep focus on the OP in this issue

No luck, it does not work.

[webpro]

While I appreciate you've tested this patch, the snippets do not seem to represent what the original issue is about. I've tried to handle the issue and the patch I've posted seems effective in the reproduction.

The patch doesn't seem to work as you expect in your situation, which is unfortunate. If you want your particular issue—which may include additional variables such as source mapping, git-ignored files, etcetera—to be looked into properly a minimal and complete reproduction is needed. The context as provided is not actionable.

[webpro]

:rocket: This issue has been resolved in v5.49.0. See Release 5.49.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

[webpro]

Today we've tried this out on a pretty large monorepo and all seems well. Including source mapping. To get an idea of what's currently supported, see https://github.com/webpro-nl/knip/blob/main/packages/knip/fixtures/package-entry-points/package.json#