Support subpath import with arbitrary extensions #723
Conversation
|
| Name | Link |
|---|---|
| 🔨 Latest commit | 2fb135c |
|
commit: knip
|
| ...ts.sys, | ||
| // We trick TypeScript into resolving paths with arbitrary extensions. When | ||
| // a module "./foo.ext" is imported TypeScript only tries to resolve it to | ||
| // "./foo.d.ext.ts". TypeScript never checks whether "./foo.ext" itself exists. |
There was a problem hiding this comment.
Don't you mean module.ext.ts and module.ext.d.ts (and same for .mts + .cts)?
There was a problem hiding this comment.
I was confused by this as well but it is correct. If the import has an unknown extension like .../foo.ext typescript looks for .../foo.d.ext.ts (code in TypeScript).
There was a problem hiding this comment.
TIL! Thanks for figuring that out + the link, much appreciated.
| @@ -147,3 +155,19 @@ export function createCustomModuleResolver( | |||
|
|
|||
| return timerify(resolveModuleNames); | |||
| } | |||
|
|
|||
| const declarationPathRe = /^(.*)\.d(\.[^.]+)\.ts$/; | |||
There was a problem hiding this comment.
This matches a lot, but I guess I'm having the same confusion as in the comment: I think you're saying "TypeScript tries to resolve foo.ext to foo.d.ext.ts", which I doubt is true? Doesn't it resolve index to index.d.ts or anything.really to anything.really.d.ts?
Just to be sure: do you mean Knip doesn't report false positives, or we've stopped opening issues for bugs? :) |
webpro
force-pushed
the
main
branch
3 times, most recently
from
d91d007
to
d4121d9
Compare
Good point, 😅. Knip stops reporting “unresolved” for imports like |
| @@ -35,6 +34,32 @@ export function createCustomModuleResolver( | |||
| const customCompilerExtensionsSet = new Set(customCompilerExtensions); | |||
| const extensions = [...DEFAULT_EXTENSIONS, ...customCompilerExtensions]; | |||
|
|
|||
| const virtualDeclarationFiles = new Map<string, { path: string; ext: string }>(); | |||
|
|
|||
| const tsSys: ts.System = { | |||
There was a problem hiding this comment.
Unless there's a good reason not to, I think we could move virtualDeclarationFiles and tsSys out of the function scope, easier for moving/refactors later.
There was a problem hiding this comment.
My thinking behind this was that virtualDeclarationFiles is specific to the instance of resolveModuleName that is created by createCustomModuleResolver. I think it is theoretically possible for two instances with different compiler settings to interfere with each other. Also, keeping virtualDeclarationFiles local and not global allows it to be garbage collected. Whether all this is important in practice, I don’t know. But conceptually I feel like stateful things should be localized.
If you agree, I’ll keep it there, otherwise I can move it out.
| const declarationPathRe = /^(.*)\.d(\.[^.]+)\.ts$/; | ||
|
|
||
| /** | ||
| * For paths that look like `.../foo.d.yyy.ts` returns path `.../foo.yyy` and |
There was a problem hiding this comment.
Trying to use "foo" as little as possible myself, any chance you could rid of that too? I guess "module" could do.
| @@ -75,7 +75,6 @@ | |||
| "smol-toml": "^1.1.4", | |||
| "strip-json-comments": "5.0.1", | |||
| "summary": "2.1.0", | |||
| "tsconfig-paths": "^4.2.0", | |||
Stop reporting unresolved filesfor subpath imports with arbitrary extensions (e.g. `import "#src/foo.ext"`). This already worked for path mappings defined in `tsconfig.json` but now it also works for mappings defined in `package.json`. The implementation drops `tsconfig-paths` and reverts back to using virtual declaration files, albeit much simpler.
geigerzaehler
force-pushed
the
all-path-mapping
branch
from
12fc0dd
to
a269b0c
Compare
|
Thanks @geigerzaehler, excellent stuff! |
|
🚀 This pull request is included in v5.26.0. See Release 5.26.0 for release notes. Using Knip in a commercial project? Please consider becoming a sponsor. |
Knip stops reporting issues for subpath imports with arbitrary extensions (e.g.
import "#src/foo.ext"). This already worked for path mappings defined intsconfig.jsonbut now it also works for mappings defined inpackage.json.When trying different approaches I found it easiest to go back to the initial idea of fooling TypeScript into believing files with non-code extensions exists. But the implementation is much simpler now. And I tried to document the approach in the code.