Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Knip module resolution doesn't take account of TypeScript rootDirs from referenced project #873

Open
6 tasks done
pawelblaszczyk5 opened this issue Dec 8, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@pawelblaszczyk5
Copy link

Prerequisites

Reproduction url

https://github.com/pawelblaszczyk5/knip-root-dirs-repro

Reproduction access

  • I've made sure the reproduction is publicly accessible

Description of the issue

Hello 👋

In one of my monorepos, where I use Knip I'm migrating from Remix to React Router. They're using rootDirs from TypeScript for some funky typesafety codegen stuff. I'm also using project references to split up settings for e.g. tests and app code. While importing from this "magically" mapped stuff I'm getting "unresolved imports" errors.

In my reproduction you can see these behaviours:

  1. Running pnpm knip fails with unresolved imports error
  2. Running pnpm knip --tsConfig tsconfig.source.json on the other hand works so Knip can correctly resolve rootDirs
  3. Uncommenting rootDirs in main tsconfig.json also works, so same as 2.

It seems like Knip isn't correctly using stuff from projects that are referenced from root tsconfig.json. I'm not sure but maybe related to these two: #780 #779

@pawelblaszczyk5 pawelblaszczyk5 added the bug Something isn't working label Dec 8, 2024
@webpro
Copy link
Collaborator

webpro commented Dec 9, 2024

Thanks Paweł, useful to have that repro.

Looks related to the issues you mention indeed.

Hopefully easy to resolve in Knip, but need to dig some deeper. For starters, Knip uses the same config loader as tsc itself. Having references.path point to another TS config file does not result in configs getting merged or something. E.g. tsc --build or bundler takes care of that, and Knip will need to do something similar. Just not sure yet if and how to merge those TS configs.

@pawelblaszczyk5
Copy link
Author

That's interesting, in theory just tsc won't even resolve anything in this file because it's not included in the root tsconfig.json. As you mentioned - you have to use tsc --build to run typechecking for all the referenced projects.

Also afaik, it's not really "merging" it's just using completely different config based on which one of the references includes a given file. That should maybe be a little bit easier, because you don't need any merging logic? I'm not knowledgable with Knip internals enough 😄 Also curious how LSP does it ootb without any additional changes

@webpro
Copy link
Collaborator

webpro commented Dec 9, 2024

It's different config but a "Go To Definition" still resolves it properly, so I was also thinking of looking into LSP stuff.

Fwiw, it's quite literally sending compilerOptions from ts.parseJsonConfigFileContent to ts.resolveModuleName.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants