fix(unstable): various resolution bugs for npm: specifiers (#15546)

Co-authored-by: David Sherret <dsherret@gmail.com>

6 files changed, 94 insertions, 12 deletions

diff --git a/cli/module_loader.rs b/cli/module_loader.rs
index 05dd5bd73..5d567f8c5 100644
--- a/cli/module_loader.rs
+++ b/cli/module_loader.rs
@@ -63,6 +63,13 @@ impl CliModuleLoader {
     &self,
     specifier: &ModuleSpecifier,
   ) -> Result<ModuleCodeSource, AnyError> {
+    if specifier.as_str() == "node:module" {
+      return Ok(ModuleCodeSource {
+        code: deno_runtime::deno_node::MODULE_ES_SHIM.to_string(),
+        found_url: specifier.to_owned(),
+        media_type: MediaType::JavaScript,
+      });
+    }
     let graph_data = self.ps.graph_data.read();
     let found_url = graph_data.follow_redirect(specifier);
     match graph_data.get(&found_url) {
diff --git a/cli/node/mod.rs b/cli/node/mod.rs
index e9bf04d60..21d642fa9 100644
--- a/cli/node/mod.rs
+++ b/cli/node/mod.rs
@@ -16,7 +16,6 @@ use deno_core::serde_json::Value;
 use deno_core::url::Url;
 use deno_core::JsRuntime;
 use deno_graph::source::ResolveResponse;
-use deno_runtime::deno_node::get_package_scope_config;
 use deno_runtime::deno_node::legacy_main_resolve;
 use deno_runtime::deno_node::package_exports_resolve;
 use deno_runtime::deno_node::package_imports_resolve;
@@ -136,6 +135,13 @@ pub fn node_resolve(
 ) -> Result<Option<ResolveResponse>, AnyError> {
   // TODO(bartlomieju): skipped "policy" part as we don't plan to support it
 
+  // NOTE(bartlomieju): this will force `ProcState` to use Node.js polyfill for
+  // `module` from `ext/node/`.
+  if specifier == "module" {
+    return Ok(Some(ResolveResponse::Esm(
+      Url::parse("node:module").unwrap(),
+    )));
+  }
   if let Some(resolved) = compat::try_resolve_builtin_module(specifier) {
     return Ok(Some(ResolveResponse::Esm(resolved)));
   }
@@ -150,6 +156,15 @@ pub fn node_resolve(
     if protocol == "node" {
       let split_specifier = url.as_str().split(':');
       let specifier = split_specifier.skip(1).collect::<String>();
+
+      // NOTE(bartlomieju): this will force `ProcState` to use Node.js polyfill for
+      // `module` from `ext/node/`.
+      if specifier == "module" {
+        return Ok(Some(ResolveResponse::Esm(
+          Url::parse("node:module").unwrap(),
+        )));
+      }
+
       if let Some(resolved) = compat::try_resolve_builtin_module(&specifier) {
         return Ok(Some(ResolveResponse::Esm(resolved)));
       } else {
@@ -329,7 +344,7 @@ fn url_to_resolve_response(
   Ok(if url.as_str().starts_with("http") {
     ResolveResponse::Esm(url)
   } else if url.as_str().ends_with(".js") {
-    let package_config = get_package_scope_config(&url, npm_resolver)?;
+    let package_config = get_closest_package_json(&url, npm_resolver)?;
     if package_config.typ == "module" {
       ResolveResponse::Esm(url)
     } else {
@@ -342,6 +357,37 @@ fn url_to_resolve_response(
   })
 }
 
+fn get_closest_package_json(
+  url: &ModuleSpecifier,
+  npm_resolver: &dyn DenoDirNpmResolver,
+) -> Result<PackageJson, AnyError> {
+  let package_json_path = get_closest_package_json_path(url, npm_resolver)?;
+  PackageJson::load(npm_resolver, package_json_path)
+}
+
+fn get_closest_package_json_path(
+  url: &ModuleSpecifier,
+  npm_resolver: &dyn DenoDirNpmResolver,
+) -> Result<PathBuf, AnyError> {
+  let file_path = url.to_file_path().unwrap();
+  let mut current_dir = file_path.parent().unwrap();
+  let package_json_path = current_dir.join("package.json");
+  if package_json_path.exists() {
+    return Ok(package_json_path);
+  }
+  let root_folder = npm_resolver
+    .resolve_package_folder_from_path(&url.to_file_path().unwrap())?;
+  while current_dir.starts_with(&root_folder) {
+    current_dir = current_dir.parent().unwrap();
+    let package_json_path = current_dir.join("./package.json");
+    if package_json_path.exists() {
+      return Ok(package_json_path);
+    }
+  }
+
+  bail!("did not find package.json in {}", root_folder.display())
+}
+
 fn finalize_resolution(
   resolved: ModuleSpecifier,
   base: &ModuleSpecifier,
diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs
index 3b7dd4251..1604f4b11 100644
--- a/cli/npm/registry.rs
+++ b/cli/npm/registry.rs
@@ -27,7 +27,7 @@ use super::version_req::NpmVersionReq;
 
 // npm registry docs: https://github.com/npm/registry/blob/master/docs/REGISTRY-API.md
 
-#[derive(Deserialize, Serialize, Clone)]
+#[derive(Debug, Deserialize, Serialize, Clone)]
 pub struct NpmPackageInfo {
   pub name: String,
   pub versions: HashMap<String, NpmPackageVersionInfo>,
@@ -39,7 +39,7 @@ pub struct NpmDependencyEntry {
   pub version_req: NpmVersionReq,
 }
 
-#[derive(Deserialize, Serialize, Clone)]
+#[derive(Debug, Deserialize, Serialize, Clone)]
 pub struct NpmPackageVersionInfo {
   pub version: String,
   pub dist: NpmPackageVersionDistInfo,
diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs
index 3b5f0b89a..e102e2fa8 100644
--- a/cli/npm/resolution.rs
+++ b/cli/npm/resolution.rs
@@ -186,16 +186,37 @@ impl NpmResolutionSnapshot {
   ) -> Result<&NpmResolutionPackage, AnyError> {
     match self.packages.get(referrer) {
       Some(referrer_package) => {
-        match referrer_package.dependencies.get(name_without_path(name)) {
-          Some(id) => Ok(self.packages.get(id).unwrap()),
-          None => {
-            bail!(
-              "could not find npm package '{}' referenced by '{}'",
-              name,
-              referrer
-            )
+        let name_ = name_without_path(name);
+        if let Some(id) = referrer_package.dependencies.get(name_) {
+          return Ok(self.packages.get(id).unwrap());
+        }
+
+        if referrer_package.id.name == name_ {
+          return Ok(referrer_package);
+        }
+
+        // TODO(bartlomieju): this should use a reverse lookup table in the
+        // snapshot instead of resolving best version again.
+        let req = NpmPackageReq {
+          name: name_.to_string(),
+          version_req: None,
+        };
+
+        if let Some(version) = self.resolve_best_package_version(name_, &req) {
+          let id = NpmPackageId {
+            name: name_.to_string(),
+            version,
+          };
+          if let Some(pkg) = self.packages.get(&id) {
+            return Ok(pkg);
           }
         }
+
+        bail!(
+          "could not find npm package '{}' referenced by '{}'",
+          name,
+          referrer
+        )
       }
       None => bail!("could not find referrer npm package '{}'", referrer),
     }
diff --git a/cli/tests/testdata/npm/dynamic_import/main.out b/cli/tests/testdata/npm/dynamic_import/main.out
index 7e2fb7a0f..cefb3ad44 100644
--- a/cli/tests/testdata/npm/dynamic_import/main.out
+++ b/cli/tests/testdata/npm/dynamic_import/main.out
@@ -3,3 +3,4 @@ Download http://localhost:4545/npm/registry/chalk
 Download http://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz
 B
 C
+devDependency import failed: TypeError: Relative import path "xo"[WILDCARD]
\ No newline at end of file
diff --git a/cli/tests/testdata/npm/dynamic_import/other.ts b/cli/tests/testdata/npm/dynamic_import/other.ts
index e5d3b6dc3..008f8833e 100644
--- a/cli/tests/testdata/npm/dynamic_import/other.ts
+++ b/cli/tests/testdata/npm/dynamic_import/other.ts
@@ -2,3 +2,10 @@ console.log("B");
 const chalk = (await import("npm:chalk@5")).default;
 
 console.log(chalk.green("C"));
+
+try {
+  // Trying to import a devDependency should result in an error
+  await import("xo");
+} catch (e) {
+  console.error("devDependency import failed:", e);
+}