Comments (13)
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 inapp/src/index.tsx
in the repro isn't resolved (and shouldn't be, sinceapp/
is not a root from TS perspective) library
is an unlisted dependency, because it's defined as a workspace
- The
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.
Related Issues (20)
- Run the script only against the staged files. HOT 2
- `ignore` is not applied for Auto-fix HOT 1
- Not working with @emotion/styled during string interpolation HOT 1
- 'TypeError: importItems.importedAs.entries is not a function' error when using --cache, but not without the cache HOT 2
- Dependency reported as "unused" even if it's used with require.resolve(...) HOT 5
- Fails to run due to TypeError in `graphql-codegen` HOT 3
- `No "exports" main defined` error with `estree-walker` when using `mdx-js` plugin HOT 4
- "RangeError: Maximum call stack size exceeded" with self import HOT 1
- Binary reported even `ignoreBinaries` is used. HOT 1
- Links are broken all over the docs HOT 2
- Stricter check for `--workspace` paths HOT 3
- `ignoreBinaries` config not respected anymore HOT 6
- Path mapping configuration not respected for .json files HOT 4
- React Rouer V6 lazy api how to ignore HOT 11
- 5.7.0: Failing to detect extensionless imports in some TS configurations. HOT 7
- ts-jest reported as unused after renaming jest.config.js → jest.config.mjs HOT 3
- Git-ignore rules inside a project should not match parts of the absolute path outside the project HOT 5
- require imports no longer work HOT 9
- picomatch error HOT 1
- WebWorker not detected as being used HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from knip.