fix(node): use closest package.json to resolve package.json imports (#21075)

5 files changed, 34 insertions, 52 deletions

diff --git a/ext/node/errors.rs b/ext/node/errors.rs
index e72cb8557..e07a8347a 100644
--- a/ext/node/errors.rs
+++ b/ext/node/errors.rs
@@ -138,11 +138,12 @@ pub fn err_package_import_not_defined(
   base: &str,
 ) -> AnyError {
   let mut msg = format!(
-    "[ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier \"{specifier}\" is not defined in"
+    "[ERR_PACKAGE_IMPORT_NOT_DEFINED] Package import specifier \"{specifier}\" is not defined"
   );
 
   if let Some(package_path) = package_path {
-    msg = format!("{msg} in package {package_path}package.json");
+    let pkg_json_path = PathBuf::from(package_path).join("package.json");
+    msg = format!("{} in package {}", msg, pkg_json_path.display());
   }
 
   msg = format!("{msg} imported from {base}");
diff --git a/ext/node/lib.rs b/ext/node/lib.rs
index 730554f2d..7a43fc4d4 100644
--- a/ext/node/lib.rs
+++ b/ext/node/lib.rs
@@ -81,12 +81,6 @@ pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync {
     mode: NodeResolutionMode,
   ) -> Result<PathBuf, AnyError>;
 
-  /// Resolves the npm package folder path from the specified path.
-  fn resolve_package_folder_from_path(
-    &self,
-    specifier: &ModuleSpecifier,
-  ) -> Result<Option<PathBuf>, AnyError>;
-
   fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool;
 
   fn in_npm_package_at_path(&self, path: &Path) -> bool {
diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs
index 2cf59b0a3..ceb771bf9 100644
--- a/ext/node/ops/require.rs
+++ b/ext/node/ops/require.rs
@@ -389,7 +389,7 @@ where
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
   let permissions = state.borrow::<P>();
   let pkg = node_resolver
-    .get_package_scope_config(
+    .get_closest_package_json(
       &Url::from_file_path(parent_path.unwrap()).unwrap(),
       permissions,
     )
diff --git a/ext/node/polyfills/internal/errors.ts b/ext/node/polyfills/internal/errors.ts
index 2ba7ec28e..7f7ce9d3a 100644
--- a/ext/node/polyfills/internal/errors.ts
+++ b/ext/node/polyfills/internal/errors.ts
@@ -2421,7 +2421,7 @@ export class ERR_INVALID_PACKAGE_TARGET extends NodeError {
     if (key === ".") {
       assert(isImport === false);
       msg = `Invalid "exports" main target ${JSON.stringify(target)} defined ` +
-        `in the package config ${pkgPath}package.json${
+        `in the package config ${displayJoin(pkgPath, "package.json")}${
           base ? ` imported from ${base}` : ""
         }${relError ? '; targets must start with "./"' : ""}`;
     } else {
@@ -2429,9 +2429,11 @@ export class ERR_INVALID_PACKAGE_TARGET extends NodeError {
         JSON.stringify(
           target,
         )
-      } defined for '${key}' in the package config ${pkgPath}package.json${
-        base ? ` imported from ${base}` : ""
-      }${relError ? '; targets must start with "./"' : ""}`;
+      } defined for '${key}' in the package config ${
+        displayJoin(pkgPath, "package.json")
+      }${base ? ` imported from ${base}` : ""}${
+        relError ? '; targets must start with "./"' : ""
+      }`;
     }
     super("ERR_INVALID_PACKAGE_TARGET", msg);
   }
@@ -2444,7 +2446,9 @@ export class ERR_PACKAGE_IMPORT_NOT_DEFINED extends NodeTypeError {
     base: string,
   ) {
     const msg = `Package import specifier "${specifier}" is not defined${
-      packagePath ? ` in package ${packagePath}package.json` : ""
+      packagePath
+        ? ` in package ${displayJoin(packagePath, "package.json")}`
+        : ""
     } imported from ${base}`;
 
     super("ERR_PACKAGE_IMPORT_NOT_DEFINED", msg);
@@ -2455,14 +2459,13 @@ export class ERR_PACKAGE_PATH_NOT_EXPORTED extends NodeError {
   constructor(subpath: string, pkgPath: string, basePath?: string) {
     let msg: string;
     if (subpath === ".") {
-      msg = `No "exports" main defined in ${pkgPath}package.json${
-        basePath ? ` imported from ${basePath}` : ""
-      }`;
+      msg = `No "exports" main defined in ${
+        displayJoin(pkgPath, "package.json")
+      }${basePath ? ` imported from ${basePath}` : ""}`;
     } else {
-      msg =
-        `Package subpath '${subpath}' is not defined by "exports" in ${pkgPath}package.json${
-          basePath ? ` imported from ${basePath}` : ""
-        }`;
+      msg = `Package subpath '${subpath}' is not defined by "exports" in ${
+        displayJoin(pkgPath, "package.json")
+      }${basePath ? ` imported from ${basePath}` : ""}`;
     }
 
     super("ERR_PACKAGE_PATH_NOT_EXPORTED", msg);
@@ -2621,6 +2624,12 @@ function determineSpecificType(value: any) {
   return `type ${typeof value} (${inspected})`;
 }
 
+// Non-robust path join
+function displayJoin(dir: string, fileName: string) {
+  const sep = dir.includes("\\") ? "\\" : "/";
+  return dir.endsWith(sep) ? dir + fileName : dir + sep + fileName;
+}
+
 export { codes, genericNodeError, hideStackFrames };
 
 export default {
diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs
index cb4fef405..65c2e1bcd 100644
--- a/ext/node/resolution.rs
+++ b/ext/node/resolution.rs
@@ -543,7 +543,7 @@ impl NodeResolver {
 
     let mut package_json_path = None;
     if let Some(package_config) =
-      self.get_package_scope_config(referrer, permissions)?
+      self.get_closest_package_json(referrer, permissions)?
     {
       if package_config.exists {
         package_json_path = Some(package_config.path.clone());
@@ -977,12 +977,12 @@ impl NodeResolver {
     let (package_name, package_subpath, _is_scoped) =
       parse_npm_pkg_name(specifier, referrer)?;
 
-    // ResolveSelf
     let Some(package_config) =
-      self.get_package_scope_config(referrer, permissions)?
+      self.get_closest_package_json(referrer, permissions)?
     else {
       return Ok(None);
     };
+    // ResolveSelf
     if package_config.exists
       && package_config.name.as_ref() == Some(&package_name)
     {
@@ -1086,24 +1086,7 @@ impl NodeResolver {
     }
   }
 
-  pub(super) fn get_package_scope_config(
-    &self,
-    referrer: &ModuleSpecifier,
-    permissions: &dyn NodePermissions,
-  ) -> Result<Option<PackageJson>, AnyError> {
-    let Some(root_folder) = self
-      .npm_resolver
-      .resolve_package_folder_from_path(referrer)?
-    else {
-      return Ok(None);
-    };
-    let package_json_path = root_folder.join("package.json");
-    self
-      .load_package_json(permissions, package_json_path)
-      .map(Some)
-  }
-
-  pub(super) fn get_closest_package_json(
+  pub fn get_closest_package_json(
     &self,
     url: &ModuleSpecifier,
     permissions: &dyn NodePermissions,
@@ -1121,7 +1104,9 @@ impl NodeResolver {
     &self,
     url: &ModuleSpecifier,
   ) -> Result<Option<PathBuf>, AnyError> {
-    let file_path = url.to_file_path().unwrap();
+    let Ok(file_path) = url.to_file_path() else {
+      return Ok(None);
+    };
     let current_dir = deno_core::strip_unc_prefix(
       self.fs.realpath_sync(file_path.parent().unwrap())?,
     );
@@ -1130,15 +1115,8 @@ impl NodeResolver {
     if self.fs.exists_sync(&package_json_path) {
       return Ok(Some(package_json_path));
     }
-    let Some(root_pkg_folder) =
-      self.npm_resolver.resolve_package_folder_from_path(
-        &ModuleSpecifier::from_directory_path(current_dir).unwrap(),
-      )?
-    else {
-      return Ok(None);
-    };
-    while current_dir.starts_with(&root_pkg_folder) {
-      current_dir = current_dir.parent().unwrap();
+    while let Some(parent) = current_dir.parent() {
+      current_dir = parent;
       let package_json_path = current_dir.join("package.json");
       if self.fs.exists_sync(&package_json_path) {
         return Ok(Some(package_json_path));