Imports with foreign file extensions fail with "Unable to find" file error

[peplin]

When upgrading from 5.0.1 to 5.12.1, I am running into a new problem with imports of foreign files, e.g. .svg, .webp, and .scss.

As an example, there is a file client/src/components/pages/settings/login-logo.tsx with:

import Logo from "client/src/static/images/logo/logo.svg";

When I run the newer version of knip (5.12.1), it exits with an internal error:

Error: Unable to find /workspaces/obsidian/client/src/static/images/logo/logo.svg
    at ProjectPrincipal.analyzeSourceFile (file:///workspaces/obsidian/node_modules/knip/dist/ProjectPrincipal.js:168:19)
    at analyzeSourceFile (file:///workspaces/obsidian/node_modules/knip/dist/index.js:231:61)
    at main (file:///workspaces/obsidian/node_modules/knip/dist/index.js:291:17)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async run (file:///workspaces/obsidian/node_modules/knip/dist/cli.js:23:73)
    at async file:///workspaces/obsidian/node_modules/knip/dist/cli.js:90:1

From instrumenting the code with a few debug statements, I found that the issues is triggered when the TypeScript getResolvedModule(...) function returns a module for this import. knip eventually tries to load the source file for this module, but it doesn't exist in the file manager.

I've tried but so far failed to create a minimal reproduction repo. There must be an edge case in my particular private monorepo that's triggering this bug. In all of my repro attempts so far, the .svg is not resolved to a module, so the problematic code is never triggered.

I was able to work around this by adding custom no-op compilers (as mentioned in #504), but I gather from the code that these foreign file types are supposed to be automatically excluded. The issue also goes away if I comment out this line.

[webpro]

Thanks for the report @peplin, but I do need a reproduction, otherwise there's nothing to test against.

There's some coverage already, so similar imports give no issues (e.g. https://github.com/webpro/knip/blob/main/packages/knip/fixtures/module-resolution-non-std/src/index.ts)

[peplin]

I don't know what I was doing wrong yesterday, but this morning I was able to create a minimal repro: https://stackblitz.com/edit/github-8sedwy

One potential clue: this is happening in an npm workspace. When you run npm install (or yarn install), a relative symlink is created in node_modules pointing to that workspace. I think that's what leads TypeScript to resolve these files to a module. If I temporarily removed the symlink, knip works.

[webpro]

This is an bit of an odd repro. Knip is supposed to be installed in the root workspace. And perhaps the culprit is that there's an app workspace, so inside that workspace, I wouldn't expect import specifiers to start with app/.

[peplin]

I updated the repro to move knip into the root workspace devDependencies list - same error.

[webpro]

My main point remains: when inside the app workspace, importing from app/... is not supported. It may work with TypeScript, but it's going "out and in" of the workspace, which confuses Knip.

[peplin]

That's unfortunate, especially because it worked in prior releases. Why would we only have this problem with foreign files, like SVG and .scss? It seems like knip is very close to supporting absolute imports

We intentionally have our workspaces configured to only allow absolute imports, so if those won't be supported by knip officially, would you recommend adding custom no-op compilers for all forieng files as a workaround?

[webpro]

Knip should support "absolute" imports like TypeScript does, but it doesn't (and shouldn't) support "crossing boundaries" of workspaces like this. I'd expect the same issue for any file. In general my recommendation would be to separate workspaces properly to group dependencies in workspaces, to group reported issues, etc etc. Personally I would not even recommend such "absolute" imports, but that's yet another discussion.

[peplin]

Thanks for the clarification. I don't think I understand what you mean by crossing boundaries.

I updated the repro to have 2 separate workspaces to avoid the problem of importing app/... from within app, and I get the same error:

// Importing a .ts from the same workspace using an absolute path works fine
import 'app/src/component';

// Importing a .ts from the 'library' workspace works fine
import 'library/src/index';

// Importing a "foreign" file raises an "Unable to find" erorr,  regardless of if its in a different workspace:
import 'library/src/image.svg';

// or in the same workspace:
import 'app/src/App.scss';

I narrowed it down further: the problem was introduced in 47ff3eb. The new resolver resolves a file if it finds an exact match with the imported path. That causes it to be treated as an internal import by knip, and we get the internal error when knip is surprised not to find the file in the file manager.

Previuosly, when ts.resolveModuleName was used instead, it did not resolve a file, so the import is treated as unresolved. Eventually it's analyzed by this code and the "foreign" files are ignored.

[webpro]

Alright, you're not giving up, I like that.

By "crossing boundaries" I mean that if you are inside the app/ workspace (e.g. in app/src/component.ts), then use an absolute path which means going to the root and thus outside the workspace, and then with the import specifier starting with app/ going in again. Not saying it's wrong per se, but this feels very odd to me.

If something "works" doesn't always necessarily mean it's a good idea. What we are combining here are two separate things: module resolution (TypeScript) and workspaces (dependency management). E.g. if module resolution works as expected we can still publish broken packages, or vice versa.

Now that we got this far I'll try and look into it soon.

[webpro]

I've ensured that Knip does no longer throw as reported.

• Foreign files should not result in exceptions or be reported as unused.
• You can still add compilers for file types that can import files.
• Module resolution is still the same
  • The "src/App.scss" specifier in app/src/index.tsx in the repro isn't resolved (and shouldn't be, since app/ is not a root from TS perspective)
  • library is an unlisted dependency, because it's defined as a workspace

For details, also see this fixture and what issues it reports:

• https://github.com/webpro/knip/blob/main/packages/knip/fixtures/module-resolution-non-std-absolute/self/index.ts
• https://github.com/webpro/knip/blob/main/packages/knip/test/module-resolution-non-std-absolute.test.ts

[webpro]

🚀 This issue has been resolved in v5.13.0. See Release 5.13.0 for release notes.

Using Knip in a commercial project? Please consider sponsoring me.

[peplin]

Thank you!

[webpro]

Thanks for the report, there was definitely a bug in Knip