fix(npm): support cjs resolution of package subpath with package.json (#15855)

cli/node/mod.rs

@@ -9,6 +9,7 @@ use crate::deno_std::CURRENT_STD_URL;
 use deno_ast::CjsAnalysis;
 use deno_ast::MediaType;
 use deno_ast::ModuleSpecifier;
+use deno_core::anyhow::anyhow;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::generic_error;
@@ -534,9 +535,7 @@ fn package_config_resolve(
   referrer_kind: NodeModuleKind,
 ) -> Result<PathBuf, AnyError> {
   let package_json_path = package_dir.join("package.json");
-  let referrer =
+  let referrer = ModuleSpecifier::from_directory_path(package_dir).unwrap();
-    ModuleSpecifier::from_directory_path(package_json_path.parent().unwrap())
-      .unwrap();
   let package_config =
     PackageJson::load(npm_resolver, package_json_path.clone())?;
   if let Some(exports) = &package_config.exports {
@@ -770,7 +769,16 @@ pub fn translate_cjs_to_esm(
     let reexport_specifier =
       ModuleSpecifier::from_file_path(&resolved_reexport).unwrap();
     // Second, read the source code from disk
-    let reexport_file = file_fetcher.get_source(&reexport_specifier).unwrap();
+    let reexport_file = file_fetcher
+      .get_source(&reexport_specifier)
+      .ok_or_else(|| {
+        anyhow!(
+          "Could not find '{}' ({}) referenced from {}",
+          reexport,
+          reexport_specifier,
+          referrer
+        )
+      })?;
 
     {
       let analysis = perform_cjs_analysis(
@@ -873,11 +881,21 @@ fn resolve(
       let d = module_dir.join(package_subpath);
       if let Ok(m) = d.metadata() {
         if m.is_dir() {
+          // subdir might have a package.json that specifies the entrypoint
+          let package_json_path = d.join("package.json");
+          if package_json_path.exists() {
+            let package_json =
+              PackageJson::load(npm_resolver, package_json_path)?;
+            if let Some(main) = package_json.main(NodeModuleKind::Cjs) {
+              return Ok(d.join(main).clean());
+            }
+          }
+
           return Ok(d.join("index.js").clean());
         }
       }
       return file_extension_probe(d, &referrer_path);
-    } else if let Some(main) = package_json.main {
+    } else if let Some(main) = package_json.main(NodeModuleKind::Cjs) {
       return Ok(module_dir.join(main).clean());
     } else {
       return Ok(module_dir.join("index.js").clean());
@@ -895,7 +913,7 @@ fn parse_specifier(specifier: &str) -> Option<(String, String)> {
   } else if specifier.starts_with('@') {
     // is_scoped = true;
     if let Some(index) = separator_index {
-      separator_index = specifier[index + 1..].find('/');
+      separator_index = specifier[index + 1..].find('/').map(|i| i + index + 1);
     } else {
       valid_package_name = false;
     }
@@ -1027,4 +1045,12 @@ mod tests {
       ]
     )
   }
+
+  #[test]
+  fn test_parse_specifier() {
+    assert_eq!(
+      parse_specifier("@some-package/core/actions"),
+      Some(("@some-package/core".to_string(), "./actions".to_string()))
+    );
+  }
 }

cli/tests/testdata/npm/dual_cjs_esm/main.out

@@ -1 +1,3 @@
 esm
+cjs
+cjs

cli/tests/testdata/npm/dual_cjs_esm/main.ts

@@ -1,3 +1,6 @@
 import { getKind } from "npm:@denotest/dual-cjs-esm";
+import * as cjs from "npm:@denotest/dual-cjs-esm/cjs/main.cjs";
 
 console.log(getKind());
+console.log(cjs.getKind());
+console.log(cjs.getSubPathKind());

cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/cjs/main.cjs

@@ -0,0 +1,10 @@
+const root = require("../");
+const subPath = require("../subpath");
+
+module.exports.getKind = function() {
+  return root.getKind();
+};
+
+module.exports.getSubPathKind = function() {
+  return subPath.getSubPathKind();
+};

cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/cjs/package.json

@@ -0,0 +1,3 @@
+{
+  "main": "./main.cjs"
+}

cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/package.json

@@ -2,6 +2,6 @@
   "name": "@denotest/dual-cjs-esm",
   "version": "1.0.0",
   "type": "module",
-  "main": "./index.cjs",
+  "main": "./main.cjs",
-  "module": "./index.mjs"
+  "module": "./main.mjs"
 }

cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/subpath/main.cjs

@@ -0,0 +1,3 @@
+exports.getSubPathKind = function() {
+  return "cjs";
+};

cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/subpath/main.mjs

@@ -0,0 +1,3 @@
+export function getSubPathKind() {
+  return "esm";
+}

cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/subpath/package.json

@@ -0,0 +1,5 @@
+{
+  "type": "module",
+  "main": "./main.cjs",
+  "module": "./main.mjs"
+}

ext/node/02_require.js

@@ -98,7 +98,11 @@
   }
 
   function tryPackage(requestPath, exts, isMain, originalPath) {
-    const pkg = core.ops.op_require_read_package_scope(requestPath).main;
+    const packageJsonPath = pathResolve(
+      requestPath,
+      "package.json",
+    );
+    const pkg = core.ops.op_require_read_package_scope(packageJsonPath).main;
     if (!pkg) {
       return tryExtensions(
         pathResolve(requestPath, "index"),
@@ -135,12 +139,8 @@
         err.requestPath = originalPath;
         throw err;
       } else {
-        const jsonPath = pathResolve(
-          requestPath,
-          "package.json",
-        );
         node.globalThis.process.emitWarning(
-          `Invalid 'main' field in '${jsonPath}' of '${pkg}'. ` +
+          `Invalid 'main' field in '${packageJsonPath}' of '${pkg}'. ` +
             "Please either fix that or report it to the module author",
           "DeprecationWarning",
           "DEP0128",

ext/node/lib.rs

@@ -604,15 +604,12 @@ where
 #[op]
 fn op_require_read_package_scope(
   state: &mut OpState,
-  filename: String,
+  package_json_path: String,
 ) -> Option<PackageJson> {
   check_unstable(state);
   let resolver = state.borrow::<Rc<dyn DenoDirNpmResolver>>().clone();
-  resolution::get_package_scope_config(
+  let package_json_path = PathBuf::from(package_json_path);
-    &Url::from_file_path(filename).unwrap(),
+  PackageJson::load(&*resolver, package_json_path).ok()
-    &*resolver,
-  )
-  .ok()
 }
 
 #[op]

ext/node/package_json.rs

@@ -1,5 +1,7 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
+use crate::NodeModuleKind;
+
 use super::DenoDirNpmResolver;
 use deno_core::anyhow;
 use deno_core::anyhow::bail;
@@ -17,8 +19,8 @@ pub struct PackageJson {
   pub exports: Option<Map<String, Value>>,
   pub imports: Option<Map<String, Value>>,
   pub bin: Option<Value>,
-  pub main: Option<String>,
+  main: Option<String>,   // use .main(...)
-  pub module: Option<String>,
+  module: Option<String>, // use .main(...)
   pub name: Option<String>,
   pub path: PathBuf,
   pub typ: String,
@@ -123,6 +125,14 @@ impl PackageJson {
     };
     Ok(package_json)
   }
+
+  pub fn main(&self, referrer_kind: NodeModuleKind) -> Option<&String> {
+    if referrer_kind == NodeModuleKind::Esm && self.typ == "module" {
+      self.module.as_ref().or(self.main.as_ref())
+    } else {
+      self.main.as_ref()
+    }
+  }
 }
 
 fn is_conditional_exports_main_sugar(exports: &Value) -> bool {

ext/node/resolution.rs

@@ -707,12 +707,7 @@ pub fn legacy_main_resolve(
   package_json: &PackageJson,
   referrer_kind: NodeModuleKind,
 ) -> Result<PathBuf, AnyError> {
-  let maybe_main =
+  let maybe_main = package_json.main(referrer_kind);
-    if referrer_kind == NodeModuleKind::Esm && package_json.typ == "module" {
-      package_json.module.as_ref().or(package_json.main.as_ref())
-    } else {
-      package_json.main.as_ref()
-    };
   let mut guess;
 
   if let Some(main) = maybe_main {