Resolve JS files referenced by TypeScript declaration files if possible

[junyi]

Context

First of all, thanks for developing this tool!

I am looking into using this tool at work and am running into a false positive scenario where JavaScript files which are referenced by TypeScript declaration files (.d.ts) are showing up as unused.

A summary of the issue looks like this:

// tsconfig.json
{
  "compilerOptions": {
    "baseUrl": "src"
  }
}

// This is index.ts
// This imports from './utils/fn.js' which has a corresponding './utils/fn.d.ts'
// but Knip is reporting './utils/fn.js' as unused
import { g } from 'utils/fn';

I have included in the PR here test & fixtures to describe the issue above.

I believe this has something to do with this part of the code in resolveModuleNames:
https://github.com/webpro/knip/blob/0b02923352940b3129ea8bb411b46e5804b2ce0a/packages/knip/src/typescript/resolveModuleNames.ts#L62-L71

Approach

Create a new function jsMatchingDeclarationFileExists where given a TypeScript declaration file (.d.ts), it checks if there exists a file by replacing the extension to .js and returns the module.

Note: The same behaviour is done for .d.mts to .mjs and .d.cts to .cjs but I am not sure if this is necessary.

This function is used to augment the existing behaviour in resolveModuleNames as follows:

https://github.com/webpro/knip/blob/b521b156c7c805c62e17643172634b52e6081def/packages/knip/src/typescript/resolveModuleNames.ts#L86-L101

[webpro]

Thanks @junyi! Feel free to open a pull request. Looks like you know where to look for 👍 Would be awesome!

One note: please delete any files that are not relevant to the issue.

[litera]

When can we expect this merged and released into knip? I'm having the same issue and for the time being I've mitigated this by adding the javascript file to the configuration ignore list.

[junyi]

Hi @webpro, thanks for the advice! I have updated the PR to include a fix for the module resolution and removed unnecessary files from the tests.

[webpro]

Thanks for the fix! Interestingly, knip reports this issue in its own repo:

Unused files (1)
packages/eslint-config/index.d.ts

I'll dig some deeper when I have time.

[webpro]

OK I think I've figured out why it didn't pass in the Knip repo itself, and I've also added the other DTS extensions during TS init.

@junyi Could you please format the changes (npm run format from root or workspace)?

[webpro]

The changes "broke" tests, they're fixed. If you could wrap it up when/however you like (so you're the contributor of it all) I'm happy to merge!

[junyi]

Thank you for looking into this @webpro! I have pushed a commit to format the changes. Let me know if there's anything else I should do to wrap this up

[webpro]

🚀 This pull request is included in v5.0.4. See Release 5.0.4 for release notes.

[alexeyr-ci]

This doesn't seem to cover the case where the JavaScript file's extension is .jsx.

[webpro]

This doesn't seem to cover the case where the JavaScript file's extension is .jsx.

That's right. It also doesn't cover entry files in certain cases. Feel free to open a PR!

[webpro]

This doesn't seem to cover the case where the JavaScript file's extension is .jsx.

This should be OK in the latest canary (v5.1.3-canary.0)