Fixes #313 by correctly using projects moduleResolution strategy if configured #333
Conversation
| @@ -158,8 +158,15 @@ function emitDirectModuleImportWithName(moduleSpecifier: string, node: Node, con | |||
| result = (context.program as any)["getResolvedModuleWithFailedLookupLocationsFromCache"](moduleSpecifier, fromSourceFile.fileName); | |||
| } else { | |||
| const cache = (context.program as MaybeModernProgram).getModuleResolutionCache?.(); | |||
| let mode: tsModule.ModuleKind.CommonJS | tsModule.ModuleKind.ESNext | undefined = undefined; // tsModule.ModuleKind.ESNext; | |||
| if ((context.ts.isImportDeclaration(node) && !node.importClause?.isTypeOnly) || (context.ts.isExportDeclaration(node) && !node.isTypeOnly)) { | |||
There was a problem hiding this comment.
!node.importClause?.isTypeOnlyShould this instead be:
(node.importClause && !node.importClause.isTypeOnly)Because this import doesn't have an import clause, but it shouldn't be treated like a type only import:
import 'foo';Hm, actually, looking at the docs for getModeForUsageLocation, I think we should just pass the node itself and not have any of these checks, including the typeOnly check:
/**
* Calculates the final resolution mode for a given module reference node. This is generally the explicitly provided resolution mode, if
* one exists, or the mode of the containing source file. (Excepting import=require, which is always commonjs, and dynamic import, which is always esm).
* Notably, this function always returns `undefined` if the containing file has an `undefined` `impliedNodeFormat` - this field is only set when
* `moduleResolution` is `node16`+.
* @param file The file the import or import-like reference is contained within
* @param usage The module reference string
* @returns The final resolution mode of the import
*/There was a problem hiding this comment.
Yeah I tried to do that without the checks but it broke in TS 4.8.3 while passed in later TS versions. Not sure why exactly.
But yeah this conditional I just copied from elsewhere in this file so I'm not sure its the right one exactly.
There was a problem hiding this comment.
Hmm thinking on it more, this conditional issue you raised might explain why i was still seeing issues in my larger repository.
There was a problem hiding this comment.
How does it break in 4.8.3?
There was a problem hiding this comment.
Hmm testing it now its not reproducing so maybe a red herring. I have simplified the conditional as you suggested above and it seems to be behaving as expected.
…ModeForUsageLocation
|
Thanks for this great change! I'll stamp a release for this on Monday |
|
Nice! Thanks @rictic hopefully this solves a bunch of problems for folks. |
|
Published as v2.0.2 of lit-analyzer on npm and v1.4.1 of runem/lit-plugin in the vscode extension marketplace |
getNodeForUsageLocationto properly determineModuleKindfor resolving modules.resolveModuleNameFromCacheto properly pass the right mode argument to resolve in more situations correctly.compilerOptionsto not overwrite the project under tests tsconfig options if provided. Will still default to the original behavior, but will now obey whats in thetsconfig.jsonif its specified.mainand were updated automatically.