Skip to content

Commit e06d4b2

Browse files
authored
vtsls: Remove redundant TypeScript installation (#60951)
Context: While looking at #60618, I found out the TypeScript 7 upgrade only breaks the `typescript-language-server` adapter, not the default vtsls adapter. After digging in, it turns out the initial vtsls implementation in #12606 followed the same two-package installation pattern as the `typescript-language-server` adapter: it installed both `@vtsls/language-server` and the latest `typescript`. However, vtsls already bundles a compatible TypeScript version via `@vtsls/language-service` and uses it whenever a workspace SDK is unavailable. I also cross-checked an intermediate vtsls upgrade issue, #18349, where we used the installed TypeScript version to decide whether both packages needed updating, so a new vtsls release could go uninstalled when TypeScript hadn't changed. #20197 fixed that by checking both packages independently, but kept the unnecessary standalone TypeScript installation. Changes: This PR removes that redundant installation and version tracking from the vtsls adapter. Zed now tracks only `@vtsls/language-server` and relies on its declared TypeScript dependency. Workspace TypeScript and Yarn SDK detection remain unchanged, so vtsls still uses a workspace SDK when one is available and otherwise falls back to its bundled version. As for the consequence of this change: 1. New users are not affected since npm installs vtsls's own pinned TypeScript dependency anyway. 2. For existing users, the previously installed standalone `typescript` was never resolvable by vtsls (its pinned version gets installed nested when the versions conflict), so leaving it there is harmless. To verify the standalone copy was really unused, I nuked the vtsls dir and ran main (without this fix), which installed both the top-level `typescript@7.0.2` and vtsls's own pinned `typescript@5.9.3` nested under `@vtsls/language-service`. Node's module resolution shows vtsls can only ever see the nested copy as well as the LSP logs from the running server shows, all lib definition locations point at the nested copy: ``` "uri": "file:///.../vtsls/node_modules/@vtsls/language-service/node_modules/typescript/lib/lib.es5.d.ts" ``` I also tested this branch with a fresh install TypeScript files work the same as before. Release Notes: - N/A
1 parent afc13dc commit e06d4b2

1 file changed

Lines changed: 7 additions & 33 deletions

File tree

crates/languages/src/vtsls.rs

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ impl VtslsLspAdapter {
3939
const PACKAGE_NAME: &'static str = "@vtsls/language-server";
4040
const SERVER_PATH: &'static str = "node_modules/@vtsls/language-server/bin/vtsls.js";
4141

42-
const TYPESCRIPT_PACKAGE_NAME: &'static str = "typescript";
4342
const TYPESCRIPT_TSDK_PATH: &'static str = "node_modules/typescript/lib";
4443
const TYPESCRIPT_YARN_TSDK_PATH: &'static str = ".yarn/sdks/typescript/lib";
4544

@@ -84,29 +83,20 @@ impl VtslsLspAdapter {
8483
}
8584
}
8685

87-
pub struct TypeScriptVersions {
88-
typescript_version: Version,
89-
server_version: Version,
90-
}
91-
9286
const SERVER_NAME: LanguageServerName = LanguageServerName::new_static("vtsls");
9387

9488
impl LspInstaller for VtslsLspAdapter {
95-
type BinaryVersion = TypeScriptVersions;
89+
type BinaryVersion = Version;
9690

9791
async fn fetch_latest_server_version(
9892
&self,
9993
_: &Arc<dyn LspAdapterDelegate>,
10094
_: bool,
10195
_: &mut AsyncApp,
10296
) -> Result<Self::BinaryVersion> {
103-
Ok(TypeScriptVersions {
104-
typescript_version: self.node.npm_package_latest_version("typescript").await?,
105-
server_version: self
106-
.node
107-
.npm_package_latest_version("@vtsls/language-server")
108-
.await?,
109-
})
97+
self.node
98+
.npm_package_latest_version(Self::PACKAGE_NAME)
99+
.await
110100
}
111101

112102
async fn check_if_user_installed(
@@ -135,11 +125,8 @@ impl LspInstaller for VtslsLspAdapter {
135125
async move {
136126
let server_path = container_dir.join(Self::SERVER_PATH);
137127

138-
node.npm_install_latest_packages(
139-
&container_dir,
140-
&[Self::PACKAGE_NAME, Self::TYPESCRIPT_PACKAGE_NAME],
141-
)
142-
.await?;
128+
node.npm_install_latest_packages(&container_dir, &[Self::PACKAGE_NAME])
129+
.await?;
143130

144131
Ok(LanguageServerBinary {
145132
path: node.binary_path().await?,
@@ -156,8 +143,7 @@ impl LspInstaller for VtslsLspAdapter {
156143
_: &Arc<dyn LspAdapterDelegate>,
157144
) -> impl Send + Future<Output = Option<LanguageServerBinary>> + use<> {
158145
let node = self.node.clone();
159-
let typescript_version = version.typescript_version.clone();
160-
let server_version = version.server_version.clone();
146+
let server_version = version.clone();
161147
let container_dir = container_dir.clone();
162148

163149
async move {
@@ -175,18 +161,6 @@ impl LspInstaller for VtslsLspAdapter {
175161
return None;
176162
}
177163

178-
if node
179-
.should_install_npm_package(
180-
Self::TYPESCRIPT_PACKAGE_NAME,
181-
&container_dir.join(Self::TYPESCRIPT_TSDK_PATH),
182-
&container_dir,
183-
VersionStrategy::Latest(&typescript_version),
184-
)
185-
.await
186-
{
187-
return None;
188-
}
189-
190164
Some(LanguageServerBinary {
191165
path: node.binary_path().await.ok()?,
192166
env: None,

0 commit comments

Comments
 (0)