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

…15546)

Co-authored-by: David Sherret <[email protected]>

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) {

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,

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,

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),
  }

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

@@ -3,3 +3,4 @@ Download http:https://localhost:4545/npm/registry/chalk
 Download http:https://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz
 B
 C
+devDependency import failed: TypeError: Relative import path "xo"[WILDCARD]

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);
+}

ext/node/02_require.js

@@ -40,6 +40,9 @@
  const cjsParseCache = new SafeWeakMap();
 
  function pathDirname(filepath) {
+ if (filepath == null || filepath === "") {
+ throw new Error("Empty filepath.");
+ }
  return ops.op_require_path_dirname(filepath);
  }
 
@@ -470,6 +473,7 @@
 
  if (isMain) {
  node.globalThis.process.mainModule = module;
+ mainModule = module;
  module.id = ".";
  }
 
@@ -884,6 +888,7 @@
  cjsParseCache,
  readPackageScope,
  bindExport,
+ moduleExports: m,
  },
  };
 })(globalThis);

ext/node/lib.rs

@@ -40,6 +40,8 @@ mod errors;
 mod package_json;
 mod resolution;
 
+pub const MODULE_ES_SHIM: &str = include_str!("./module_es_shim.js");
+
 struct Unstable(pub bool);
 
 pub fn init(

ext/node/module_es_shim.js

@@ -0,0 +1,17 @@
+const m = Deno[Deno.internal].require.moduleExports;
+export const _cache = m._cache;
+export const _extensions = m._extensions;
+export const _findPath = m._findPath;
+export const _initPaths = m._initPaths;
+export const _load = m._load;
+export const _nodeModulePaths = m._nodeModulePaths;
+export const _pathCache = m._pathCache;
+export const _preloadModules = m._preloadModules;
+export const _resolveFilename = m._resolveFilename;
+export const _resolveLookupPaths = m._resolveLookupPaths;
+export const builtinModules = m.builtinModules;
+export const createRequire = m.createRequire;
+export const globalPaths = m.globalPaths;
+export const Module = m.Module;
+export const wrap = m.wrap;
+export default m;

ext/node/resolution.rs

@@ -587,12 +587,10 @@ pub fn package_resolve(
  }
  }
 
- let package_dir_path = npm_resolver
- .resolve_package_folder_from_package(
- &package_name,
- &referrer.to_file_path().unwrap(),
- )
- .unwrap();
+ let package_dir_path = npm_resolver.resolve_package_folder_from_package(
+ &package_name,
+ &referrer.to_file_path().unwrap(),
+ )?;
  let package_json_path = package_dir_path.join("package.json");
  let package_json_url =
  ModuleSpecifier::from_file_path(&package_json_path).unwrap();