refactor: NpmRegistryApi - #[async_trait(?Send)] (#24126)

Cargo.lock

@@ -1734,9 +1734,9 @@ dependencies = [
 
 [[package]]
 name = "deno_npm"
-version = "0.21.2"
+version = "0.21.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5c7ae566a3cba78bf05751c20708f28385fe339b1d07bd8daff16316317d4228"
+checksum = "a78b95f0daab64d3cfb28b13be2b40d73c8b9563bbce3aa50fc322a7325ce0b9"
 dependencies = [
  "anyhow",
  "async-trait",

cli/Cargo.toml

@@ -72,7 +72,7 @@ deno_emit = "=0.42.0"
 deno_graph = { version = "=0.78.0", features = ["tokio_executor"] }
 deno_lint = { version = "=0.60.0", features = ["docs"] }
 deno_lockfile.workspace = true
-deno_npm = "=0.21.2"
+deno_npm = "=0.21.3"
 deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
 deno_semver = "=0.5.4"
 deno_task_shell = "=0.16.1"

cli/npm/managed/mod.rs

@@ -83,26 +83,31 @@ pub async fn create_managed_npm_resolver_for_lsp(
 ) -> Arc<dyn CliNpmResolver> {
   let npm_cache = create_cache(&options);
   let npm_api = create_api(&options, npm_cache.clone());
-  let snapshot = match resolve_snapshot(&npm_api, options.snapshot).await {
+  // spawn due to the lsp's `Send` requirement
-    Ok(snapshot) => snapshot,
+  deno_core::unsync::spawn(async move {
-    Err(err) => {
+    let snapshot = match resolve_snapshot(&npm_api, options.snapshot).await {
-      log::warn!("failed to resolve snapshot: {}", err);
+      Ok(snapshot) => snapshot,
-      None
+      Err(err) => {
-    }
+        log::warn!("failed to resolve snapshot: {}", err);
-  };
+        None
-  create_inner(
+      }
-    options.fs,
+    };
-    options.http_client_provider,
+    create_inner(
-    options.maybe_lockfile,
+      options.fs,
-    npm_api,
+      options.http_client_provider,
-    npm_cache,
+      options.maybe_lockfile,
-    options.npmrc,
+      npm_api,
-    options.package_json_installer,
+      npm_cache,
-    options.text_only_progress_bar,
+      options.npmrc,
-    options.maybe_node_modules_path,
+      options.package_json_installer,
-    options.npm_system_info,
+      options.text_only_progress_bar,
-    snapshot,
+      options.maybe_node_modules_path,
-  )
+      options.npm_system_info,
+      snapshot,
+    )
+  })
+  .await
+  .unwrap()
 }
 
 pub async fn create_managed_npm_resolver(

cli/npm/managed/registry.rs

@@ -50,7 +50,7 @@ impl CliNpmRegistryApi {
   }
 }
 
-#[async_trait]
+#[async_trait(?Send)]
 impl NpmRegistryApi for CliNpmRegistryApi {
   async fn package_info(
     &self,

cli/npm/managed/resolvers/common.rs

@@ -24,7 +24,7 @@ use deno_runtime::deno_node::NodeResolutionMode;
 use crate::npm::managed::cache::TarballCache;
 
 /// Part of the resolution that interacts with the file system.
-#[async_trait]
+#[async_trait(?Send)]
 pub trait NpmPackageFsResolver: Send + Sync {
   /// Specifier for the root directory.
   fn root_dir_url(&self) -> &Url;

cli/npm/managed/resolvers/global.rs

@@ -70,7 +70,7 @@ impl GlobalNpmPackageResolver {
   }
 }
 
-#[async_trait]
+#[async_trait(?Send)]
 impl NpmPackageFsResolver for GlobalNpmPackageResolver {
   fn root_dir_url(&self) -> &Url {
     self.cache.root_dir_url()

cli/npm/managed/resolvers/local.rs

@@ -5,12 +5,14 @@
 mod bin_entries;
 
 use std::borrow::Cow;
+use std::cell::RefCell;
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::fs;
 use std::path::Path;
 use std::path::PathBuf;
+use std::rc::Rc;
 use std::sync::Arc;
 
 use crate::cache::CACHE_PERM;
@@ -29,7 +31,6 @@ use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::futures::stream::FuturesUnordered;
 use deno_core::futures::StreamExt;
-use deno_core::parking_lot::Mutex;
 use deno_core::url::Url;
 use deno_npm::resolution::NpmResolutionSnapshot;
 use deno_npm::NpmPackageCacheFolderId;
@@ -141,7 +142,7 @@ impl LocalNpmPackageResolver {
   }
 }
 
-#[async_trait]
+#[async_trait(?Send)]
 impl NpmPackageFsResolver for LocalNpmPackageResolver {
   fn root_dir_url(&self) -> &Url {
     &self.root_node_modules_url
@@ -296,7 +297,7 @@ async fn sync_resolution_with_fs(
   let mut cache_futures = FuturesUnordered::new();
   let mut newest_packages_by_name: HashMap<&String, &NpmResolutionPackage> =
     HashMap::with_capacity(package_partitions.packages.len());
-  let bin_entries = Arc::new(Mutex::new(bin_entries::BinEntries::new()));
+  let bin_entries = Rc::new(RefCell::new(bin_entries::BinEntries::new()));
   for package in &package_partitions.packages {
     if let Some(current_pkg) =
       newest_packages_by_name.get_mut(&package.id.nv.name)
@@ -349,7 +350,7 @@ async fn sync_resolution_with_fs(
 
         if package.bin.is_some() {
           bin_entries_to_setup
-            .lock()
+            .borrow_mut()
             .add(package.clone(), package_path);
         }
 
@@ -482,7 +483,7 @@ async fn sync_resolution_with_fs(
 
   // 6. Set up `node_modules/.bin` entries for packages that need it.
   {
-    let bin_entries = std::mem::take(&mut *bin_entries.lock());
+    let bin_entries = std::mem::take(&mut *bin_entries.borrow_mut());
     bin_entries.finish(snapshot, &bin_node_modules_dir_path)?;
   }