Skip to content

Commit de5c878

Browse files
divybotlittledivy
andauthored
fix(lsp): preserve URL extensions in typeof import(...) hovers (#34565)
## Summary When hovering over a value whose type is a remote module namespace, vscode displayed `typeof import('https://.../mod')` instead of the URL the user actually wrote (`typeof import('https://.../mod.ts')`). The cause was that the LSP's custom document registry (`cli/tsc/98_lsp.js`) built source files via `ts.createLanguageServiceSourceFile` but never set `sourceFile.moduleName`. Without `moduleName`, tsc's `getSpecifierForModuleSymbol` falls through to `getModuleSpecifiers`, which strips `.ts` from URL specifiers. The CLI path (`cli/tsc/97_ts_host.js`) already sets `moduleName = specifier` after creating a source file, so this only affects the LSP. Mirror that behavior on the LSP side by assigning `moduleName` in both `acquireDocumentWithKey` and `updateDocumentWithKey`, but only for non-`file:` specifiers. Skipping `file:` keeps tsc's relative-path computation for local files and prevents the internal `/$node_modules/` rewrite from leaking into hovers for npm packages. Fixes #16058. Closes denoland/orchid#320 ## Test plan - [x] `cargo test -p deno --lib lsp::tsc::tests` — the existing `test_modify_sources` already exercises this exact code path through a `Property X does not exist on type 'typeof import("URL")'` diagnostic; its expected message is updated to keep `.ts`. - [x] Outdated comment block in `display_parts_to_string` that documented the stripped-extension behavior as a tsc limitation has been removed (#16058 is fixed). Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent f455715 commit de5c878

2 files changed

Lines changed: 24 additions & 31 deletions

File tree

cli/lsp/tsc.rs

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2076,36 +2076,6 @@ fn display_parts_to_string<'a>(
20762076
// should decode percent-encoding string when hovering over the right edge of module specifier like below
20772077
// module "file:///path/to/🦕"
20782078
to_percent_decoded_str(&part.text),
2079-
// NOTE: The reason why an example above that lacks `.ts` extension is caused by the implementation of tsc itself.
2080-
// The request `tsc.request.getQuickInfoAtPosition` receives the payload from tsc host as follows.
2081-
// {
2082-
// text_span: {
2083-
// start: 19,
2084-
// length: 9,
2085-
// },
2086-
// displayParts:
2087-
// [
2088-
// {
2089-
// text: "module",
2090-
// kind: "keyword",
2091-
// target: null,
2092-
// },
2093-
// {
2094-
// text: " ",
2095-
// kind: "space",
2096-
// target: null,
2097-
// },
2098-
// {
2099-
// text: "\"file:///path/to/%F0%9F%A6%95\"",
2100-
// kind: "stringLiteral",
2101-
// target: null,
2102-
// },
2103-
// ],
2104-
// documentation: [],
2105-
// tags: null,
2106-
// }
2107-
//
2108-
// related issue: https://github.com/denoland/deno/issues/16058
21092079
),
21102080
}
21112081
}
@@ -6578,7 +6548,7 @@ mod tests {
65786548
"line": 2,
65796549
"character": 17
65806550
},
6581-
"messageText": "Property \'a\' does not exist on type \'typeof import(\"https://deno.land/x/example/a\", { with: { \"resolution-mode\": \"import\" } })\'.",
6551+
"messageText": "Property \'a\' does not exist on type \'typeof import(\"https://deno.land/x/example/a.ts\", { with: { \"resolution-mode\": \"import\" } })\'.",
65826552
"sourceLine": " if (a.a === \"b\") {",
65836553
"fileName": specifier,
65846554
}

cli/tsc/98_lsp.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,22 @@ function getCompilationSettings(settingsOrHost) {
4242
return /** @type {ts.CompilerOptions} */ (settingsOrHost);
4343
}
4444

45+
/**
46+
* Assigns `moduleName` on a source file so that tsc displays the original
47+
* module specifier (e.g. in `typeof import("...")`) for non-file URL
48+
* specifiers like `https://...`. For `file://` specifiers we leave it
49+
* unset so tsc keeps its usual relative-path computation and so internal
50+
* details like the `/$node_modules/` rewrite don't leak into hovers.
51+
*
52+
* @param {ts.SourceFile} sourceFile
53+
* @param {string} fileName
54+
*/
55+
function setSourceFileModuleNameForDisplay(sourceFile, fileName) {
56+
if (!fileName.startsWith("file:")) {
57+
sourceFile.moduleName = fileName;
58+
}
59+
}
60+
4561
// We need to use a custom document registry in order to provide source files
4662
// with an impliedNodeFormat to the ts language service
4763

@@ -105,6 +121,12 @@ const documentRegistry = {
105121
if (scriptSnapshot.isClassicScript) {
106122
sourceFile.externalModuleIndicator = undefined;
107123
}
124+
// Preserve the original module specifier so that `typeof import(...)`
125+
// displays the URL exactly as the user wrote it, including extensions.
126+
// Without this, tsc's `getSpecifierForModuleSymbol` recomputes the
127+
// specifier via `getModuleSpecifiers`, which strips `.ts` from URL
128+
// imports like `https://example.com/mod.ts`. See denoland/deno#16058.
129+
setSourceFileModuleNameForDisplay(sourceFile, fileName);
108130
documentRegistrySourceFileCache.set(mapKey, sourceFile);
109131
}
110132
const sourceRefCount = SOURCE_REF_COUNTS.get(fileName) ?? 0;
@@ -170,6 +192,7 @@ const documentRegistry = {
170192
if (scriptSnapshot.isClassicScript) {
171193
sourceFile.externalModuleIndicator = undefined;
172194
}
195+
setSourceFileModuleNameForDisplay(sourceFile, fileName);
173196
documentRegistrySourceFileCache.set(mapKey, sourceFile);
174197
}
175198
return sourceFile;

0 commit comments

Comments
 (0)