fix(npm): cache bust npm specifiers more aggressively

Cargo.toml

@@ -311,3 +311,6 @@ opt-level = 3
 opt-level = 3
 [profile.release.package.base64-simd]
 opt-level = 3
+
+[patch.crates-io]
+deno_graph = { path = "../deno_graph" }

cli/args/mod.rs

@@ -64,8 +64,8 @@ use std::sync::Arc;
 
 use crate::cache::DenoDir;
 use crate::file_fetcher::FileFetcher;
+use crate::npm::CliNpmRegistryApi;
 use crate::npm::NpmProcessState;
-use crate::npm::NpmRegistry;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
 use crate::version;
 
@@ -746,7 +746,7 @@ impl CliOptions {
 
  pub async fn resolve_npm_resolution_snapshot(
  &self,
- api: &NpmRegistry,
+ api: &CliNpmRegistryApi,
  ) -> Result<Option<NpmResolutionSnapshot>, AnyError> {
  if let Some(state) = &*NPM_PROCESS_STATE {
  // TODO(bartlomieju): remove this clone

cli/graph_util.rs

@@ -254,7 +254,7 @@ pub async fn build_graph_with_npm_resolution<'a>(
  loader: &mut dyn deno_graph::source::Loader,
  options: deno_graph::BuildOptions<'a>,
 ) -> Result<(), AnyError> {
- graph.build(roots, loader, options).await;
+ graph.build(roots.clone(), loader, options).await;
 
  // resolve the dependencies of any pending dependencies
  // that were inserted by building the graph

cli/lsp/documents.rs

@@ -20,8 +20,8 @@ use crate::lsp::logging::lsp_warn;
 use crate::node;
 use crate::node::node_resolve_npm_reference;
 use crate::node::NodeResolution;
+use crate::npm::CliNpmRegistryApi;
 use crate::npm::NpmPackageResolver;
-use crate::npm::NpmRegistry;
 use crate::npm::NpmResolution;
 use crate::npm::PackageJsonDepsInstaller;
 use crate::resolver::CliGraphResolver;
@@ -1166,7 +1166,7 @@ impl Documents {
  maybe_import_map: Option<Arc<import_map::ImportMap>>,
  maybe_config_file: Option<&ConfigFile>,
  maybe_package_json: Option<&PackageJson>,
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
  npm_resolution: NpmResolution,
  ) {
  fn calculate_resolver_config_hash(
@@ -1864,7 +1864,7 @@ console.log(b, "hello deno");
 
  #[test]
  fn test_documents_refresh_dependencies_config_change() {
- let npm_registry_api = NpmRegistry::new_uninitialized();
+ let npm_registry_api = CliNpmRegistryApi::new_uninitialized();
  let npm_resolution =
  NpmResolution::new(npm_registry_api.clone(), None, None);
 

cli/lsp/language_server.rs

@@ -79,9 +79,9 @@ use crate::graph_util;
 use crate::http_util::HttpClient;
 use crate::lsp::urls::LspUrlKind;
 use crate::npm::create_npm_fs_resolver;
+use crate::npm::CliNpmRegistryApi;
 use crate::npm::NpmCache;
 use crate::npm::NpmPackageResolver;
-use crate::npm::NpmRegistry;
 use crate::npm::NpmResolution;
 use crate::proc_state::ProcState;
 use crate::tools::fmt::format_file;
@@ -145,7 +145,7 @@ pub struct Inner {
  /// A lazily create "server" for handling test run requests.
  maybe_testing_server: Option<testing::TestServer>,
  /// Npm's registry api.
- npm_api: NpmRegistry,
+ npm_api: CliNpmRegistryApi,
  /// Npm cache
  npm_cache: NpmCache,
  /// Npm resolution that is stored in memory.
@@ -417,8 +417,13 @@ impl LanguageServer {
 fn create_lsp_structs(
  dir: &DenoDir,
  http_client: HttpClient,
-) -> (NpmRegistry, NpmCache, NpmPackageResolver, NpmResolution) {
- let registry_url = NpmRegistry::default_url();
+) -> (
+ CliNpmRegistryApi,
+ NpmCache,
+ NpmPackageResolver,
+ NpmResolution,
+) {
+ let registry_url = CliNpmRegistryApi::default_url();
  let progress_bar = ProgressBar::new(ProgressBarStyle::TextOnly);
  let npm_cache = NpmCache::from_deno_dir(
  dir,
@@ -430,7 +435,7 @@ fn create_lsp_structs(
  http_client.clone(),
  progress_bar.clone(),
  );
- let api = NpmRegistry::new(
+ let api = CliNpmRegistryApi::new(
  registry_url.clone(),
  npm_cache.clone(),
  http_client,

cli/npm/installer.rs

@@ -10,13 +10,13 @@ use deno_npm::registry::NpmRegistryApi;
 
 use crate::args::package_json::PackageJsonDeps;
 
-use super::NpmRegistry;
+use super::CliNpmRegistryApi;
 use super::NpmResolution;
 
 #[derive(Debug)]
 struct PackageJsonDepsInstallerInner {
  has_installed: AtomicBool,
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
  npm_resolution: NpmResolution,
  package_deps: PackageJsonDeps,
 }
@@ -27,7 +27,7 @@ pub struct PackageJsonDepsInstaller(Option<Arc<PackageJsonDepsInstallerInner>>);
 
 impl PackageJsonDepsInstaller {
  pub fn new(
- npm_registry_api: NpmRegistry,
+ npm_registry_api: CliNpmRegistryApi,
  npm_resolution: NpmResolution,
  deps: Option<PackageJsonDeps>,
  ) -> Self {

cli/npm/mod.rs

@@ -10,7 +10,7 @@ mod tarball;
 pub use cache::should_sync_download;
 pub use cache::NpmCache;
 pub use installer::PackageJsonDepsInstaller;
-pub use registry::NpmRegistry;
+pub use registry::CliNpmRegistryApi;
 pub use resolution::NpmResolution;
 pub use resolvers::create_npm_fs_resolver;
 pub use resolvers::NpmPackageResolver;

cli/npm/registry.rs

@@ -54,9 +54,9 @@ static NPM_REGISTRY_DEFAULT_URL: Lazy<Url> = Lazy::new(|| {
 });
 
 #[derive(Clone, Debug)]
-pub struct NpmRegistry(Option<Arc<NpmRegistryApiInner>>);
+pub struct CliNpmRegistryApi(Option<Arc<CliNpmRegistryApiInner>>);
 
-impl NpmRegistry {
+impl CliNpmRegistryApi {
  pub fn default_url() -> &'static Url {
  &NPM_REGISTRY_DEFAULT_URL
  }
@@ -67,7 +67,7 @@ impl NpmRegistry {
  http_client: HttpClient,
  progress_bar: ProgressBar,
  ) -> Self {
- Self(Some(Arc::new(NpmRegistryApiInner {
+ Self(Some(Arc::new(CliNpmRegistryApiInner {
  base_url,
  cache,
  force_reload: AtomicBool::new(false),
@@ -105,14 +105,18 @@ impl NpmRegistry {
  ///
  /// Returns true if it was successfully set for the first time.
  pub fn mark_force_reload(&self) -> bool {
- // never force reload the registry information if reloading is disabled
- if matches!(self.inner().cache.cache_setting(), CacheSetting::Only) {
+ // never force reload the registry information if reloading
+ // is disabled or if we're already reloading
+ if matches!(
+ self.inner().cache.cache_setting(),
+ CacheSetting::Only | CacheSetting::ReloadAll
+ ) {
  return false;
  }
  !self.inner().force_reload.swap(true, Ordering::SeqCst)
  }
 
- fn inner(&self) -> &Arc<NpmRegistryApiInner> {
+ fn inner(&self) -> &Arc<CliNpmRegistryApiInner> {
  // this panicking indicates a bug in the code where this
  // wasn't initialized
  self.0.as_ref().unwrap()
@@ -123,7 +127,7 @@ static SYNC_DOWNLOAD_TASK_QUEUE: Lazy<TaskQueue> =
  Lazy::new(TaskQueue::default);
 
 #[async_trait]
-impl NpmRegistryApi for NpmRegistry {
+impl NpmRegistryApi for CliNpmRegistryApi {
  async fn package_info(
  &self,
  name: &str,
@@ -148,16 +152,17 @@ impl NpmRegistryApi for NpmRegistry {
  }
 }
 
+type CacheItemPendingResult =
+ Result<Option<Arc<NpmPackageInfo>>, Arc<AnyError>>;
+
 #[derive(Debug)]
 enum CacheItem {
- Pending(
- Shared<BoxFuture<'static, Result<Option<Arc<NpmPackageInfo>>, String>>>,
- ),
+ Pending(Shared<BoxFuture<'static, CacheItemPendingResult>>),
  Resolved(Option<Arc<NpmPackageInfo>>),
 }
 
 #[derive(Debug)]
-struct NpmRegistryApiInner {
+struct CliNpmRegistryApiInner {
  base_url: Url,
  cache: NpmCache,
  force_reload: AtomicBool,
@@ -167,7 +172,7 @@ struct NpmRegistryApiInner {
  progress_bar: ProgressBar,
 }
 
-impl NpmRegistryApiInner {
+impl CliNpmRegistryApiInner {
  pub async fn maybe_package_info(
  self: &Arc<Self>,
  name: &str,
@@ -197,9 +202,15 @@ impl NpmRegistryApiInner {
  let future = {
  let api = self.clone();
  let name = name.to_string();
- async move { api.load_package_info_from_registry(&name).await }
- .boxed()
- .shared()
+ async move {
+ api
+ .load_package_info_from_registry(&name)
+ .await
+ .map(|info| info.map(Arc::new))
+ .map_err(Arc::new)
+ }
+ .boxed()
+ .shared()
  };
  mem_cache
  .insert(name.to_string(), CacheItem::Pending(future.clone()));
@@ -304,7 +315,7 @@ impl NpmRegistryApiInner {
  async fn load_package_info_from_registry(
  &self,
  name: &str,
- ) -> Result<Option<Arc<NpmPackageInfo>>, String> {
+ ) -> Result<Option<NpmPackageInfo>, AnyError> {
  self
  .load_package_info_from_registry_inner(name)
  .await
@@ -315,9 +326,6 @@ impl NpmRegistryApiInner {
  name
  )
  })
- .map(|info| info.map(Arc::new))
- // make cloneable
- .map_err(|err| format!("{err:#}"))
  }
 
  async fn load_package_info_from_registry_inner(

cli/npm/resolution.rs

@@ -27,7 +27,7 @@ use deno_semver::npm::NpmPackageReqReference;
 
 use crate::args::Lockfile;
 
-use super::registry::NpmRegistry;
+use super::registry::CliNpmRegistryApi;
 
 /// Handles updating and storing npm resolution in memory where the underlying
 /// snapshot can be updated concurrently. Additionally handles updating the lockfile
@@ -38,7 +38,7 @@ use super::registry::NpmRegistry;
 pub struct NpmResolution(Arc<NpmResolutionInner>);
 
 struct NpmResolutionInner {
- api: NpmRegistry,
+ api: CliNpmRegistryApi,
  snapshot: RwLock<NpmResolutionSnapshot>,
  update_queue: TaskQueue,
  maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
@@ -55,7 +55,7 @@ impl std::fmt::Debug for NpmResolution {
 
 impl NpmResolution {
  pub fn new(
- api: NpmRegistry,
+ api: CliNpmRegistryApi,
  initial_snapshot: Option<NpmResolutionSnapshot>,
  maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
  ) -> Self {
@@ -247,7 +247,7 @@ impl NpmResolution {
 }
 
 async fn add_package_reqs_to_snapshot(
- api: &NpmRegistry,
+ api: &CliNpmRegistryApi,
  // todo(18079): it should be possible to pass &[NpmPackageReq] in here
  // and avoid all these clones, but the LSP complains because of its
  // `Send` requirement

cli/proc_state.rs

@@ -26,9 +26,9 @@ use crate::http_util::HttpClient;
 use crate::node;
 use crate::node::NodeResolution;
 use crate::npm::create_npm_fs_resolver;
+use crate::npm::CliNpmRegistryApi;
 use crate::npm::NpmCache;
 use crate::npm::NpmPackageResolver;
-use crate::npm::NpmRegistry;
 use crate::npm::NpmResolution;
 use crate::npm::PackageJsonDepsInstaller;
 use crate::resolver::CliGraphResolver;
@@ -95,7 +95,7 @@ pub struct Inner {
  pub resolver: Arc<CliGraphResolver>,
  maybe_file_watcher_reporter: Option<FileWatcherReporter>,
  pub node_analysis_cache: NodeAnalysisCache,
- pub npm_api: NpmRegistry,
+ pub npm_api: CliNpmRegistryApi,
  pub npm_cache: NpmCache,
  pub npm_resolver: NpmPackageResolver,
  pub npm_resolution: NpmResolution,
@@ -233,14 +233,14 @@ impl ProcState {
 
  let lockfile = cli_options.maybe_lock_file();
 
- let npm_registry_url = NpmRegistry::default_url().to_owned();
+ let npm_registry_url = CliNpmRegistryApi::default_url().to_owned();
  let npm_cache = NpmCache::from_deno_dir(
  &dir,
  cli_options.cache_setting(),
  http_client.clone(),
  progress_bar.clone(),
  );
- let npm_api = NpmRegistry::new(
+ let npm_api = CliNpmRegistryApi::new(
  npm_registry_url.clone(),
  npm_cache.clone(),
  http_client.clone(),