From dbb5373eab5a416d2178d26e7caa3d92c80b6f3e Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Fri, 18 Apr 2025 16:08:15 -0700 Subject: [PATCH] fix(lockfile): re-fetch packuments if version not found, properly pass patch packages (#28964) Fixes two issues: - If a cached packument was out of date and missing a version from the lockfile, we would fail. Instead we should try again with a forced re-fetch - We weren't threading through the workspace patch packages correctly --- cli/args/lockfile.rs | 2 +- cli/factory.rs | 15 +- cli/lsp/config.rs | 75 +++++---- cli/lsp/diagnostics.rs | 20 +-- cli/lsp/documents.rs | 19 +-- cli/lsp/language_server.rs | 12 +- cli/lsp/mod.rs | 5 +- cli/lsp/repl.rs | 5 +- cli/lsp/resolver.rs | 3 +- cli/lsp/tsc.rs | 19 +-- cli/main.rs | 2 +- cli/npm/installer/common/mod.rs | 8 +- cli/npm/installer/local.rs | 7 + cli/npm/installer/mod.rs | 3 + cli/npm/mod.rs | 150 +++++++++++------- cli/tools/jupyter/mod.rs | 2 +- cli/tools/repl/mod.rs | 2 +- cli/tools/repl/session.rs | 5 +- .../out_of_date_npm_info/__test__.jsonc | 37 +++++ .../lockfile/out_of_date_npm_info/deno.json | 5 + .../out_of_date_npm_info/deno.lock.old | 16 ++ .../out_of_date_npm_info/deno.lock.out | 17 ++ .../specs/lockfile/out_of_date_npm_info/mv.ts | 3 + .../out_of_date_npm_info/rm-version.ts | 14 ++ 24 files changed, 311 insertions(+), 135 deletions(-) create mode 100644 tests/specs/lockfile/out_of_date_npm_info/__test__.jsonc create mode 100644 tests/specs/lockfile/out_of_date_npm_info/deno.json create mode 100644 tests/specs/lockfile/out_of_date_npm_info/deno.lock.old create mode 100644 tests/specs/lockfile/out_of_date_npm_info/deno.lock.out create mode 100644 tests/specs/lockfile/out_of_date_npm_info/mv.ts create mode 100644 tests/specs/lockfile/out_of_date_npm_info/rm-version.ts diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs index d8337457f1..f4adffbbad 100644 --- a/cli/args/lockfile.rs +++ b/cli/args/lockfile.rs @@ -293,7 +293,7 @@ impl CliLockfile { pub async fn read_from_path( sys: &CliSys, opts: CliLockfileReadFromPathOptions, - api: &(dyn NpmPackageInfoProvider + Send + Sync), + api: &(dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync), ) -> Result { let lockfile = match std::fs::read_to_string(&opts.file_path) { Ok(text) => { diff --git a/cli/factory.rs b/cli/factory.rs index 02b4e84ca5..97d977d26f 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -404,10 +404,7 @@ impl CliFactory { let workspace_directory = workspace_factory.workspace_directory()?; let maybe_external_import_map = self.workspace_external_import_map_loader()?.get_or_load()?; - let provider = self.npm_registry_info_provider()?.clone(); - let adapter = crate::npm::NpmPackageInfoApiAdapter(Arc::new( - provider.as_npm_registry_api(), - )); + let adapter = self.lockfile_npm_package_info_provider()?; let maybe_lock_file = CliLockfile::discover( &self.sys(), @@ -660,6 +657,7 @@ impl CliFactory { .map(|p| p.to_path_buf()), cli_options.lifecycle_scripts_config(), cli_options.npm_system_info(), + self.workspace_npm_patch_packages()?.clone(), ))) }) .await @@ -680,6 +678,15 @@ impl CliFactory { }) } + pub fn lockfile_npm_package_info_provider( + &self, + ) -> Result { + Ok(crate::npm::NpmPackageInfoApiAdapter::new( + Arc::new(self.npm_registry_info_provider()?.as_npm_registry_api()), + self.workspace_npm_patch_packages()?.clone(), + )) + } + pub async fn npm_resolution_initializer( &self, ) -> Result<&Arc, AnyError> { diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index c50b347b04..8a7d666511 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -42,7 +42,6 @@ use deno_lib::args::has_flag_env_var; use deno_lib::util::hash::FastInsecureHasher; use deno_lint::linter::LintConfig as DenoLintConfig; use deno_npm::npm_rc::ResolvedNpmRc; -use deno_npm::registry::NpmRegistryApi; use deno_package_json::PackageJsonCache; use deno_path_util::url_to_file_path; use deno_resolver::npmrc::discover_npmrc_from_workspace; @@ -1270,7 +1269,9 @@ impl ConfigData { deno_json_cache: &(dyn DenoJsonCache + Sync), pkg_json_cache: &(dyn PackageJsonCache + Sync), workspace_cache: &(dyn WorkspaceCache + Sync), - npm_package_info_provider: &Arc, + lockfile_package_info_provider: &Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Self { let scope = scope.clone(); let discover_result = match scope.to_file_path() { @@ -1309,7 +1310,7 @@ impl ConfigData { scope, settings, Some(file_fetcher), - npm_package_info_provider, + lockfile_package_info_provider, ) .await } @@ -1325,7 +1326,7 @@ impl ConfigData { scope.clone(), settings, Some(file_fetcher), - npm_package_info_provider, + lockfile_package_info_provider, ) .await; // check if any of these need to be added to the workspace @@ -1368,7 +1369,9 @@ impl ConfigData { scope: Arc, settings: &Settings, file_fetcher: Option<&Arc>, - npm_package_info_provider: &Arc, + lockfile_package_info_provider: &Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Self { let (settings, workspace_folder) = settings.get_for_specifier(&scope); let mut watched_files = HashMap::with_capacity(10); @@ -1513,10 +1516,12 @@ impl ConfigData { let vendor_dir = member_dir.workspace.vendor_dir_path().cloned(); // todo(dsherret): add caching so we don't load this so many times - let lockfile = - resolve_lockfile_from_workspace(&member_dir, npm_package_info_provider) - .await - .map(Arc::new); + let lockfile = resolve_lockfile_from_workspace( + &member_dir, + lockfile_package_info_provider, + ) + .await + .map(Arc::new); if let Some(lockfile) = &lockfile { if let Ok(specifier) = ModuleSpecifier::from_file_path(&lockfile.filename) { @@ -1928,7 +1933,9 @@ impl ConfigTree { workspace_files: &IndexSet, file_fetcher: &Arc, deno_dir: &DenoDir, - npm_package_info_provider: &Arc, + lockfile_package_info_provider: &Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) { lsp_log!("Refreshing configuration tree..."); // since we're resolving a workspace multiple times in different @@ -1961,7 +1968,7 @@ impl ConfigTree { &deno_json_cache, &pkg_json_cache, &workspace_cache, - npm_package_info_provider, + lockfile_package_info_provider, ) .await, ), @@ -1995,7 +2002,7 @@ impl ConfigTree { &deno_json_cache, &pkg_json_cache, &workspace_cache, - npm_package_info_provider, + lockfile_package_info_provider, ) .await, ); @@ -2012,7 +2019,7 @@ impl ConfigTree { &deno_json_cache, &pkg_json_cache, &workspace_cache, - npm_package_info_provider, + lockfile_package_info_provider, ) .await; scopes.insert(member_scope.clone(), Arc::new(member_data)); @@ -2030,7 +2037,9 @@ impl ConfigTree { pub async fn inject_config_file( &mut self, config_file: ConfigFile, - npm_package_info_provider: &Arc, + lockfile_package_info_provider: &Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) { use sys_traits::FsCreateDirAll; use sys_traits::FsWrite; @@ -2061,7 +2070,7 @@ impl ConfigTree { scope.clone(), &Default::default(), None, - npm_package_info_provider, + lockfile_package_info_provider, ) .await, ); @@ -2072,7 +2081,9 @@ impl ConfigTree { async fn resolve_lockfile_from_workspace( workspace: &WorkspaceDirectory, - npm_package_info_provider: &Arc, + lockfile_package_info_provider: &Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Option { let lockfile_path = match workspace.workspace.resolve_lockfile_path() { Ok(Some(value)) => value, @@ -2087,8 +2098,12 @@ async fn resolve_lockfile_from_workspace( .root_deno_json() .and_then(|c| c.to_lock_config().ok().flatten().map(|c| c.frozen())) .unwrap_or(false); - resolve_lockfile_from_path(lockfile_path, frozen, npm_package_info_provider) - .await + resolve_lockfile_from_path( + lockfile_path, + frozen, + lockfile_package_info_provider, + ) + .await } fn resolve_node_modules_dir( @@ -2124,7 +2139,9 @@ fn resolve_node_modules_dir( async fn resolve_lockfile_from_path( lockfile_path: PathBuf, frozen: bool, - npm_package_info_provider: &Arc, + lockfile_package_info_provider: &Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Option { match CliLockfile::read_from_path( &CliSys::default(), @@ -2133,7 +2150,7 @@ async fn resolve_lockfile_from_path( frozen, skip_write: false, }, - &crate::npm::NpmPackageInfoApiAdapter(npm_package_info_provider.clone()), + &**lockfile_package_info_provider, ) .await { @@ -2198,8 +2215,6 @@ mod tests { use deno_core::resolve_url; use deno_core::serde_json; use deno_core::serde_json::json; - use deno_npm::registry::NpmPackageInfo; - use deno_npm::registry::NpmRegistryPackageInfoLoadError; use pretty_assertions::assert_eq; use super::*; @@ -2465,16 +2480,20 @@ mod tests { struct DefaultRegistry; #[async_trait::async_trait(?Send)] - impl deno_npm::registry::NpmRegistryApi for DefaultRegistry { - async fn package_info( + impl deno_lockfile::NpmPackageInfoProvider for DefaultRegistry { + async fn get_npm_package_info( &self, - _name: &str, - ) -> Result, NpmRegistryPackageInfoLoadError> { - Ok(Arc::new(NpmPackageInfo::default())) + values: &[deno_semver::package::PackageNv], + ) -> Result< + Vec, + Box, + > { + Ok(values.iter().map(|_| Default::default()).collect()) } } - fn default_registry() -> Arc { + fn default_registry( + ) -> Arc { Arc::new(DefaultRegistry) } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 20269119f9..8171cc9628 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1967,9 +1967,7 @@ mod tests { use deno_config::deno_json::ConfigFile; use deno_core::resolve_url; - use deno_npm::registry::NpmPackageInfo; - use deno_npm::registry::NpmRegistryApi; - use deno_npm::registry::NpmRegistryPackageInfoLoadError; + use deno_semver::package::PackageNv; use pretty_assertions::assert_eq; use test_util::TempDir; @@ -2011,16 +2009,20 @@ mod tests { struct DefaultRegistry; #[async_trait::async_trait(?Send)] - impl NpmRegistryApi for DefaultRegistry { - async fn package_info( + impl deno_lockfile::NpmPackageInfoProvider for DefaultRegistry { + async fn get_npm_package_info( &self, - _name: &str, - ) -> Result, NpmRegistryPackageInfoLoadError> { - Ok(Arc::new(NpmPackageInfo::default())) + values: &[PackageNv], + ) -> Result< + Vec, + Box, + > { + Ok(values.iter().map(|_| Default::default()).collect()) } } - fn default_registry() -> Arc { + fn default_registry( + ) -> Arc { Arc::new(DefaultRegistry) } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index a4f06c65d0..4aff5fc6d3 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -2054,9 +2054,6 @@ mod tests { use deno_config::deno_json::ConfigFile; use deno_core::serde_json; use deno_core::serde_json::json; - use deno_npm::registry::NpmPackageInfo; - use deno_npm::registry::NpmRegistryApi; - use deno_npm::registry::NpmRegistryPackageInfoLoadError; use pretty_assertions::assert_eq; use test_util::TempDir; @@ -2066,16 +2063,20 @@ mod tests { struct DefaultRegistry; #[async_trait::async_trait(?Send)] - impl deno_npm::registry::NpmRegistryApi for DefaultRegistry { - async fn package_info( + impl deno_lockfile::NpmPackageInfoProvider for DefaultRegistry { + async fn get_npm_package_info( &self, - _name: &str, - ) -> Result, NpmRegistryPackageInfoLoadError> { - Ok(Arc::new(NpmPackageInfo::default())) + values: &[deno_semver::package::PackageNv], + ) -> Result< + Vec, + Box, + > { + Ok(values.iter().map(|_| Default::default()).collect()) } } - fn default_registry() -> Arc { + fn default_registry( + ) -> Arc { Arc::new(DefaultRegistry) } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index a05e2dd4dd..9e6411a533 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -29,7 +29,6 @@ use deno_graph::Resolution; use deno_lib::args::get_root_cert_store; use deno_lib::args::CaData; use deno_lib::version::DENO_VERSION_INFO; -use deno_npm::registry::NpmRegistryApi; use deno_path_util::url_to_file_path; use deno_runtime::deno_tls::rustls::RootCertStore; use deno_runtime::deno_tls::RootCertStoreProvider; @@ -260,7 +259,8 @@ pub struct Inner { /// Set to `self.config.settings.enable_settings_hash()` after /// refreshing `self.workspace_files`. workspace_files_hash: u64, - registry_provider: Arc, + registry_provider: + Arc, _tracing: Option, } @@ -299,7 +299,9 @@ impl std::fmt::Debug for Inner { impl LanguageServer { pub fn new( client: Client, - registry_provider: Arc, + registry_provider: Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Self { let performance = Arc::new(Performance::default()); Self { @@ -533,7 +535,9 @@ impl Inner { fn new( client: Client, performance: Arc, - registry_provider: Arc, + registry_provider: Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Self { let cache = LspCache::default(); let http_client_provider = Arc::new(HttpClientProvider::new(None, None)); diff --git a/cli/lsp/mod.rs b/cli/lsp/mod.rs index 85c25be5d2..bcceea85c8 100644 --- a/cli/lsp/mod.rs +++ b/cli/lsp/mod.rs @@ -3,7 +3,6 @@ use std::sync::Arc; use deno_core::error::AnyError; -use deno_npm::registry::NpmRegistryApi; pub use repl::ReplCompletionItem; pub use repl::ReplLanguageServer; use tower_lsp::LspService; @@ -42,7 +41,9 @@ mod tsc; mod urls; pub async fn start( - registry_provider: Arc, + registry_provider: Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Result<(), AnyError> { let stdin = tokio::io::stdin(); let stdout = tokio::io::stdout(); diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs index 4c839d271b..ba96578cbe 100644 --- a/cli/lsp/repl.rs +++ b/cli/lsp/repl.rs @@ -9,7 +9,6 @@ use deno_ast::SourceTextInfo; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; use deno_core::serde_json; -use deno_npm::registry::NpmRegistryApi; use lsp_types::Uri; use tokio_util::sync::CancellationToken; use tower_lsp::lsp_types::ClientCapabilities; @@ -63,7 +62,9 @@ pub struct ReplLanguageServer { impl ReplLanguageServer { pub async fn new_initialized( - registry_provider: Arc, + registry_provider: Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Result { // downgrade info and warn lsp logging to debug super::logging::set_lsp_log_level(log::Level::Debug); diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 2081f4672c..2086bbf7c1 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -811,7 +811,7 @@ impl<'a> ResolverFactory<'a> { registry_info_provider.clone(), self.services.npm_resolution.clone(), maybe_lockfile.clone(), - patch_packages, + patch_packages.clone(), )); let npm_installer = Arc::new(NpmInstaller::new( npm_cache.clone(), @@ -827,6 +827,7 @@ impl<'a> ResolverFactory<'a> { maybe_node_modules_path.clone(), LifecycleScriptsConfig::default(), NpmSystemInfo::default(), + patch_packages, )); self.set_npm_installer(npm_installer); if let Err(err) = npm_resolution_initializer.ensure_initialized().await { diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 23ac43b43e..dbe8331b2f 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -5751,9 +5751,6 @@ impl TscRequest { #[cfg(test)] mod tests { - use deno_npm::registry::NpmPackageInfo; - use deno_npm::registry::NpmRegistryApi; - use deno_npm::registry::NpmRegistryPackageInfoLoadError; use pretty_assertions::assert_eq; use test_util::TempDir; @@ -5770,16 +5767,20 @@ mod tests { struct DefaultRegistry; #[async_trait::async_trait(?Send)] - impl deno_npm::registry::NpmRegistryApi for DefaultRegistry { - async fn package_info( + impl deno_lockfile::NpmPackageInfoProvider for DefaultRegistry { + async fn get_npm_package_info( &self, - _name: &str, - ) -> Result, NpmRegistryPackageInfoLoadError> { - Ok(Arc::new(NpmPackageInfo::default())) + values: &[deno_semver::package::PackageNv], + ) -> Result< + Vec, + Box, + > { + Ok(values.iter().map(|_| Default::default()).collect()) } } - fn default_registry() -> Arc { + fn default_registry( + ) -> Arc { Arc::new(DefaultRegistry) } diff --git a/cli/main.rs b/cli/main.rs index 279158a194..c3b30d18da 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -193,7 +193,7 @@ async fn run_subcommand(flags: Arc) -> Result { ", colors::cyan("deno lsp")); } let factory = CliFactory::from_flags(flags.clone()); - lsp::start(Arc::new(factory.npm_registry_info_provider()?.as_npm_registry_api())).await + lsp::start(Arc::new(factory.lockfile_npm_package_info_provider()?)).await }), DenoSubcommand::Lint(lint_flags) => spawn_subcommand(async { if lint_flags.rules { diff --git a/cli/npm/installer/common/mod.rs b/cli/npm/installer/common/mod.rs index 7afcef7e49..237ae27f84 100644 --- a/cli/npm/installer/common/mod.rs +++ b/cli/npm/installer/common/mod.rs @@ -1,6 +1,5 @@ // Copyright 2018-2025 the Deno authors. MIT license. -use std::collections::HashMap; use std::path::Path; use std::sync::Arc; @@ -14,6 +13,7 @@ use deno_semver::package::PackageNv; use super::PackageCaching; use crate::npm::CliNpmCache; +use crate::npm::WorkspaceNpmPatchPackages; pub mod bin_entries; pub mod lifecycle_scripts; @@ -56,6 +56,7 @@ pub struct ExtraInfoProvider { npm_cache: Arc, npm_registry_info_provider: Arc, cache: RwLock>, + workspace_patch_packages: Arc, } impl std::fmt::Debug for ExtraInfoProvider { @@ -71,11 +72,13 @@ impl ExtraInfoProvider { pub fn new( npm_cache: Arc, npm_registry_info_provider: Arc, + workspace_patch_packages: Arc, ) -> Self { Self { npm_cache, npm_registry_info_provider, cache: RwLock::new(rustc_hash::FxHashMap::default()), + workspace_patch_packages, } } } @@ -107,9 +110,8 @@ impl ExtraInfoProvider { .package_info(&package_nv.name) .await .map_err(JsErrorBox::from_err)?; - let patched_packages = HashMap::new(); let version_info = package_info - .version_info(package_nv, &patched_packages) + .version_info(package_nv, &self.workspace_patch_packages.0) .map_err(JsErrorBox::from_err)?; Ok(NpmPackageExtraInfo { deprecated: version_info.deprecated.clone(), diff --git a/cli/npm/installer/local.rs b/cli/npm/installer/local.rs index f7b8b77764..2079f2e58c 100644 --- a/cli/npm/installer/local.rs +++ b/cli/npm/installer/local.rs @@ -49,6 +49,7 @@ use crate::colors; use crate::npm::installer::common::NpmPackageExtraInfoProvider; use crate::npm::CliNpmCache; use crate::npm::CliNpmTarballCache; +use crate::npm::WorkspaceNpmPatchPackages; use crate::sys::CliSys; use crate::util::fs::clone_dir_recursive; use crate::util::fs::symlink_dir; @@ -70,6 +71,7 @@ pub struct LocalNpmPackageInstaller { system_info: NpmSystemInfo, npm_registry_info_provider: Arc, + workspace_patch_packages: Arc, } impl std::fmt::Debug for LocalNpmPackageInstaller { @@ -103,6 +105,7 @@ impl LocalNpmPackageInstaller { npm_registry_info_provider: Arc< dyn deno_npm::registry::NpmRegistryApi + Send + Sync, >, + workspace_patch_packages: Arc, ) -> Self { Self { cache, @@ -115,6 +118,7 @@ impl LocalNpmPackageInstaller { root_node_modules_path: node_modules_folder, system_info, npm_registry_info_provider, + workspace_patch_packages, } } } @@ -140,6 +144,7 @@ impl NpmPackageFsInstaller for LocalNpmPackageInstaller { &self.sys, &self.system_info, &self.lifecycle_scripts, + &self.workspace_patch_packages, ) .await .map_err(JsErrorBox::from_err) @@ -246,6 +251,7 @@ async fn sync_resolution_with_fs( sys: &CliSys, system_info: &NpmSystemInfo, lifecycle_scripts_config: &LifecycleScriptsConfig, + workspace_patch_packages: &Arc, ) -> Result<(), SyncResolutionWithFsError> { if snapshot.is_empty() && npm_install_deps_provider.local_pkgs().is_empty() { return Ok(()); // don't create the directory @@ -318,6 +324,7 @@ async fn sync_resolution_with_fs( let extra_info_provider = Arc::new(super::common::ExtraInfoProvider::new( cache.clone(), npm_registry_info_provider.clone(), + workspace_patch_packages.clone(), )); for package in &package_partitions.packages { if let Some(current_pkg) = diff --git a/cli/npm/installer/mod.rs b/cli/npm/installer/mod.rs index 0c8bfb151b..5ea9b84993 100644 --- a/cli/npm/installer/mod.rs +++ b/cli/npm/installer/mod.rs @@ -21,6 +21,7 @@ use self::local::LocalNpmPackageInstaller; pub use self::resolution::AddPkgReqsResult; pub use self::resolution::NpmResolutionInstaller; use super::NpmResolutionInitializer; +use super::WorkspaceNpmPatchPackages; use crate::args::CliLockfile; use crate::args::LifecycleScriptsConfig; use crate::args::NpmInstallDepsProvider; @@ -71,6 +72,7 @@ impl NpmInstaller { maybe_node_modules_path: Option, lifecycle_scripts: LifecycleScriptsConfig, system_info: NpmSystemInfo, + workspace_patch_packages: Arc, ) -> Self { let fs_installer: Arc = match maybe_node_modules_path { @@ -85,6 +87,7 @@ impl NpmInstaller { lifecycle_scripts, system_info, npm_registry_info_provider, + workspace_patch_packages, )), None => Arc::new(GlobalNpmPackageInstaller::new( npm_cache, diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 213c68f1d5..dcdb2ea081 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -8,7 +8,6 @@ use std::sync::Arc; use dashmap::DashMap; use deno_config::workspace::Workspace; -use deno_core::anyhow::anyhow; use deno_core::futures::stream::FuturesOrdered; use deno_core::futures::TryStreamExt; use deno_core::serde_json; @@ -56,7 +55,85 @@ pub type CliNpmResolverCreateOptions = pub type CliByonmNpmResolverCreateOptions = ByonmNpmResolverCreateOptions; -pub struct NpmPackageInfoApiAdapter(pub Arc); +pub struct NpmPackageInfoApiAdapter { + api: Arc, + workspace_patch_packages: Arc, +} + +impl NpmPackageInfoApiAdapter { + pub fn new( + api: Arc, + workspace_patch_packages: Arc, + ) -> Self { + Self { + api, + workspace_patch_packages, + } + } +} +async fn get_infos( + info_provider: &(dyn NpmRegistryApi + Send + Sync), + workspace_patch_packages: &WorkspaceNpmPatchPackages, + values: &[PackageNv], +) -> Result< + Vec, + Box, +> { + let futs = values + .iter() + .map(|v| async move { + let info = info_provider.package_info(v.name.as_str()).await?; + let version_info = info.version_info(v, &workspace_patch_packages.0)?; + Ok::<_, Box>( + deno_lockfile::Lockfile5NpmInfo { + tarball_url: version_info.dist.as_ref().and_then(|d| { + if d.tarball + == DefaultTarballUrl.default_tarball_url(&NpmPackageId { + nv: v.clone(), + // TODO(nathanwhit): this function takes an `NpmPackageId` + // but it should take a `PackageNv`. For now just + // use default here. + peer_dependencies: Default::default(), + }) + { + None + } else { + Some(d.tarball.clone()) + } + }), + optional_dependencies: version_info + .optional_dependencies + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect::>(), + cpu: version_info.cpu.iter().map(|s| s.to_string()).collect(), + os: version_info.os.iter().map(|s| s.to_string()).collect(), + deprecated: version_info.deprecated.is_some(), + has_bin: version_info.bin.is_some(), + has_scripts: version_info.scripts.contains_key("preinstall") + || version_info.scripts.contains_key("install") + || version_info.scripts.contains_key("postinstall"), + optional_peers: version_info + .peer_dependencies_meta + .iter() + .filter_map(|(k, v)| { + if v.optional { + version_info + .peer_dependencies + .get(k) + .map(|v| (k.to_string(), v.to_string())) + } else { + None + } + }) + .collect::>(), + }, + ) + }) + .collect::>(); + let package_infos = futs.try_collect::>().await?; + Ok(package_infos) +} #[async_trait::async_trait(?Send)] impl deno_lockfile::NpmPackageInfoProvider for NpmPackageInfoApiAdapter { @@ -67,62 +144,19 @@ impl deno_lockfile::NpmPackageInfoProvider for NpmPackageInfoApiAdapter { Vec, Box, > { - let futs = values - .iter() - .map(|v| async move { - let info = self.0.package_info(v.name.as_str()).await?; - let version_info = info.versions.get(&v.version).ok_or_else(|| { - anyhow!("Version {} not found for package {}", v.version, v.name) - })?; - Ok::<_, Box>( - deno_lockfile::Lockfile5NpmInfo { - tarball_url: version_info.dist.as_ref().and_then(|d| { - if d.tarball - == DefaultTarballUrl.default_tarball_url(&NpmPackageId { - nv: v.clone(), - // TODO(nathanwhit): this function takes an `NpmPackageId` - // but it should take a `PackageNv`. For now just - // use default here. - peer_dependencies: Default::default(), - }) - { - None - } else { - Some(d.tarball.clone()) - } - }), - optional_dependencies: version_info - .optional_dependencies - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect::>(), - cpu: version_info.cpu.iter().map(|s| s.to_string()).collect(), - os: version_info.os.iter().map(|s| s.to_string()).collect(), - deprecated: version_info.deprecated.is_some(), - has_bin: version_info.bin.is_some(), - has_scripts: version_info.scripts.contains_key("preinstall") - || version_info.scripts.contains_key("install") - || version_info.scripts.contains_key("postinstall"), - optional_peers: version_info - .peer_dependencies_meta - .iter() - .filter_map(|(k, v)| { - if v.optional { - version_info - .peer_dependencies - .get(k) - .map(|v| (k.to_string(), v.to_string())) - } else { - None - } - }) - .collect::>(), - }, - ) - }) - .collect::>(); - let package_infos = futs.try_collect::>().await?; - Ok(package_infos) + let package_infos = + get_infos(&*self.api, &self.workspace_patch_packages, values).await; + + match package_infos { + Ok(package_infos) => Ok(package_infos), + Err(err) => { + if self.api.mark_force_reload() { + get_infos(&*self.api, &self.workspace_patch_packages, values).await + } else { + Err(err) + } + } + } } } diff --git a/cli/tools/jupyter/mod.rs b/cli/tools/jupyter/mod.rs index 137824a6b2..0826731f40 100644 --- a/cli/tools/jupyter/mod.rs +++ b/cli/tools/jupyter/mod.rs @@ -62,7 +62,7 @@ pub async fn kernel( let factory = CliFactory::from_flags(flags); let registry_provider = - Arc::new(factory.npm_registry_info_provider()?.as_npm_registry_api()); + Arc::new(factory.lockfile_npm_package_info_provider()?); let cli_options = factory.cli_options()?; let main_module = resolve_url_or_path("./$deno$jupyter.mts", cli_options.initial_cwd()) diff --git a/cli/tools/repl/mod.rs b/cli/tools/repl/mod.rs index b23668f1b8..0f2b61e533 100644 --- a/cli/tools/repl/mod.rs +++ b/cli/tools/repl/mod.rs @@ -200,7 +200,7 @@ pub async fn run( worker, main_module.clone(), test_event_receiver, - Arc::new(factory.npm_registry_info_provider()?.as_npm_registry_api()), + Arc::new(factory.lockfile_npm_package_info_provider()?), ) .await?; let rustyline_channel = rustyline_channel(); diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 0d5f0b733b..2351eb8920 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -33,7 +33,6 @@ use deno_graph::Position; use deno_graph::PositionRange; use deno_graph::SpecifierWithRange; use deno_lib::util::result::any_and_jserrorbox_downcast_ref; -use deno_npm::registry::NpmRegistryApi; use deno_runtime::worker::MainWorker; use deno_semver::npm::NpmPackageReqReference; use node_resolver::NodeResolutionKind; @@ -210,7 +209,9 @@ impl ReplSession { mut worker: MainWorker, main_module: ModuleSpecifier, test_event_receiver: TestEventReceiver, - registry_provider: Arc, + registry_provider: Arc< + dyn deno_lockfile::NpmPackageInfoProvider + Send + Sync, + >, ) -> Result { let language_server = ReplLanguageServer::new_initialized(registry_provider).await?; diff --git a/tests/specs/lockfile/out_of_date_npm_info/__test__.jsonc b/tests/specs/lockfile/out_of_date_npm_info/__test__.jsonc new file mode 100644 index 0000000000..3dba46a2b5 --- /dev/null +++ b/tests/specs/lockfile/out_of_date_npm_info/__test__.jsonc @@ -0,0 +1,37 @@ +{ + "tempDir": true, + "envs": { + "DENO_DIR": "$PWD/deno_dir" + }, + "steps": [ + { + "args": "install", + "output": "[WILDCARD]" + }, + { + "args": "run -A --no-lock ./rm-version.ts $PWD/deno_dir/npm/localhost_4260/@denotest/add/registry.json 1.0.0", + "output": "[WILDCARD]" + }, + { + "args": "run -A --no-lock ./mv.ts ./deno.lock.old ./deno.lock", + "output": "" + }, + // trigger a lockfile update + { + "args": "install npm:chalk", + "output": "[WILDCARD]" + }, + { + "args": "remove npm:chalk", + "output": "[WILDCARD]" + }, + { + "args": [ + "eval", + "--no-lock", + "console.log(Deno.readTextFileSync('./deno.lock').trim())" + ], + "output": "deno.lock.out" + } + ] +} diff --git a/tests/specs/lockfile/out_of_date_npm_info/deno.json b/tests/specs/lockfile/out_of_date_npm_info/deno.json new file mode 100644 index 0000000000..d00ce1095e --- /dev/null +++ b/tests/specs/lockfile/out_of_date_npm_info/deno.json @@ -0,0 +1,5 @@ +{ + "imports": { + "@denotest/add": "npm:@denotest/add@1.0.0" + } +} diff --git a/tests/specs/lockfile/out_of_date_npm_info/deno.lock.old b/tests/specs/lockfile/out_of_date_npm_info/deno.lock.old new file mode 100644 index 0000000000..3cc272b12c --- /dev/null +++ b/tests/specs/lockfile/out_of_date_npm_info/deno.lock.old @@ -0,0 +1,16 @@ +{ + "version": "4", + "specifiers": { + "npm:@denotest/add@1.0.0": "1.0.0" + }, + "npm": { + "@denotest/add@1.0.0": { + "integrity": "3b2e675c1ad7fba2a45bc251992e01aff08a3c974ac09079b11e6a5b95d4bfcb" + } + }, + "workspace": { + "dependencies": [ + "npm:@denotest/add@1.0.0" + ] + } +} diff --git a/tests/specs/lockfile/out_of_date_npm_info/deno.lock.out b/tests/specs/lockfile/out_of_date_npm_info/deno.lock.out new file mode 100644 index 0000000000..cd245e66e7 --- /dev/null +++ b/tests/specs/lockfile/out_of_date_npm_info/deno.lock.out @@ -0,0 +1,17 @@ +{ + "version": "5", + "specifiers": { + "npm:@denotest/add@1.0.0": "1.0.0" + }, + "npm": { + "@denotest/add@1.0.0": { + "integrity": "sha512-P7KqAn8qFLI/13TfERSPS0NYt7CmA0diT6bhscyT6FzZjFaAVCja3K/dk96eZDsCyI1gtwtHg5Ahh8XDIvAgrw==", + "tarball": "http://localhost:4260/@denotest/add/1.0.0.tgz" + } + }, + "workspace": { + "dependencies": [ + "npm:@denotest/add@1.0.0" + ] + } +} diff --git a/tests/specs/lockfile/out_of_date_npm_info/mv.ts b/tests/specs/lockfile/out_of_date_npm_info/mv.ts new file mode 100644 index 0000000000..bd948e4169 --- /dev/null +++ b/tests/specs/lockfile/out_of_date_npm_info/mv.ts @@ -0,0 +1,3 @@ +const [from, to] = Deno.args; + +Deno.renameSync(from.trim(), to.trim()); diff --git a/tests/specs/lockfile/out_of_date_npm_info/rm-version.ts b/tests/specs/lockfile/out_of_date_npm_info/rm-version.ts new file mode 100644 index 0000000000..9b81f99df7 --- /dev/null +++ b/tests/specs/lockfile/out_of_date_npm_info/rm-version.ts @@ -0,0 +1,14 @@ +const registryPath = Deno.args[0].trim(); +const version = Deno.args[1].trim(); + +const registryJson = JSON.parse(Deno.readTextFileSync(registryPath)); + +delete registryJson.versions[version]; + +if (registryJson["dist-tags"]["latest"] === version) { + const latestVersion = Object.keys(registryJson.versions).sort()[0]; + registryJson["dist-tags"]["latest"] = latestVersion; +} +const registryJsonString = JSON.stringify(registryJson, null, 2); +Deno.writeTextFileSync(registryPath, registryJsonString); +console.log(registryJsonString);