diff --git a/cli/args/mod.rs b/cli/args/mod.rs index b32cffd76f..4026edc47f 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -1225,7 +1225,15 @@ impl CliOptions { } pub fn default_npm_caching_strategy(&self) -> NpmCachingStrategy { - if self.flags.unstable_config.npm_lazy_caching { + if matches!( + self.sub_command(), + DenoSubcommand::Install(InstallFlags::Local( + InstallFlagsLocal::TopLevel | InstallFlagsLocal::Add(_) + )) | DenoSubcommand::Add(_) + | DenoSubcommand::Outdated(_) + ) { + NpmCachingStrategy::Manual + } else if self.flags.unstable_config.npm_lazy_caching { NpmCachingStrategy::Lazy } else { NpmCachingStrategy::Eager diff --git a/cli/npm/installer/common/lifecycle_scripts.rs b/cli/npm/installer/common/lifecycle_scripts.rs index 0938956db2..5ec52b2a81 100644 --- a/cli/npm/installer/common/lifecycle_scripts.rs +++ b/cli/npm/installer/common/lifecycle_scripts.rs @@ -437,7 +437,10 @@ async fn resolve_custom_commands_from_packages< extra.clone() } else { let Ok(extra) = provider - .get_package_extra_info(&package.id.nv, package.is_deprecated) + .get_package_extra_info( + &package.id.nv, + super::ExpectedExtraInfo::from_package(package), + ) .await else { continue; diff --git a/cli/npm/installer/common/mod.rs b/cli/npm/installer/common/mod.rs index ca950e0a18..3595c0cf34 100644 --- a/cli/npm/installer/common/mod.rs +++ b/cli/npm/installer/common/mod.rs @@ -8,6 +8,7 @@ use deno_core::parking_lot::RwLock; use deno_error::JsErrorBox; use deno_npm::registry::NpmRegistryApi; use deno_npm::NpmPackageExtraInfo; +use deno_npm::NpmResolutionPackage; use deno_semver::package::PackageNv; use super::PackageCaching; @@ -29,7 +30,7 @@ pub trait NpmPackageExtraInfoProvider: std::fmt::Debug + Send + Sync { async fn get_package_extra_info( &self, package_id: &PackageNv, - is_deprecated: bool, + expected: ExpectedExtraInfo, ) -> Result; } @@ -39,11 +40,11 @@ impl NpmPackageExtraInfoProvider async fn get_package_extra_info( &self, package_id: &PackageNv, - is_deprecated: bool, + expected: ExpectedExtraInfo, ) -> Result { self .as_ref() - .get_package_extra_info(package_id, is_deprecated) + .get_package_extra_info(package_id, expected) .await } } @@ -76,46 +77,96 @@ impl ExtraInfoProvider { } } +#[derive(Debug, Clone, Copy, Default)] +pub struct ExpectedExtraInfo { + pub deprecated: bool, + pub bin: bool, + pub scripts: bool, +} + +impl ExpectedExtraInfo { + pub fn from_package(package: &NpmResolutionPackage) -> Self { + Self { + deprecated: package.is_deprecated, + bin: package.has_bin, + scripts: package.has_scripts, + } + } +} + +impl ExtraInfoProvider { + async fn fetch_from_registry( + &self, + package_nv: &PackageNv, + ) -> Result { + let package_info = self + .npm_registry_info_provider + .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) + .map_err(JsErrorBox::from_err)?; + Ok(NpmPackageExtraInfo { + deprecated: version_info.deprecated.clone(), + bin: version_info.bin.clone(), + scripts: version_info.scripts.clone(), + }) + } + + async fn fetch_from_package_json( + &self, + package_nv: &PackageNv, + ) -> Result { + let folder_path = self.npm_cache.package_folder_for_nv(package_nv); + let package_json_path = folder_path.join("package.json"); + let extra_info: NpmPackageExtraInfo = + tokio::task::spawn_blocking(move || { + let package_json = std::fs::read_to_string(&package_json_path) + .map_err(JsErrorBox::from_err)?; + let extra_info: NpmPackageExtraInfo = + deno_core::serde_json::from_str(&package_json) + .map_err(JsErrorBox::from_err)?; + + Ok::<_, JsErrorBox>(extra_info) + }) + .await + .map_err(JsErrorBox::from_err)??; + Ok(extra_info) + } +} + impl super::common::NpmPackageExtraInfoProvider for ExtraInfoProvider { async fn get_package_extra_info( &self, package_nv: &PackageNv, - deprecated: bool, + expected: ExpectedExtraInfo, ) -> Result { if let Some(extra_info) = self.cache.read().get(package_nv) { return Ok(extra_info.clone()); } - if deprecated { + let extra_info = if expected.deprecated { // we need the registry version info to get the deprecated string, since it's not in the // package's package.json - let package_info = self - .npm_registry_info_provider - .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) - .map_err(JsErrorBox::from_err)?; - Ok(NpmPackageExtraInfo { - deprecated: version_info.deprecated.clone(), - bin: version_info.bin.clone(), - scripts: version_info.scripts.clone(), - }) + self.fetch_from_registry(package_nv).await? } else { - let folder_path = self.npm_cache.package_folder_for_nv(package_nv); - let package_json_path = folder_path.join("package.json"); - let package_json = std::fs::read_to_string(&package_json_path) - .map_err(JsErrorBox::from_err)?; - let extra_info: NpmPackageExtraInfo = - deno_core::serde_json::from_str(&package_json) - .map_err(JsErrorBox::from_err)?; - self - .cache - .write() - .insert(package_nv.clone(), extra_info.clone()); - Ok(extra_info) - } + let extra_info = self.fetch_from_package_json(package_nv).await?; + // some packages have bin in registry but not in package.json (e.g. esbuild-wasm) + // still not sure how that happens + if (expected.bin && extra_info.bin.is_none()) + || (expected.scripts && extra_info.scripts.is_empty()) + { + self.fetch_from_registry(package_nv).await? + } else { + extra_info + } + }; + self + .cache + .write() + .insert(package_nv.clone(), extra_info.clone()); + Ok(extra_info) } } diff --git a/cli/npm/installer/local.rs b/cli/npm/installer/local.rs index f78d29943e..089e4e738c 100644 --- a/cli/npm/installer/local.rs +++ b/cli/npm/installer/local.rs @@ -414,12 +414,16 @@ async fn sync_resolution_with_fs( Ok::<_, SyncResolutionWithFsError>(()) } }); - let extra_fut = if package.has_bin + let extra_fut = if (package.has_bin || package.has_scripts - || package.is_deprecated && package.extra.is_none() + || package.is_deprecated) + && package.extra.is_none() { extra_info_provider - .get_package_extra_info(&package.id.nv, package.is_deprecated) + .get_package_extra_info( + &package.id.nv, + super::common::ExpectedExtraInfo::from_package(package), + ) .boxed_local() } else { std::future::ready(Ok(package.extra.clone().unwrap_or_default())) @@ -465,7 +469,10 @@ async fn sync_resolution_with_fs( cache_futures.push( async move { let extra = extra_info_provider - .get_package_extra_info(&package.id.nv, package.is_deprecated) + .get_package_extra_info( + &package.id.nv, + super::common::ExpectedExtraInfo::from_package(package), + ) .await .map_err(JsErrorBox::from_err)?; diff --git a/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.2.5-missingdeprecated.tgz b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.2.5-missingdeprecated.tgz new file mode 100644 index 0000000000..469bda48b3 Binary files /dev/null and b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.2.5-missingdeprecated.tgz differ 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 new file mode 100644 index 0000000000..bb789c3d68 Binary files /dev/null and b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz differ diff --git a/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-1.0.0-missingbin.tgz b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-1.0.0-missingbin.tgz new file mode 100644 index 0000000000..b176594358 Binary files /dev/null and b/tests/registry/npm/denotest-packagejson-missing-info/denotest-packagejson-missing-info-1.0.0-missingbin.tgz differ diff --git a/tests/registry/npm/denotest-packagejson-missing-info/registry.json b/tests/registry/npm/denotest-packagejson-missing-info/registry.json new file mode 100644 index 0000000000..f26a9e8f7c --- /dev/null +++ b/tests/registry/npm/denotest-packagejson-missing-info/registry.json @@ -0,0 +1,36 @@ +{ + "name": "denotest-packagejson-missing-info", + "dist-tags": { + "latest": "1.0.0-missingbin" + }, + "versions": { + "1.0.0-missingbin": { + "name": "denotest-packagejson-missing-info", + "version": "1.0.0-missingbin", + "bin": { + "denotest": "./bin/denotest.js" + }, + "dist": { + "tarball": "http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-1.0.0-missingbin.tgz" + } + }, + "0.5.0-missingscripts": { + "name": "denotest-packagejson-missing-info", + "version": "0.5.0-missingscripts", + "scripts": { + "postinstall": "echo 'postinstall'" + }, + "dist": { + "tarball": "http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.5.0-missingscripts.tgz" + } + }, + "0.2.5-missingdeprecated": { + "name": "denotest-packagejson-missing-info", + "version": "0.2.5-missingdeprecated", + "deprecated": "Use 0.5.0 or later", + "dist": { + "tarball": "http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.2.5-missingdeprecated.tgz" + } + } + } +} \ No newline at end of file diff --git a/tests/specs/install/packagejson_missing_extra/__test__.jsonc b/tests/specs/install/packagejson_missing_extra/__test__.jsonc new file mode 100644 index 0000000000..71165c348a --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/__test__.jsonc @@ -0,0 +1,45 @@ +{ + "tempDir": true, + "tests": { + "missing_bin": { + "steps": [ + { + "args": "install npm:denotest-packagejson-missing-info@1.0.0-missingbin", + "output": "missingbin.out" + }, + { + "args": "run -A ./has-bin.ts denotest", + "output": "" + } + ] + }, + "missing_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" + } + ] + }, + "missing_deprecated": { + "steps": [ + { + "args": "install npm:denotest-packagejson-missing-info@0.2.5-missingdeprecated", + "output": "missingdeprecated.out" + } + ] + }, + "missing_deprecated_with_lazy_caching": { + "steps": [ + { + "args": "install --unstable-npm-lazy-caching npm:denotest-packagejson-missing-info@0.2.5-missingdeprecated", + "output": "missingdeprecated.out" + } + ] + } + } +} diff --git a/tests/specs/install/packagejson_missing_extra/deno.json b/tests/specs/install/packagejson_missing_extra/deno.json new file mode 100644 index 0000000000..fbd70ec480 --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/deno.json @@ -0,0 +1,3 @@ +{ + "nodeModulesDir": "auto" +} diff --git a/tests/specs/install/packagejson_missing_extra/has-bin.ts b/tests/specs/install/packagejson_missing_extra/has-bin.ts new file mode 100644 index 0000000000..aebf17f3eb --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/has-bin.ts @@ -0,0 +1,20 @@ +const name = Deno.args[0].trim(); + +function exists(path: string) { + try { + Deno.statSync(path); + return true; + } catch (error) { + return false; + } +} + +if ( + !(exists(`node_modules/.bin/${name}`) || + exists(`node_modules/.bin/${name}.cmd`)) +) { + console.log("missing bin"); + console.log(`node_modules/.bin/${name}`); + console.log(Deno.readDirSync("node_modules/.bin").toArray()); + Deno.exit(1); +} diff --git a/tests/specs/install/packagejson_missing_extra/missingbin.out b/tests/specs/install/packagejson_missing_extra/missingbin.out new file mode 100644 index 0000000000..2369092aaa --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/missingbin.out @@ -0,0 +1,4 @@ +Add npm:denotest-packagejson-missing-info@1.0.0-missingbin +Download http://localhost:4260/denotest-packagejson-missing-info +Download http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-1.0.0-missingbin.tgz +Initialize denotest-packagejson-missing-info@1.0.0-missingbin diff --git a/tests/specs/install/packagejson_missing_extra/missingdeprecated.out b/tests/specs/install/packagejson_missing_extra/missingdeprecated.out new file mode 100644 index 0000000000..15467dcce2 --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/missingdeprecated.out @@ -0,0 +1,6 @@ +Add npm:denotest-packagejson-missing-info@0.2.5-missingdeprecated +Download http://localhost:4260/denotest-packagejson-missing-info +Download http://localhost:4260/denotest-packagejson-missing-info/denotest-packagejson-missing-info-0.2.5-missingdeprecated.tgz +Initialize denotest-packagejson-missing-info@0.2.5-missingdeprecated +Warning The following packages are deprecated: +┖─ npm:denotest-packagejson-missing-info@0.2.5-missingdeprecated (Use 0.5.0 or later) diff --git a/tests/specs/install/packagejson_missing_extra/missingscripts.out b/tests/specs/install/packagejson_missing_extra/missingscripts.out new file mode 100644 index 0000000000..ebea77f6af --- /dev/null +++ b/tests/specs/install/packagejson_missing_extra/missingscripts.out @@ -0,0 +1,5 @@ +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 +Warning The following packages contained npm lifecycle scripts (preinstall/install/postinstall) that were not executed:[WILDCARD] \ No newline at end of file