diff --git a/cli/factory.rs b/cli/factory.rs index c724135fe63c13..5b4f4e7681eb0d 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -95,6 +95,7 @@ use crate::npm::CliNpmInstaller; use crate::npm::CliNpmInstallerFactory; use crate::npm::CliNpmResolver; use crate::npm::DenoTaskLifeCycleScriptsExecutor; +use crate::npm::NpmPackumentFormat; use crate::resolver::CliCjsTracker; use crate::resolver::CliNpmReqResolver; use crate::resolver::CliResolver; @@ -579,11 +580,21 @@ impl CliFactory { self.services.npm_installer_factory.get_or_try_init(|| { let cli_options = self.cli_options()?; let resolver_factory = self.resolver_factory()?; + let needs_full_packument = resolver_factory + .minimum_dependency_age_config() + .ok() + .and_then(|c| c.age.as_ref().and_then(|d| d.into_option())) + .is_some(); Ok(CliNpmInstallerFactory::new( resolver_factory.clone(), Arc::new(CliNpmCacheHttpClient::new( self.http_client_provider().clone(), self.text_only_progress_bar().clone(), + if needs_full_packument { + NpmPackumentFormat::Full + } else { + NpmPackumentFormat::Abbreviated + }, )), match resolver_factory.npm_resolver()?.as_managed() { Some(managed_npm_resolver) => { diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 61e54aff6d17d9..244f968b977add 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -69,6 +69,7 @@ use crate::file_fetcher::CliFileFetcher; use crate::http_util::HttpClientProvider; use crate::lsp::logging::lsp_warn; use crate::npm::CliNpmCacheHttpClient; +use crate::npm::NpmPackumentFormat; use crate::sys::CliSys; use crate::util::fs::canonicalize_path; use crate::util::fs::canonicalize_path_maybe_not_exists; @@ -1483,6 +1484,7 @@ impl ConfigData { // will only happen in the tests .unwrap_or_else(|| Arc::new(HttpClientProvider::new(None, None))), pb.clone(), + NpmPackumentFormat::Abbreviated, )), Arc::new(NullLifecycleScriptsExecutor), pb, diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 6ae336c83e63c8..d11f4e70014bb2 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -81,6 +81,7 @@ use crate::npm::CliNpmInstaller; use crate::npm::CliNpmRegistryInfoProvider; use crate::npm::CliNpmResolver; use crate::npm::CliNpmResolverCreateOptions; +use crate::npm::NpmPackumentFormat; use crate::resolver::CliIsCjsResolver; use crate::resolver::CliNpmReqResolver; use crate::resolver::CliResolver; @@ -906,11 +907,13 @@ impl<'a> ResolverFactory<'a> { let npm_client = Arc::new(CliNpmCacheHttpClient::new( http_client_provider.clone(), pb.clone(), + NpmPackumentFormat::Abbreviated, )); let registry_info_provider = Arc::new(CliNpmRegistryInfoProvider::new( npm_cache.clone(), npm_client.clone(), npmrc.clone(), + NpmPackumentFormat::Abbreviated, )); let link_packages: WorkspaceNpmLinkPackagesRc = self .config_data diff --git a/cli/npm.rs b/cli/npm.rs index c38c453b5e4f3c..669760419822ff 100644 --- a/cli/npm.rs +++ b/cli/npm.rs @@ -64,20 +64,25 @@ pub type CliNpmGraphResolver = deno_npm_installer::graph::NpmDenoGraphResolver< CliSys, >; +pub use deno_npm_cache::NpmPackumentFormat; + #[derive(Debug)] pub struct CliNpmCacheHttpClient { http_client_provider: Arc, progress_bar: ProgressBar, + packument_format: NpmPackumentFormat, } impl CliNpmCacheHttpClient { pub fn new( http_client_provider: Arc, progress_bar: ProgressBar, + packument_format: NpmPackumentFormat, ) -> Self { Self { http_client_provider, progress_bar, + packument_format, } } } @@ -110,6 +115,22 @@ impl deno_npm_cache::NpmCacheHttpClient for CliNpmCacheHttpClient { http::header::HeaderValue::try_from(etag).unwrap(), ); } + if self.packument_format == NpmPackumentFormat::Abbreviated { + // Request the abbreviated install manifest when possible. This is 2-5x + // smaller than the full packument (e.g. @types/node: 2.3 MB vs 10.9 MB). + // Uses content negotiation with quality factors for registry compatibility + // (some registries like older Artifactory don't support the abbreviated + // format and need the JSON fallback). + // + // Not used when minimumDependencyAge is configured, because the + // abbreviated format omits the `time` field needed for date filtering. + headers.insert( + http::header::ACCEPT, + http::header::HeaderValue::from_static( + "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*", + ), + ); + } client .download_with_progress_and_retries(url, &headers, &guard) .await diff --git a/libs/npm/registry.rs b/libs/npm/registry.rs index be8068552a8a2a..1d27f36d8eee2b 100644 --- a/libs/npm/registry.rs +++ b/libs/npm/registry.rs @@ -229,6 +229,12 @@ pub struct NpmPackageVersionInfo { #[serde(default, skip_serializing_if = "HashMap::is_empty")] #[serde(deserialize_with = "deserializers::hashmap")] pub scripts: HashMap, + /// From the abbreviated install manifest format. When `true`, this version + /// has preinstall/install/postinstall lifecycle scripts. This field is only + /// present in abbreviated packuments where the full `scripts` map is not + /// available. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub has_install_script: Option, #[serde(default, skip_serializing_if = "Option::is_none")] #[serde(deserialize_with = "deserializers::string")] pub deprecated: Option, @@ -1498,6 +1504,7 @@ mod test { os: Default::default(), cpu: Default::default(), scripts: Default::default(), + has_install_script: Default::default(), deprecated: Default::default(), }; let text = serde_json::to_string(&data).unwrap(); diff --git a/libs/npm/resolution/graph.rs b/libs/npm/resolution/graph.rs index 66879c3fba169d..1cec88b8274ce5 100644 --- a/libs/npm/resolution/graph.rs +++ b/libs/npm/resolution/graph.rs @@ -790,7 +790,8 @@ impl Graph { }), is_deprecated: version_info.deprecated.is_some(), has_bin: version_info.bin.is_some(), - has_scripts: version_info.scripts.contains_key("preinstall") + has_scripts: version_info.has_install_script.unwrap_or(false) + || version_info.scripts.contains_key("preinstall") || version_info.scripts.contains_key("install") || version_info.scripts.contains_key("postinstall"), optional_peer_dependencies: version_info diff --git a/libs/npm_cache/lib.rs b/libs/npm_cache/lib.rs index 275937931fa7d8..0f6a7baaaf8346 100644 --- a/libs/npm_cache/lib.rs +++ b/libs/npm_cache/lib.rs @@ -68,6 +68,14 @@ impl std::fmt::Display for DownloadError { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NpmPackumentFormat { + /// Request the abbreviated install manifest (smaller, but omits `time` and `scripts`). + Abbreviated, + /// Request the full packument (needed when `minimumDependencyAge` is configured). + Full, +} + pub enum NpmCacheHttpClientResponse { NotFound, NotModified, diff --git a/libs/npm_cache/registry_info.rs b/libs/npm_cache/registry_info.rs index 704eee577504f5..bad81b7f8b5f08 100644 --- a/libs/npm_cache/registry_info.rs +++ b/libs/npm_cache/registry_info.rs @@ -23,6 +23,7 @@ use crate::NpmCacheHttpClient; use crate::NpmCacheHttpClientResponse; use crate::NpmCacheSetting; use crate::NpmCacheSys; +use crate::NpmPackumentFormat; use crate::remote::maybe_auth_header_value_for_npm_registry; use crate::rt::MultiRuntimeAsyncValueCreator; use crate::rt::spawn_blocking; @@ -140,6 +141,7 @@ struct RegistryInfoProviderInner< cache: Arc>, http_client: Arc, npmrc: Arc, + packument_format: NpmPackumentFormat, force_reload_flag: AtomicFlag, memory_cache: Mutex, previously_loaded_packages: Mutex>, @@ -251,7 +253,15 @@ impl { // attempt to load from the file cache match downloader.cache.load_package_info(&name).await.map_err(JsErrorBox::from_err)? { Some(cached_info) => { - return Ok(FutureResult::SavedFsCache(Arc::new(cached_info.info))); + if downloader.packument_format == NpmPackumentFormat::Full && cached_info.info.time.is_empty() && !cached_info.info.versions.is_empty() { + // Cached data is from the abbreviated install manifest which + // doesn't include the `time` field. Since minimumDependencyAge + // is configured, we need to re-fetch the full packument. + // Don't use the etag since it corresponds to the abbreviated format. + Some(SerializedCachedPackageInfo { etag: None, ..cached_info }) + } else { + return Ok(FutureResult::SavedFsCache(Arc::new(cached_info.info))); + } } _ => { None }} @@ -353,11 +363,13 @@ impl cache: Arc>, http_client: Arc, npmrc: Arc, + packument_format: NpmPackumentFormat, ) -> Self { Self(Arc::new(RegistryInfoProviderInner { cache, http_client, npmrc, + packument_format, force_reload_flag: AtomicFlag::lowered(), memory_cache: Default::default(), previously_loaded_packages: Default::default(), diff --git a/libs/npm_installer/extra_info.rs b/libs/npm_installer/extra_info.rs index c284776a73781c..08edf798e65e10 100644 --- a/libs/npm_installer/extra_info.rs +++ b/libs/npm_installer/extra_info.rs @@ -1,5 +1,6 @@ // Copyright 2018-2026 the Deno authors. MIT license. +use std::collections::HashMap; use std::path::Path; use std::sync::Arc; @@ -106,14 +107,28 @@ impl NpmPackageExtraInfoProvider { self.fetch_from_registry(package_nv).await } else { match self.fetch_from_package_json(package_path).await { - Ok(extra_info) => { + Ok(mut extra_info) => { // some packages that use "directories.bin" have a "bin" entry in // the packument, but not in package.json (e.g. esbuild-wasm) - if (expected.bin && extra_info.bin.is_none()) - || (expected.scripts && extra_info.scripts.is_empty()) - { + if expected.bin && extra_info.bin.is_none() { self.fetch_from_registry(package_nv).await } else { + // When a package has a binding.gyp and no install/preinstall script, + // npm injects `"install": "node-gyp rebuild"` at publish time. This + // script appears in the packument but not in the tarball's package.json. + // Match pnpm's behavior and detect this case by checking for the file. + if expected.scripts + && extra_info.scripts.is_empty() + && self + .sys + .base_fs_read(&package_path.join("binding.gyp")) + .is_ok() + { + extra_info.scripts = HashMap::from([( + "install".into(), + "node-gyp rebuild".to_string(), + )]); + } Ok(extra_info) } } diff --git a/libs/npm_installer/factory.rs b/libs/npm_installer/factory.rs index 58efb848db7e70..482e0755c03e31 100644 --- a/libs/npm_installer/factory.rs +++ b/libs/npm_installer/factory.rs @@ -7,6 +7,7 @@ use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot; use deno_npm_cache::NpmCache; use deno_npm_cache::NpmCacheHttpClient; use deno_npm_cache::NpmCacheSetting; +use deno_npm_cache::NpmPackumentFormat; use deno_npm_cache::RegistryInfoProvider; use deno_npm_cache::TarballCache; use deno_resolver::factory::ResolverFactory; @@ -393,10 +394,22 @@ impl< anyhow::Error, > { self.registry_info_provider.get_or_try_init(|| { + let packument_format = if self + .resolver_factory + .minimum_dependency_age_config() + .ok() + .and_then(|c| c.age.as_ref().and_then(|d| d.into_option())) + .is_some() + { + NpmPackumentFormat::Full + } else { + NpmPackumentFormat::Abbreviated + }; Ok(Arc::new(RegistryInfoProvider::new( self.npm_cache()?.clone(), self.http_client().clone(), self.workspace_factory().npmrc()?.clone(), + packument_format, ))) }) } diff --git a/libs/npm_installer/local.rs b/libs/npm_installer/local.rs index f66548fd072b6d..266f40ab5184da 100644 --- a/libs/npm_installer/local.rs +++ b/libs/npm_installer/local.rs @@ -376,32 +376,49 @@ impl< Ok::<_, SyncResolutionWithFsError>(()) } }); - let extra_fut = if (package.has_bin + let needs_extra_from_disk = package.extra.is_none() + // When using abbreviated packument format, has_scripts may + // be true (from hasInstallScript) while extra.scripts is + // empty. In that case, read from disk to get real scripts. + || (package.has_scripts + && package + .extra + .as_ref() + .is_some_and(|e| e.scripts.is_empty())); + let extra = if (package.has_bin || package.has_scripts || package.is_deprecated) - && package.extra.is_none() + && needs_extra_from_disk { + // Wait for extraction to complete first, since + // get_package_extra_info may read from the on-disk + // package.json which doesn't exist until extraction finishes. + handle + .await + .map_err(JsErrorBox::from_err)? + .map_err(JsErrorBox::from_err)?; extra_info_provider .get_package_extra_info( &package.id.nv, &package_path, ExpectedExtraInfo::from_package(package), ) - .boxed_local() + .await + .map_err(JsErrorBox::from_err)? } else { - std::future::ready(Ok( - package.extra.clone().unwrap_or_default(), - )) - .boxed_local() + let (result, extra) = futures::future::join( + handle, + std::future::ready(Ok::<_, JsErrorBox>( + package.extra.clone().unwrap_or_default(), + )), + ) + .await; + result + .map_err(JsErrorBox::from_err)? + .map_err(JsErrorBox::from_err)?; + extra? }; - let (result, extra) = - futures::future::join(handle, extra_fut).await; - result - .map_err(JsErrorBox::from_err)? - .map_err(JsErrorBox::from_err)?; - let extra = extra.map_err(JsErrorBox::from_err)?; - if package.has_bin { bin_entries_to_setup.borrow_mut().add( package, diff --git a/libs/resolver/workspace.rs b/libs/resolver/workspace.rs index 0af1dc4f0c49ac..3b80df16b071b6 100644 --- a/libs/resolver/workspace.rs +++ b/libs/resolver/workspace.rs @@ -1740,6 +1740,7 @@ fn pkg_json_to_version_info( .collect() }) .unwrap_or_default(), + has_install_script: None, // not worth increasing memory for showing a deprecated // message for linked packages deprecated: None, @@ -3233,6 +3234,7 @@ mod test { NpmPackageVersionInfo { version: Version::parse_from_npm("1.0.0").unwrap(), dist: None, + has_install_script: None, bin: Some(deno_npm::registry::NpmPackageVersionBinEntry::String( "./bin.js".to_string() )), diff --git a/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz deleted file mode 100644 index bb789c3d681a1b..00000000000000 Binary files a/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz and /dev/null differ diff --git a/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.7.0-bindinggyp.tgz b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.7.0-bindinggyp.tgz new file mode 100644 index 00000000000000..c0f6163e4a2d12 Binary files /dev/null and b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.7.0-bindinggyp.tgz differ diff --git a/tests/registry/npm/denotest-packagejson-missing-info/registry.json b/tests/registry/npm/denotest-packagejson-missing-info/registry.json index 03b193b397d027..2541e5f66fa8ab 100644 --- a/tests/registry/npm/denotest-packagejson-missing-info/registry.json +++ b/tests/registry/npm/denotest-packagejson-missing-info/registry.json @@ -14,14 +14,14 @@ "tarball": "http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-1.0.0-missingbin.tgz" } }, - "0.5.0-missingscripts": { + "0.7.0-bindinggyp": { "name": "denotest-packagejson-missing-info", - "version": "0.5.0-missingscripts", + "version": "0.7.0-bindinggyp", "scripts": { - "postinstall": "echo 'postinstall'" + "install": "node-gyp rebuild" }, "dist": { - "tarball": "http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz" + "tarball": "http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.7.0-bindinggyp.tgz" } }, "0.2.5-missingdeprecated": { diff --git a/tests/specs/install/packagejson_missing_extra/__test__.jsonc b/tests/specs/install/packagejson_missing_extra/__test__.jsonc index a6b32c1d3f24d1..d7bb679b17de9d 100644 --- a/tests/specs/install/packagejson_missing_extra/__test__.jsonc +++ b/tests/specs/install/packagejson_missing_extra/__test__.jsonc @@ -13,15 +13,11 @@ } ] }, - "missing_scripts": { + "binding_gyp_scripts": { "steps": [ { - "args": "install npm:denotest-packagejson-missing-info@0.5.0-missingscripts", - "output": "missingscripts.out" - }, - { - "args": "install --allow-scripts", - "output": "[WILDCARD]running 'postinstall' script\n[WILDCARD]" + "args": "install npm:denotest-packagejson-missing-info@0.7.0-bindinggyp", + "output": "bindinggyp.out" } ] }, diff --git a/tests/specs/install/packagejson_missing_extra/bindinggyp.out b/tests/specs/install/packagejson_missing_extra/bindinggyp.out new file mode 100644 index 00000000000000..fd2adc2315b82c --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/bindinggyp.out @@ -0,0 +1,16 @@ +Add npm:denotest-packagejson-missing-info@0.7.0-bindinggyp +Download http://localhost:4260/denotest-packagejson-missing-info +Download http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.7.0-bindinggyp.tgz +Initialize denotest-packagejson-missing-info@0.7.0-bindinggyp +Installed 1 package in [WILDCARD] +Reused 0 packages from cache +Downloaded 0 packages from JSR +Downloaded 1 package from npm ++ + +Dependencies: ++ npm:denotest-packagejson-missing-info 0.7.0-bindinggyp + +[WILDCARD]Ignored build scripts for packages: +[WILDCARD]npm:denotest-packagejson-missing-info@0.7.0-bindinggyp +[WILDCARD] diff --git a/tests/specs/install/packagejson_missing_extra/missingscripts.out b/tests/specs/install/packagejson_missing_extra/missingscripts.out deleted file mode 100644 index 074a0cbf31c7fa..00000000000000 --- a/tests/specs/install/packagejson_missing_extra/missingscripts.out +++ /dev/null @@ -1,20 +0,0 @@ -Add npm:denotest-packagejson-missing-info@0.5.0-missingscripts -Download http://localhost:4260/denotest-packagejson-missing-info -Download http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz -Initialize denotest-packagejson-missing-info@0.5.0-missingscripts -Installed 1 package in [WILDCARD] -Reused 0 packages from cache -Downloaded 0 packages from JSR -Downloaded 1 package from npm -+ - -Dependencies: -+ npm:denotest-packagejson-missing-info 0.5.0-missingscripts - -╭ Warning -│ -│ Ignored build scripts for packages: -│ npm:denotest-packagejson-missing-info@0.5.0-missingscripts -│ -│ Run "deno approve-scripts" to run build scripts. -╰─ diff --git a/tests/util/server/servers/npm_registry.rs b/tests/util/server/servers/npm_registry.rs index 5bfbab153794d5..372f09d81d5e0d 100644 --- a/tests/util/server/servers/npm_registry.rs +++ b/tests/util/server/servers/npm_registry.rs @@ -221,6 +221,56 @@ async fn handle_req_for_registry( .map_err(|e| e.into()) } +/// Returns true if the Accept header indicates the client wants the +/// abbreviated install manifest (`application/vnd.npm.install-v1+json`). +fn wants_abbreviated(headers: &HeaderMap) -> bool { + headers + .get(http::header::ACCEPT) + .and_then(|v| v.to_str().ok()) + .map(|v| v.contains("application/vnd.npm.install-v1+json")) + .unwrap_or(false) +} + +/// Converts a full packument JSON into the abbreviated install manifest format. +/// Strips `time` and `scripts`, replaces scripts with `hasInstallScript`. +fn to_abbreviated_packument(full: &[u8]) -> Vec { + let mut packument: serde_json::Value = serde_json::from_slice(full).unwrap(); + let obj = packument.as_object_mut().unwrap(); + + // Remove `time` (not present in abbreviated format) + obj.remove("time"); + + // Transform each version entry + if let Some(versions) = + obj.get_mut("versions").and_then(|v| v.as_object_mut()) + { + for version_info in versions.values_mut() { + let vi = version_info.as_object_mut().unwrap(); + + // Check for install lifecycle scripts before removing + let has_install_script = vi + .get("scripts") + .and_then(|s| s.as_object()) + .map(|scripts| { + scripts.contains_key("preinstall") + || scripts.contains_key("install") + || scripts.contains_key("postinstall") + }) + .unwrap_or(false); + + // Remove `scripts` (not present in abbreviated format) + vi.remove("scripts"); + + // Add `hasInstallScript` if applicable + if has_install_script { + vi.insert("hasInstallScript".to_string(), json!(true)); + } + } + } + + serde_json::to_vec(&packument).unwrap() +} + fn handle_custom_npm_registry_path( scope_name: &str, path: &str, @@ -246,6 +296,12 @@ fn handle_custom_npm_registry_path( && let Some(registry_file) = test_npm_registry.registry_file(&package_name)? { + let registry_file = if wants_abbreviated(headers) { + to_abbreviated_packument(®istry_file) + } else { + registry_file + }; + let actual_etag = format!( "\"{}\"", BASE64_STANDARD.encode(sha2::Sha256::digest(®istry_file)) @@ -313,6 +369,11 @@ async fn try_serve_npm_registry( testdata_file_path.push("registry.json"); } if let Ok(file) = tokio::fs::read(&testdata_file_path).await { + let file = if !is_tarball && wants_abbreviated(headers) { + to_abbreviated_packument(&file) + } else { + file + }; let file_resp = custom_headers(uri_path, file); return Some(Ok(file_resp)); } else if should_download_npm_packages() {