Support resolving jest-junit file dependencies

[tryggvigy]

This PR

Adds support for resolving dependencies from jest-junit reporter configuration.
See https://github.com/jest-community/jest-junit?tab=readme-ov-file#configuration

Files declared through the combination of

testCasePropertiesFile,
testCasePropertiesDirectory,
testSuitePropertiesFile,
testSuitePropertiesDirectory,

from jest-jsunit configuration are now correctly reported as used.

Things to consider

• The code is a bit involved. We might want to consider moving resolving reporters like this to it's own file or directory separate from the plugin?
• I'm pretty sure according to the docs that both files are always required when jest-jsunit is used. They both have default values in the configuration. But could be good to just double check so we don't report unused files incorrectly if these files don't exist and should not exist. I'm not sure if jest-jsunit can work with the absence of properties file or a suit properties file or both.

[pkg-pr-new]

Open in Stackblitz

bun add https://pkg.pr.new/knip@835

commit: 1132265

[webpro]

This PR

Adds support for resolving dependencies from jest-junit reporter configuration. See https://github.com/jest-community/jest-junit?tab=readme-ov-file#configuration

Files declared through the combination of

testCasePropertiesFile,
testCasePropertiesDirectory,
testSuitePropertiesFile,
testSuitePropertiesDirectory,

from jest-jsunit configuration are now correctly reported as used.

Excellent

Things to consider

• The code is a bit involved. We might want to consider moving resolving reporters like this to it's own file or directory separate from the plugin?

Maybe we could add e.g. ./helpers.ts (some other plugins do this too).

• I'm pretty sure according to the docs that both files are always required when jest-jsunit is used. They both have default values in the configuration. But could be good to just double check so we don't report unused files incorrectly if these files don't exist and should not exist. I'm not sure if jest-jsunit can work with the absence of properties file or a suit properties file or both.

Knip does not need to verify functional/correctness (e.g. if jest-jsunit would crash on missing file we don't need to report that because it should be fixed anyway).

If we know that items are files (and can't be dependencies), we can use toEntry (instead of toDeferResolve) to be more specific. Same would go for dependencies (toDependency) Either way, I think entry files that don't exist are ignored.

[tryggvigy]

Either way, I think entry files that don't exist are ignored.

What I'm seeing is, if for example I delete packages/knip/fixtures/plugins/jest/junitProperties.js that was added in this PR we get a new unresolved file. So am I understanding you correctly that this knip ignores those unresolved files in the final report as it's not really in scope for knip as a tool to verify correctness in this way?

[webpro]

Either way, I think entry files that don't exist are ignored.

What I'm seeing is, if for example I delete packages/knip/fixtures/plugins/jest/junitProperties.js that was added in this PR we get a new unresolved file. So am I understanding you correctly that this knip ignores those unresolved files in the final report as it's not really in scope for knip as a tool to verify correctness in this way?

How it currently works if the configured file would not exist:

jUnitReporterDeps.push(toEntry(testSuiteFilePath));             // unused file + ignored
jUnitReporterDeps.push(toDeferResolveEntry(testSuiteFilePath)); // unused file + ignored
jUnitReporterDeps.push(toDeferResolve(testSuiteFilePath));      // unused file + unresolved

So in where you quote me I was referring to the toEntry and toDeferResolveEntry options. But I realize that was rather confusing, sorry about that.

Let's forget about toEntry for a moment. Number 2 and 3 behave different, because of this:

knip/packages/knip/src/util/get-referenced-inputs.ts

Line 106 in 153a836

if (!isDeferResolveEntry(input) && !isConfigPattern(input)) {

This exception is there because recently did I large refactor (to introduce the Input type and all those toDeferResolve etc. functions) and I didn't want the risk to introduce too many false positives.

So the question remains, should we use toDeferResolve here? In this case I'd say it's safe enough and a good idea. So I'm going to merge this.

Btw this does give me good input to tinker about, this whole Input idea has only just been implemented like 2 weeks ago, so thanks for that!

[webpro]

🚀 This pull request is included in v5.37.0. See Release 5.37.0 for release notes.

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

[tryggvigy]

Great! Thanks for reviewing the PR and taking the time 🎉

[webpro]

Thanks for your help! Also see https://bsky.app/profile/webpro.nl/post/3lavdqcaecc2n :)

[schnuri-schnuri]

Hello,

since updating to knip 5.37.0 (Issue persists in 5.37.1), Knip complains about "unresolved imports" errors regarding junitProperties.js and junitTestCaseProperties.js. I feel this has to do with that change.

As far as I understand this I do not need to have these files in the repository for jest-junit to work. Is there something I'm missing?
Should I open an issue?

Thank you :)

[webpro]

Oops, yes please file an issue and we'll look into it shortly.

[tryggvigy]

Yeah its what i feared! I was wrong that both files are always required. We should only identify deps explicitly defined in the config here