Repository: denoland/deno
Pull request: fix(install): read extra package info from node_modules and fallback to registry

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.

Changed files:
filename: cli/npm/installer/common/lifecycle_scripts.rs
patch: @@ -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

filename: cli/npm/installer/common/mod.rs
patch: @@ -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,15 +155,27 @@ 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?;
-      // 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
+      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())
+            || (expected.scripts && extra_info.scripts.is_empty())
+          {
+            self.fetch_from_registry(package_nv).await?
+          } 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

filename: cli/npm/installer/local.rs
patch: @@ -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

filename: tests/registry/npm/@denotest/extra-info/1.0.0/package.json
patch: @@ -0,0 +1,10 @@
+{
+  "name": "@denotest/extra-info",
+  "version": "1.0.0",
+  "bin": {
+    "denotest-extra-info": "./bin/main.js"
+  },
+  "scripts": {
+    "postinstall": "echo 'postinstall'"
+  }
+}

filename: tests/specs/install/package_extra_nmd/__test__.jsonc
patch: @@ -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"
+        }
+      ]
+    }
+  }
+}