-
-
Notifications
You must be signed in to change notification settings - Fork 203
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
🐛 Not following references
in TS config files
#779
Comments
Knip applies some "source mapping" when the config allows so: https://knip.dev/features/monorepos-and-workspaces#source-mapping to analyze only source files. It's not actually a monorepo feature, it also applies in your case.
This might be the culprit here. In general you don't want to import "resulting" files from tests or source file. Do you mean build artifacts? And maybe you're referring to import specifiers that have the |
Yes, I meant the build artefact. TS requires me to use the
microsoft/TypeScript#55346 or microsoft/TypeScript#59597 (comment) |
Target the actual existing TS source file, and replace the |
That, at least, does not work when In the code box, import { foo } from "../dist/foo.js";
import { bar } from "../src/foo.js";
console.log(foo + bar); Which is left essentially untouched in This may also depends on ESM vs CJS (this is ESM), and of From what I understand of Module specifiers are not transformed, it looks like the import string is never ever transformed, so it should always point to the build artefact even before it exists.
(emphasis not mine) |
Module specifiers are not transformed so they still work after compilation. |
Yes, that a bit the point 😅 Not sure where we're getting 😕
Am I missing something? 🤔 According to knip source mapping:
But this is not what I'm seeing since This may be linked to the directories structure I have with a top-level |
Yes, don't import the file at
Happy to discuss after we've sorted the first point. But I can imagine there'll be issues since Knip doesn't actively support |
But that just plain out doesn't work (from the TS/node point of view).
I guess that's where the problem is. Then knip doesn't see the |
Maybe it "doesn't work" in the situation you've created. There's tools like Indeed, the file doesn't exist. Perhaps another thing to "show it works": if you use |
Well, … yes… In this setup, the only correct (from TS / node point of view) import is from However, knip does not recognise it. Because it does not read the What I assume (maybe wrongly), is that this situation I've created is not inherently wrong and should be handled correctly by knip.
I should not be forced to used another tool 😄 (and runtime compilation is not a good idea to ship a library, I want to be able to pack ES files).
Which is what I'm doing. I'm compiling with knip doesn't follow the |
Not sure what to tell you. You're saying "I can't claim I really understand why at the deepest level" and then insist on doing things this way. It's kinda odd that the test files are compiled into the same directory, and then the resulting JS files are used to exercise the compiled source files. Overall this isn't a best practice and a bit of a mess in the CSB repro. Test runners like Vitest, Bun (or even ts-jest) are there to take the burden away in case the test (and source) files are written in TypeScript. There is no need and there should not be a need for a compilation step of test files. So either everything should be compiled upfront if you insist on running things directly in Node.js without any dependencies. Or, I'd recommend, use a test runner that supports TypeScript. |
outDir
references
in TS config files
The part I don't fully understand is why TS cannot rewrite module specifiers. Which I do not have a choice of doing things any other ways.
Well, I've inherited the repo with a simple test setup not using any runners (granted, it was set up in the early days of TS and TS aware test runners might not have been that common back then). Not enough time or resources to re-architect this when it is effectively working (tests are run, my code is exerciced, bugs and regressions are detected), and even gets a few benefits (low startup time for tests, easy to manually run a single test file for super quick iterations). Anyway, the same situation can be built without tests. I've updated the sandbox to have several directories inside In any case, I know have If I replace the import by So, knip not following |
Guess I steered the discussion sideways myself here, yet I'm also not the one to tell how to organize any project. Still, the repro doesn't meet the requirements:
I'm going to close this one as it's a bit convoluted. I'm happy to discuss the actual issue at hand in a new issue/repro. |
I ran into this issue when looking for For me, I have a project with the following
Which means the project can refer to The workaround I've settled on right now is to add to
The compiler options aren't being applied to any files, so it is essentially a no-op for typescript, but knip will pick this value up. |
Prerequisites
Reproduction url
Code Sandbox
Reproduction access
Description of the issue
I have a TS package with a
src
andtest
directories. Files insrc
are built todist
(which is then included in the package'sfiles
entry). Files intest
are built there (and not exported). This implies onetsconfig.json
in each of these directories.The
src/foo.ts
file creates some internal exports:In real life, this is typically used for stuff that I do not want to put in the package's API but I want to be able to test and thus import from the test files.
The test file then include these as
import { foo } from "./dist/foo.js"
. This is the correct import since TS want them to point tothe resulting filethe build artefact.However, knip reports this as unused:
When the test file instead
import { bar } from "../src/foo.js"
(which is incorrect and creates a runtime error), knip does not report it anymore.So, it seems that knip is not correctly linking the source file with the generated code.
Note: when building the repro, initially
import { bar } from "../src/foo.js"
was creating a build error. Only after removing theoutDir
option, building, then setting it up again (and cleaning) did I land in this situation (no build error, runtime error).The text was updated successfully, but these errors were encountered: