Imports with foreign file extensions fail with "Unable to find" file error about knip HOT 13 CLOSED

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.

from knip.

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?

from knip.

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)

from knip.

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.

from knip.

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/.

from knip.

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

from knip.

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.

from knip.

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.

from knip.

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.

from knip.

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

from knip.

🚀 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.

from knip.

Thank you!

from knip.

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

from knip.