fix(install): read extra package info from node_modules and fallback to registry (#28893)

Fixes #28891

We were checking if the node_modules entry for the package was present,
but then reading from the global cache.

Instead, read from the package.json in node_modules. As a fallback(which
in theory should only really happen if the node_modules dir is somehow
messed up), take the more expensive (but likely to work) path of reading
from the registry.json.
This commit is contained in:
Nathan Whitaker 2025-04-14 12:32:27 -07:00 committed by GitHub
parent 01b6da9d9b
commit 914549292e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 119 additions and 13 deletions

View file

@ -439,6 +439,7 @@ async fn resolve_custom_commands_from_packages<
let Ok(extra) = provider
.get_package_extra_info(
&package.id.nv,
&package_path,
super::ExpectedExtraInfo::from_package(package),
)
.await

View file

@ -1,6 +1,7 @@
// Copyright 2018-2025 the Deno authors. MIT license.
use std::collections::HashMap;
use std::path::Path;
use std::sync::Arc;
use async_trait::async_trait;
@ -30,6 +31,7 @@ pub trait NpmPackageExtraInfoProvider: std::fmt::Debug + Send + Sync {
async fn get_package_extra_info(
&self,
package_id: &PackageNv,
package_path: &Path,
expected: ExpectedExtraInfo,
) -> Result<deno_npm::NpmPackageExtraInfo, JsErrorBox>;
}
@ -40,11 +42,12 @@ impl<T: NpmPackageExtraInfoProvider + ?Sized> NpmPackageExtraInfoProvider
async fn get_package_extra_info(
&self,
package_id: &PackageNv,
package_path: &Path,
expected: ExpectedExtraInfo,
) -> Result<deno_npm::NpmPackageExtraInfo, JsErrorBox> {
self
.as_ref()
.get_package_extra_info(package_id, expected)
.get_package_extra_info(package_id, package_path, expected)
.await
}
}
@ -117,10 +120,9 @@ impl ExtraInfoProvider {
async fn fetch_from_package_json(
&self,
package_nv: &PackageNv,
package_path: &Path,
) -> Result<NpmPackageExtraInfo, JsErrorBox> {
let folder_path = self.npm_cache.package_folder_for_nv(package_nv);
let package_json_path = folder_path.join("package.json");
let package_json_path = package_path.join("package.json");
let extra_info: NpmPackageExtraInfo =
tokio::task::spawn_blocking(move || {
let package_json = std::fs::read_to_string(&package_json_path)
@ -141,6 +143,7 @@ impl super::common::NpmPackageExtraInfoProvider for ExtraInfoProvider {
async fn get_package_extra_info(
&self,
package_nv: &PackageNv,
package_path: &Path,
expected: ExpectedExtraInfo,
) -> Result<NpmPackageExtraInfo, JsErrorBox> {
if let Some(extra_info) = self.cache.read().get(package_nv) {
@ -152,7 +155,8 @@ impl super::common::NpmPackageExtraInfoProvider for ExtraInfoProvider {
// package's package.json
self.fetch_from_registry(package_nv).await?
} else {
let extra_info = self.fetch_from_package_json(package_nv).await?;
match self.fetch_from_package_json(package_path).await {
Ok(extra_info) => {
// 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())
@ -162,6 +166,17 @@ impl super::common::NpmPackageExtraInfoProvider for ExtraInfoProvider {
} else {
extra_info
}
}
Err(err) => {
log::debug!(
"failed to get extra info for {} from package.json at {}: {}",
package_nv,
package_path.join("package.json").display(),
err
);
self.fetch_from_registry(package_nv).await?
}
}
};
self
.cache

View file

@ -422,6 +422,7 @@ async fn sync_resolution_with_fs(
extra_info_provider
.get_package_extra_info(
&package.id.nv,
&package_path,
super::common::ExpectedExtraInfo::from_package(package),
)
.boxed_local()
@ -471,6 +472,7 @@ async fn sync_resolution_with_fs(
let extra = extra_info_provider
.get_package_extra_info(
&package.id.nv,
&package_path,
super::common::ExpectedExtraInfo::from_package(package),
)
.await

View file

@ -0,0 +1,10 @@
{
"name": "@denotest/extra-info",
"version": "1.0.0",
"bin": {
"denotest-extra-info": "./bin/main.js"
},
"scripts": {
"postinstall": "echo 'postinstall'"
}
}

View file

@ -0,0 +1,78 @@
{
"tempDir": true,
"tests": {
"auto": {
"steps": [
{
"args": "install --node-modules-dir=auto npm:@denotest/extra-info",
"output": "[WILDCARD]"
},
{
"args": "clean",
"output": "[WILDCARD]"
},
{
"args": "install --node-modules-dir=auto --allow-scripts",
"output": "[WILDCARD]running 'postinstall' script\n"
}
]
},
"manual": {
"steps": [
{
"args": "install --node-modules-dir=manual npm:@denotest/extra-info",
"output": "[WILDCARD]"
},
{
"args": "clean",
"output": "[WILDCARD]"
},
{
"args": "install --node-modules-dir=manual --allow-scripts",
"output": "[WILDCARD]running 'postinstall' script\n"
}
]
},
"none": {
"steps": [
{
"args": "install --node-modules-dir=none npm:@denotest/extra-info",
"output": "[WILDCARD]"
},
{
"args": "clean",
"output": "[WILDCARD]"
},
{
// postinstall scripts aren't a thing, so just add another package to trigger
// setting up node_modules
"args": "install --node-modules-dir=none npm:chalk",
"output": "[WILDCARD]"
}
]
},
"fallback": {
"steps": [
{
"args": "install --node-modules-dir=auto npm:@denotest/extra-info",
"output": "[WILDCARD]"
},
{
"args": "clean",
"output": "[WILDCARD]"
},
{
"args": [
"eval",
"Deno.removeSync(Deno.cwd() + '/node_modules/.deno/@denotest+extra-info@1.0.0/node_modules/@denotest/extra-info/package.json')"
],
"output": "[WILDCARD]"
},
{
"args": "install --node-modules-dir=auto --allow-scripts",
"output": "[WILDCARD]running 'postinstall' script\n"
}
]
}
}
}