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
7.06k stars 177 forks source link

Jest configured setup scripts reporting as unused exports when --include-entry-exports is enabled #716

Closed shawnmcknight closed 4 months ago

shawnmcknight commented 4 months ago

I've created a reproduction for this issue at https://github.com/shawnmcknight/knip-jest-entry-exports.

Jest allows for configuration properties globalSetup, globalTeardown, setupFiles, and setupFilesAfterEnv (possibly others, but these are the ones I am aware of and use). The jest plugin for knip will properly detect these files referenced in the jest config as being used except when --include-entry-exports is enabled. In my reproduction I've added a module for each of these configurable jest properties and referenced them in the jest config:

/** @type {import('jest').Config} */
const jestConfig = {
  globalSetup: "./test/globalSetup.ts",
  globalTeardown: "./test/globalTeardown.ts",
  setupFiles: ["./test/setupFiles.ts"],
  setupFilesAfterEnv: ["./test/setupFilesAfterEnv.ts"],
};

module.exports = jestConfig;

When running knip with --include-entry-exports off, knip reports no errors in the reproduction. However, if --include-entry-exports is enabled then knip is reporting: image

My interpretation of the includeEntryExports documentation is that plugins shouldn't behave differently for entry files:

If enabled, Knip will report unused exports in entry source files and scripts such as those referenced in package.json. But not in entry and configuration files as configured by plugins, such as next.config.js or src/routes/+page.svelte.

Please let me know if I can provide anything else to assist with this.

webpro commented 4 months ago

:rocket: This issue has been resolved in v5.24.2. See Release 5.24.2 for release notes.

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

webpro commented 4 months ago

Thanks @shawnmcknight! Plugins don't perfectly always use toEntryPattern properly everywhere yet, but at least the ones you've shown are fixed now.

shawnmcknight commented 4 months ago

@webpro Thanks for taking care of this. The originally reported issue is now fixed, but there has been a regression elsewhere. I can log a separate issue and try to create a reproduction if it helps -- just let me know.

In our jest.config.js for setupFiles and setupFilesAfterEnv, there are some references to a module from an internal workspace:

const jestConfig = {
...
  setupFiles: ['@storis/app_common.test/setup.ts'],
  setupFilesAfterEnv: ['@storis/app_common.test/setupAfterEnv.ts'],
...
};

module.exports = jestConfig;

The @storis/app_common.test package is a reference to another workspace in the monorepo and is declared in package.json as a dev dependency. With this latest update, that dependency is being reported as an unused dev dependency:

image

Prior to this latest change that was not being reported as an unused dependency. If you need more information let me know and I'll try to work up a reproduction. Thanks!

webpro commented 4 months ago

The day has come.. https://github.com/webpro-nl/knip/blob/d81a00f72c61a0e295341f6a498a0d0b2f6324ac/packages/knip/src/plugins/vitest/index.ts#L27

Will fix :)

webpro commented 4 months ago

Another fix is in the latest & greatest.

shawnmcknight commented 4 months ago

Another fix is in the latest & greatest.

That was really fast! Just tried it out and everything looking good! Thanks! 👍