From c0b3422941bbc2a110c1999ff288ff99aa77aeca Mon Sep 17 00:00:00 2001 From: tidely <43219534+tidely@users.noreply.github.com> Date: Wed, 17 Dec 2025 13:27:06 +0200 Subject: [PATCH] node_runtime: Use `semver::Version` to represent package versions (#44342) Closes #ISSUE This PR is rather a nice to have change than anything critical, so review priority should remain low. Switch to using `semver::Version` for representing node binary and npm package versions. This is in an effort to root out implicit behavior and improve type safety when interacting with the `node_runtime` crate by catching invalid versions where they appear. Currently Zed may implicitly assume the current version is correct, or always install the newest version when a invalid version is passed. `semver::Version` also doesn't require the heap, which is probably more of a fun fact than anything useful. `npm_install_packages` still takes versions as a `&str`, because `latest` can be used to fetch the latest version on npm. This could likely be made into an enum as well, but would make the PR even larger. I tested changes with some node based language servers and external agents, which all worked fine. It would be nice to have some e2e tests for node. To be safe I'd put it on nightly after a Wednesday release. Release Notes: - N/A *or* Added/Fixed/Improved ... --- Cargo.lock | 2 + crates/copilot/src/copilot.rs | 5 +- .../src/wasm_host/wit/since_v0_8_0.rs | 2 + crates/language/Cargo.toml | 1 + crates/language/src/language.rs | 3 +- crates/languages/Cargo.toml | 1 + crates/languages/src/css.rs | 10 +-- crates/languages/src/json.rs | 10 +-- crates/languages/src/python.rs | 11 ++-- crates/languages/src/tailwind.rs | 10 +-- crates/languages/src/typescript.rs | 19 +++--- crates/languages/src/vtsls.rs | 19 +++--- crates/languages/src/yaml.rs | 11 ++-- crates/node_runtime/src/node_runtime.rs | 65 ++++++++----------- crates/project/src/agent_server_store.rs | 20 +++--- crates/project/src/debugger/session.rs | 23 ++++--- crates/project/src/lsp_store.rs | 3 +- crates/project/src/prettier_store.rs | 2 +- 18 files changed, 118 insertions(+), 99 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 080a6a4cf41..20f19de832c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8814,6 +8814,7 @@ dependencies = [ "regex", "rpc", "schemars", + "semver", "serde", "serde_json", "settings", @@ -9048,6 +9049,7 @@ dependencies = [ "regex", "rope", "rust-embed", + "semver", "serde", "serde_json", "serde_json_lenient", diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 45f0796bf53..f248fbdb43e 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -1246,7 +1246,10 @@ async fn get_copilot_lsp(fs: Arc, node_runtime: NodeRuntime) -> anyhow:: .await; if should_install { node_runtime - .npm_install_packages(paths::copilot_dir(), &[(PACKAGE_NAME, &latest_version)]) + .npm_install_packages( + paths::copilot_dir(), + &[(PACKAGE_NAME, &latest_version.to_string())], + ) .await?; } diff --git a/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs b/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs index b2e0a1a4fbb..8b3f8e86b71 100644 --- a/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs +++ b/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs @@ -736,6 +736,7 @@ impl nodejs::Host for WasmState { .node_runtime .npm_package_latest_version(&package_name) .await + .map(|v| v.to_string()) .to_wasmtime_result() } @@ -747,6 +748,7 @@ impl nodejs::Host for WasmState { .node_runtime .npm_package_installed_version(&self.work_dir(), &package_name) .await + .map(|option| option.map(|version| version.to_string())) .to_wasmtime_result() } diff --git a/crates/language/Cargo.toml b/crates/language/Cargo.toml index 49ea681290c..3ba93476d2a 100644 --- a/crates/language/Cargo.toml +++ b/crates/language/Cargo.toml @@ -48,6 +48,7 @@ rand = { workspace = true, optional = true } regex.workspace = true rpc.workspace = true schemars.workspace = true +semver.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index b0805c4ddd9..eceb68f3e57 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -43,6 +43,7 @@ pub use manifest::{ManifestDelegate, ManifestName, ManifestProvider, ManifestQue use parking_lot::Mutex; use regex::Regex; use schemars::{JsonSchema, SchemaGenerator, json_schema}; +use semver::Version; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; use serde_json::Value; use settings::WorktreeId; @@ -347,7 +348,7 @@ pub trait LspAdapterDelegate: Send + Sync { async fn npm_package_installed_version( &self, package_name: &str, - ) -> Result>; + ) -> Result>; async fn which(&self, command: &OsStr) -> Option; async fn shell_env(&self) -> HashMap; async fn read_text_file(&self, path: &RelPath) -> Result; diff --git a/crates/languages/Cargo.toml b/crates/languages/Cargo.toml index c0aa9c39aac..8529bdb82ac 100644 --- a/crates/languages/Cargo.toml +++ b/crates/languages/Cargo.toml @@ -68,6 +68,7 @@ serde_json.workspace = true serde_json_lenient.workspace = true settings.workspace = true smallvec.workspace = true +semver.workspace = true smol.workspace = true snippet.workspace = true task.workspace = true diff --git a/crates/languages/src/css.rs b/crates/languages/src/css.rs index 6a925586a62..ca6bbd827e1 100644 --- a/crates/languages/src/css.rs +++ b/crates/languages/src/css.rs @@ -5,6 +5,7 @@ use language::{LspAdapter, LspAdapterDelegate, LspInstaller, Toolchain}; use lsp::{LanguageServerBinary, LanguageServerName, Uri}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; +use semver::Version; use serde_json::json; use std::{ ffi::OsString, @@ -32,14 +33,14 @@ impl CssLspAdapter { } impl LspInstaller for CssLspAdapter { - type BinaryVersion = String; + type BinaryVersion = Version; async fn fetch_latest_server_version( &self, _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { self.node .npm_package_latest_version("vscode-langservers-extracted") .await @@ -65,11 +66,12 @@ impl LspInstaller for CssLspAdapter { async fn fetch_server_binary( &self, - latest_version: String, + latest_version: Self::BinaryVersion, container_dir: PathBuf, _: &dyn LspAdapterDelegate, ) -> Result { let server_path = container_dir.join(SERVER_PATH); + let latest_version = latest_version.to_string(); self.node .npm_install_packages( @@ -87,7 +89,7 @@ impl LspInstaller for CssLspAdapter { async fn check_if_version_installed( &self, - version: &String, + version: &Self::BinaryVersion, container_dir: &PathBuf, _: &dyn LspAdapterDelegate, ) -> Option { diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs index 5168ba6e618..5e0f4907ef0 100644 --- a/crates/languages/src/json.rs +++ b/crates/languages/src/json.rs @@ -13,6 +13,7 @@ use language::{ use lsp::{LanguageServerBinary, LanguageServerName, Uri}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; +use semver::Version; use serde_json::{Value, json}; use smol::{ fs::{self}, @@ -142,14 +143,14 @@ impl JsonLspAdapter { } impl LspInstaller for JsonLspAdapter { - type BinaryVersion = String; + type BinaryVersion = Version; async fn fetch_latest_server_version( &self, _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { self.node .npm_package_latest_version(Self::PACKAGE_NAME) .await @@ -175,7 +176,7 @@ impl LspInstaller for JsonLspAdapter { async fn check_if_version_installed( &self, - version: &String, + version: &Self::BinaryVersion, container_dir: &PathBuf, _: &dyn LspAdapterDelegate, ) -> Option { @@ -204,11 +205,12 @@ impl LspInstaller for JsonLspAdapter { async fn fetch_server_binary( &self, - latest_version: String, + latest_version: Self::BinaryVersion, container_dir: PathBuf, _: &dyn LspAdapterDelegate, ) -> Result { let server_path = container_dir.join(SERVER_PATH); + let latest_version = latest_version.to_string(); self.node .npm_install_packages( diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs index b987d059cca..77d4be6f49a 100644 --- a/crates/languages/src/python.rs +++ b/crates/languages/src/python.rs @@ -19,6 +19,7 @@ use pet_core::python_environment::{PythonEnvironment, PythonEnvironmentKind}; use pet_virtualenv::is_virtualenv_dir; use project::Fs; use project::lsp_store::language_server_settings; +use semver::Version; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use settings::Settings; @@ -621,14 +622,14 @@ impl LspAdapter for PyrightLspAdapter { } impl LspInstaller for PyrightLspAdapter { - type BinaryVersion = String; + type BinaryVersion = Version; async fn fetch_latest_server_version( &self, _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { self.node .npm_package_latest_version(Self::SERVER_NAME.as_ref()) .await @@ -672,6 +673,7 @@ impl LspInstaller for PyrightLspAdapter { delegate: &dyn LspAdapterDelegate, ) -> Result { let server_path = container_dir.join(Self::SERVER_PATH); + let latest_version = latest_version.to_string(); self.node .npm_install_packages( @@ -2040,14 +2042,14 @@ impl LspAdapter for BasedPyrightLspAdapter { } impl LspInstaller for BasedPyrightLspAdapter { - type BinaryVersion = String; + type BinaryVersion = Version; async fn fetch_latest_server_version( &self, _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { self.node .npm_package_latest_version(Self::SERVER_NAME.as_ref()) .await @@ -2092,6 +2094,7 @@ impl LspInstaller for BasedPyrightLspAdapter { delegate: &dyn LspAdapterDelegate, ) -> Result { let server_path = container_dir.join(Self::SERVER_PATH); + let latest_version = latest_version.to_string(); self.node .npm_install_packages( diff --git a/crates/languages/src/tailwind.rs b/crates/languages/src/tailwind.rs index 7e23c4ba525..b4b6f76cec2 100644 --- a/crates/languages/src/tailwind.rs +++ b/crates/languages/src/tailwind.rs @@ -6,6 +6,7 @@ use language::{LanguageName, LspAdapter, LspAdapterDelegate, LspInstaller, Toolc use lsp::{LanguageServerBinary, LanguageServerName, Uri}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; +use semver::Version; use serde_json::{Value, json}; use std::{ ffi::OsString, @@ -39,14 +40,14 @@ impl TailwindLspAdapter { } impl LspInstaller for TailwindLspAdapter { - type BinaryVersion = String; + type BinaryVersion = Version; async fn fetch_latest_server_version( &self, _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { self.node .npm_package_latest_version(Self::PACKAGE_NAME) .await @@ -70,11 +71,12 @@ impl LspInstaller for TailwindLspAdapter { async fn fetch_server_binary( &self, - latest_version: String, + latest_version: Self::BinaryVersion, container_dir: PathBuf, _: &dyn LspAdapterDelegate, ) -> Result { let server_path = container_dir.join(SERVER_PATH); + let latest_version = latest_version.to_string(); self.node .npm_install_packages( @@ -92,7 +94,7 @@ impl LspInstaller for TailwindLspAdapter { async fn check_if_version_installed( &self, - version: &String, + version: &Self::BinaryVersion, container_dir: &PathBuf, _: &dyn LspAdapterDelegate, ) -> Option { diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index 7daf178d372..4f9476d5afa 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -12,6 +12,7 @@ use language::{ use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerName, Uri}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; +use semver::Version; use serde_json::{Value, json}; use smol::lock::RwLock; use std::{ @@ -635,8 +636,8 @@ impl TypeScriptLspAdapter { } pub struct TypeScriptVersions { - typescript_version: String, - server_version: String, + typescript_version: Version, + server_version: Version, } impl LspInstaller for TypeScriptLspAdapter { @@ -647,7 +648,7 @@ impl LspInstaller for TypeScriptLspAdapter { _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { Ok(TypeScriptVersions { typescript_version: self .node @@ -662,7 +663,7 @@ impl LspInstaller for TypeScriptLspAdapter { async fn check_if_version_installed( &self, - version: &TypeScriptVersions, + version: &Self::BinaryVersion, container_dir: &PathBuf, _: &dyn LspAdapterDelegate, ) -> Option { @@ -674,7 +675,7 @@ impl LspInstaller for TypeScriptLspAdapter { Self::PACKAGE_NAME, &server_path, container_dir, - VersionStrategy::Latest(version.typescript_version.as_str()), + VersionStrategy::Latest(&version.typescript_version), ) .await { @@ -687,7 +688,7 @@ impl LspInstaller for TypeScriptLspAdapter { Self::SERVER_PACKAGE_NAME, &server_path, container_dir, - VersionStrategy::Latest(version.server_version.as_str()), + VersionStrategy::Latest(&version.server_version), ) .await { @@ -703,7 +704,7 @@ impl LspInstaller for TypeScriptLspAdapter { async fn fetch_server_binary( &self, - latest_version: TypeScriptVersions, + latest_version: Self::BinaryVersion, container_dir: PathBuf, _: &dyn LspAdapterDelegate, ) -> Result { @@ -715,11 +716,11 @@ impl LspInstaller for TypeScriptLspAdapter { &[ ( Self::PACKAGE_NAME, - latest_version.typescript_version.as_str(), + &latest_version.typescript_version.to_string(), ), ( Self::SERVER_PACKAGE_NAME, - latest_version.server_version.as_str(), + &latest_version.server_version.to_string(), ), ], ) diff --git a/crates/languages/src/vtsls.rs b/crates/languages/src/vtsls.rs index b21ae1a4de2..3d38e022afe 100644 --- a/crates/languages/src/vtsls.rs +++ b/crates/languages/src/vtsls.rs @@ -7,6 +7,7 @@ use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerName, Uri}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::{Fs, lsp_store::language_server_settings}; use regex::Regex; +use semver::Version; use serde_json::Value; use std::{ ffi::OsString, @@ -74,8 +75,8 @@ impl VtslsLspAdapter { } pub struct TypeScriptVersions { - typescript_version: String, - server_version: String, + typescript_version: Version, + server_version: Version, } const SERVER_NAME: LanguageServerName = LanguageServerName::new_static("vtsls"); @@ -88,7 +89,7 @@ impl LspInstaller for VtslsLspAdapter { _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { Ok(TypeScriptVersions { typescript_version: self.node.npm_package_latest_version("typescript").await?, server_version: self @@ -115,12 +116,15 @@ impl LspInstaller for VtslsLspAdapter { async fn fetch_server_binary( &self, - latest_version: TypeScriptVersions, + latest_version: Self::BinaryVersion, container_dir: PathBuf, _: &dyn LspAdapterDelegate, ) -> Result { let server_path = container_dir.join(Self::SERVER_PATH); + let typescript_version = latest_version.typescript_version.to_string(); + let server_version = latest_version.server_version.to_string(); + let mut packages_to_install = Vec::new(); if self @@ -133,7 +137,7 @@ impl LspInstaller for VtslsLspAdapter { ) .await { - packages_to_install.push((Self::PACKAGE_NAME, latest_version.server_version.as_str())); + packages_to_install.push((Self::PACKAGE_NAME, server_version.as_str())); } if self @@ -146,10 +150,7 @@ impl LspInstaller for VtslsLspAdapter { ) .await { - packages_to_install.push(( - Self::TYPESCRIPT_PACKAGE_NAME, - latest_version.typescript_version.as_str(), - )); + packages_to_install.push((Self::TYPESCRIPT_PACKAGE_NAME, typescript_version.as_str())); } self.node diff --git a/crates/languages/src/yaml.rs b/crates/languages/src/yaml.rs index 57f254a68f1..6c1d8bc2d9e 100644 --- a/crates/languages/src/yaml.rs +++ b/crates/languages/src/yaml.rs @@ -7,6 +7,7 @@ use language::{ use lsp::{LanguageServerBinary, LanguageServerName, Uri}; use node_runtime::{NodeRuntime, VersionStrategy}; use project::lsp_store::language_server_settings; +use semver::Version; use serde_json::Value; use settings::{Settings, SettingsLocation}; use std::{ @@ -35,14 +36,14 @@ impl YamlLspAdapter { } impl LspInstaller for YamlLspAdapter { - type BinaryVersion = String; + type BinaryVersion = Version; async fn fetch_latest_server_version( &self, _: &dyn LspAdapterDelegate, _: bool, _: &mut AsyncApp, - ) -> Result { + ) -> Result { self.node .npm_package_latest_version("yaml-language-server") .await @@ -66,7 +67,7 @@ impl LspInstaller for YamlLspAdapter { async fn fetch_server_binary( &self, - latest_version: String, + latest_version: Self::BinaryVersion, container_dir: PathBuf, _: &dyn LspAdapterDelegate, ) -> Result { @@ -75,7 +76,7 @@ impl LspInstaller for YamlLspAdapter { self.node .npm_install_packages( &container_dir, - &[(Self::PACKAGE_NAME, latest_version.as_str())], + &[(Self::PACKAGE_NAME, &latest_version.to_string())], ) .await?; @@ -88,7 +89,7 @@ impl LspInstaller for YamlLspAdapter { async fn check_if_version_installed( &self, - version: &String, + version: &Self::BinaryVersion, container_dir: &PathBuf, _: &dyn LspAdapterDelegate, ) -> Option { diff --git a/crates/node_runtime/src/node_runtime.rs b/crates/node_runtime/src/node_runtime.rs index 322117cd717..5e4297988b6 100644 --- a/crates/node_runtime/src/node_runtime.rs +++ b/crates/node_runtime/src/node_runtime.rs @@ -34,9 +34,9 @@ pub struct NodeBinaryOptions { pub enum VersionStrategy<'a> { /// Install if current version doesn't match pinned version - Pin(&'a str), + Pin(&'a Version), /// Install if current version is older than latest version - Latest(&'a str), + Latest(&'a Version), } #[derive(Clone)] @@ -230,14 +230,14 @@ impl NodeRuntime { &self, local_package_directory: &Path, name: &str, - ) -> Result> { + ) -> Result> { self.instance() .await .npm_package_installed_version(local_package_directory, name) .await } - pub async fn npm_package_latest_version(&self, name: &str) -> Result { + pub async fn npm_package_latest_version(&self, name: &str) -> Result { let http = self.0.lock().await.http.clone(); let output = self .instance() @@ -280,16 +280,19 @@ impl NodeRuntime { .map(|(name, version)| format!("{name}@{version}")) .collect(); - let mut arguments: Vec<_> = packages.iter().map(|p| p.as_str()).collect(); - arguments.extend_from_slice(&[ - "--save-exact", - "--fetch-retry-mintimeout", - "2000", - "--fetch-retry-maxtimeout", - "5000", - "--fetch-timeout", - "5000", - ]); + let arguments: Vec<_> = packages + .iter() + .map(|p| p.as_str()) + .chain([ + "--save-exact", + "--fetch-retry-mintimeout", + "2000", + "--fetch-retry-maxtimeout", + "5000", + "--fetch-timeout", + "5000", + ]) + .collect(); // This is also wrong because the directory is wrong. self.run_npm_subcommand(Some(directory), "install", &arguments) @@ -320,23 +323,9 @@ impl NodeRuntime { return true; }; - let Some(installed_version) = Version::parse(&installed_version).log_err() else { - return true; - }; - match version_strategy { - VersionStrategy::Pin(pinned_version) => { - let Some(pinned_version) = Version::parse(pinned_version).log_err() else { - return true; - }; - installed_version != pinned_version - } - VersionStrategy::Latest(latest_version) => { - let Some(latest_version) = Version::parse(latest_version).log_err() else { - return true; - }; - installed_version < latest_version - } + VersionStrategy::Pin(pinned_version) => &installed_version != pinned_version, + VersionStrategy::Latest(latest_version) => &installed_version < latest_version, } } } @@ -351,12 +340,12 @@ enum ArchiveType { pub struct NpmInfo { #[serde(default)] dist_tags: NpmInfoDistTags, - versions: Vec, + versions: Vec, } #[derive(Debug, Deserialize, Default)] pub struct NpmInfoDistTags { - latest: Option, + latest: Option, } #[async_trait::async_trait] @@ -376,7 +365,7 @@ trait NodeRuntimeTrait: Send + Sync { &self, local_package_directory: &Path, name: &str, - ) -> Result>; + ) -> Result>; } #[derive(Clone)] @@ -610,7 +599,7 @@ impl NodeRuntimeTrait for ManagedNodeRuntime { &self, local_package_directory: &Path, name: &str, - ) -> Result> { + ) -> Result> { read_package_installed_version(local_package_directory.join("node_modules"), name).await } } @@ -735,7 +724,7 @@ impl NodeRuntimeTrait for SystemNodeRuntime { &self, local_package_directory: &Path, name: &str, - ) -> Result> { + ) -> Result> { read_package_installed_version(local_package_directory.join("node_modules"), name).await // todo: allow returning a globally installed version (requires callers not to hard-code the path) } @@ -744,7 +733,7 @@ impl NodeRuntimeTrait for SystemNodeRuntime { pub async fn read_package_installed_version( node_module_directory: PathBuf, name: &str, -) -> Result> { +) -> Result> { let package_json_path = node_module_directory.join(name).join("package.json"); let mut file = match fs::File::open(package_json_path).await { @@ -760,7 +749,7 @@ pub async fn read_package_installed_version( #[derive(Deserialize)] struct PackageJson { - version: String, + version: Version, } let mut contents = String::new(); @@ -797,7 +786,7 @@ impl NodeRuntimeTrait for UnavailableNodeRuntime { &self, _local_package_directory: &Path, _: &str, - ) -> Result> { + ) -> Result> { bail!("{}", self.error_message) } } diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index a2cc57beae9..287b2593567 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -22,6 +22,7 @@ use rpc::{ proto::{self, ExternalExtensionAgent}, }; use schemars::JsonSchema; +use semver::Version; use serde::{Deserialize, Serialize}; use settings::{RegisterSetting, SettingsStore}; use task::{Shell, SpawnInTerminal}; @@ -974,11 +975,10 @@ fn get_or_npm_install_builtin_agent( } versions.sort(); - let newest_version = if let Some((version, file_name)) = versions.last().cloned() + let newest_version = if let Some((version, _)) = versions.last().cloned() && minimum_version.is_none_or(|minimum_version| version >= minimum_version) { - versions.pop(); - Some(file_name) + versions.pop() } else { None }; @@ -1004,9 +1004,8 @@ fn get_or_npm_install_builtin_agent( }) .detach(); - let version = if let Some(file_name) = newest_version { + let version = if let Some((version, file_name)) = newest_version { cx.background_spawn({ - let file_name = file_name.clone(); let dir = dir.clone(); let fs = fs.clone(); async move { @@ -1015,7 +1014,7 @@ fn get_or_npm_install_builtin_agent( .await .ok(); if let Some(latest_version) = latest_version - && &latest_version != &file_name.to_string_lossy() + && latest_version != version { let download_result = download_latest_version( fs, @@ -1028,7 +1027,9 @@ fn get_or_npm_install_builtin_agent( if let Some(mut new_version_available) = new_version_available && download_result.is_some() { - new_version_available.send(Some(latest_version)).ok(); + new_version_available + .send(Some(latest_version.to_string())) + .ok(); } } } @@ -1047,6 +1048,7 @@ fn get_or_npm_install_builtin_agent( package_name.clone(), )) .await? + .to_string() .into() }; @@ -1093,7 +1095,7 @@ async fn download_latest_version( dir: PathBuf, node_runtime: NodeRuntime, package_name: SharedString, -) -> Result { +) -> Result { log::debug!("downloading latest version of {package_name}"); let tmp_dir = tempfile::tempdir_in(&dir)?; @@ -1109,7 +1111,7 @@ async fn download_latest_version( fs.rename( &tmp_dir.keep(), - &dir.join(&version), + &dir.join(version.to_string()), RenameOptions { ignore_if_exists: true, overwrite: true, diff --git a/crates/project/src/debugger/session.rs b/crates/project/src/debugger/session.rs index 82a139ea242..1bc41df4bd8 100644 --- a/crates/project/src/debugger/session.rs +++ b/crates/project/src/debugger/session.rs @@ -3118,10 +3118,11 @@ async fn get_or_install_companion(node: NodeRuntime, cx: &mut AsyncApp) -> Resul .await .context("getting installed companion version")? .context("companion was not installed")?; - smol::fs::rename(temp_dir.path(), dir.join(&version)) + let version_folder = dir.join(version.to_string()); + smol::fs::rename(temp_dir.path(), &version_folder) .await .context("moving companion package into place")?; - Ok(dir.join(version)) + Ok(version_folder) } let dir = paths::debug_adapters_dir().join("js-debug-companion"); @@ -3134,19 +3135,23 @@ async fn get_or_install_companion(node: NodeRuntime, cx: &mut AsyncApp) -> Resul .await .context("creating companion installation directory")?; - let mut children = smol::fs::read_dir(&dir) + let children = smol::fs::read_dir(&dir) .await .context("reading companion installation directory")? .try_collect::>() .await .context("reading companion installation directory entries")?; - children - .sort_by_key(|child| semver::Version::parse(child.file_name().to_str()?).ok()); - let latest_installed_version = children.last().and_then(|child| { - let version = child.file_name().into_string().ok()?; - Some((child.path(), version)) - }); + let latest_installed_version = children + .iter() + .filter_map(|child| { + Some(( + child.path(), + semver::Version::parse(child.file_name().to_str()?).ok()?, + )) + }) + .max_by_key(|(_, version)| version.clone()); + let latest_version = node .npm_package_latest_version(PACKAGE_NAME) .await diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 83c2abfbb86..9b3eeebed79 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -93,6 +93,7 @@ use rpc::{ AnyProtoClient, ErrorCode, ErrorExt as _, proto::{LspRequestId, LspRequestMessage as _}, }; +use semver::Version; use serde::Serialize; use serde_json::Value; use settings::{Settings, SettingsLocation, SettingsStore}; @@ -13981,7 +13982,7 @@ impl LspAdapterDelegate for LocalLspAdapterDelegate { async fn npm_package_installed_version( &self, package_name: &str, - ) -> Result> { + ) -> Result> { let local_package_directory = self.worktree_root_path(); let node_modules_directory = local_package_directory.join("node_modules"); diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index 40deac76404..a8b6fe37701 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -905,7 +905,7 @@ async fn install_prettier_packages( .with_context(|| { format!("fetching latest npm version for package {returned_package_name}") })?; - anyhow::Ok((returned_package_name, latest_version)) + anyhow::Ok((returned_package_name, latest_version.to_string())) }), ) .await