refactor(npm): reorganize remapping built-in Node modules to remote URLs (#15755)

Changes how built-in Node modules are mapped to polyfills
from "deno_std". Instead of intertwining this logic into Node
resolution logic, we map them to "NodeResolution::BuiltIn"
which are remapped to "deno_std" URLs in ProcState.

cli/node/mod.rs

@@ -15,7 +15,6 @@ use deno_core::located_script_name;
 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::errors;
 use deno_runtime::deno_node::get_closest_package_json;
 use deno_runtime::deno_node::legacy_main_resolve;
@@ -40,6 +39,22 @@ mod analyze;
 
 pub use analyze::esm_code_with_node_globals;
 
+pub enum NodeResolution {
+  Esm(ModuleSpecifier),
+  CommonJs(ModuleSpecifier),
+  BuiltIn(String),
+}
+
+impl NodeResolution {
+  pub fn into_url(self) -> ModuleSpecifier {
+    match self {
+      Self::Esm(u) => u,
+      Self::CommonJs(u) => u,
+      _ => unreachable!(),
+    }
+  }
+}
+
 struct NodeModulePolyfill {
   /// Name of the module like "assert" or "timers/promises"
   name: &'static str,
@@ -237,15 +252,24 @@ static NODE_COMPAT_URL: Lazy<Url> = Lazy::new(|| {
 pub static MODULE_ALL_URL: Lazy<Url> =
   Lazy::new(|| NODE_COMPAT_URL.join("node/module_all.ts").unwrap());
 
-fn try_resolve_builtin_module(specifier: &str) -> Option<Url> {
-  for module in SUPPORTED_MODULES {
-    if module.name == specifier {
-      let module_url = NODE_COMPAT_URL.join(module.specifier).unwrap();
-      return Some(module_url);
-    }
+fn find_builtin_node_module(specifier: &str) -> Option<&NodeModulePolyfill> {
+  SUPPORTED_MODULES.iter().find(|m| m.name == specifier)
+}
+
+fn is_builtin_node_module(specifier: &str) -> bool {
+  find_builtin_node_module(specifier).is_some()
+}
+
+pub fn resolve_builtin_node_module(specifier: &str) -> Result<Url, AnyError> {
+  if let Some(module) = find_builtin_node_module(specifier) {
+    let module_url = NODE_COMPAT_URL.join(module.specifier).unwrap();
+    return Ok(module_url);
   }
 
-  None
+  Err(generic_error(format!(
+    "Unknown built-in Node module: {}",
+    specifier
+  )))
 }
 
 static RESERVED_WORDS: Lazy<HashSet<&str>> = Lazy::new(|| {
@@ -342,24 +366,17 @@ pub fn node_resolve(
   specifier: &str,
   referrer: &ModuleSpecifier,
   npm_resolver: &dyn DenoDirNpmResolver,
-) -> Result<Option<ResolveResponse>, AnyError> {
+) -> Result<Option<NodeResolution>, AnyError> {
   // Note: if we are here, then the referrer is an esm module
   // 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) = try_resolve_builtin_module(specifier) {
-    return Ok(Some(ResolveResponse::Esm(resolved)));
+  if is_builtin_node_module(specifier) {
+    return Ok(Some(NodeResolution::BuiltIn(specifier.to_string())));
   }
 
   if let Ok(url) = Url::parse(specifier) {
     if url.scheme() == "data" {
-      return Ok(Some(ResolveResponse::Specifier(url)));
+      return Ok(Some(NodeResolution::Esm(url)));
     }
 
     let protocol = url.scheme();
@@ -368,18 +385,8 @@ pub fn node_resolve(
       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) = try_resolve_builtin_module(&specifier) {
-        return Ok(Some(ResolveResponse::Esm(resolved)));
-      } else {
-        return Err(generic_error(format!("Unknown module {}", specifier)));
+      if is_builtin_node_module(&specifier) {
+        return Ok(Some(NodeResolution::BuiltIn(specifier)));
       }
     }
 
@@ -390,7 +397,7 @@ pub fn node_resolve(
     // todo(dsherret): this seems wrong
     if referrer.scheme() == "data" {
       let url = referrer.join(specifier).map_err(AnyError::from)?;
-      return Ok(Some(ResolveResponse::Specifier(url)));
+      return Ok(Some(NodeResolution::Esm(url)));
     }
   }
 
@@ -401,7 +408,7 @@ pub fn node_resolve(
     None => return Ok(None),
   };
 
-  let resolve_response = url_to_resolve_response(url, npm_resolver)?;
+  let resolve_response = url_to_node_resolution(url, npm_resolver)?;
   // TODO(bartlomieju): skipped checking errors for commonJS resolution and
   // "preserveSymlinksMain"/"preserveSymlinks" options.
   Ok(Some(resolve_response))
@@ -410,7 +417,7 @@ pub fn node_resolve(
 pub fn node_resolve_npm_reference(
   reference: &NpmPackageReference,
   npm_resolver: &GlobalNpmPackageResolver,
-) -> Result<Option<ResolveResponse>, AnyError> {
+) -> Result<Option<NodeResolution>, AnyError> {
   let package_folder = npm_resolver
     .resolve_package_from_deno_module(&reference.req)?
     .folder_path;
@@ -429,7 +436,7 @@ pub fn node_resolve_npm_reference(
   })?;
 
   let url = ModuleSpecifier::from_file_path(resolved_path).unwrap();
-  let resolve_response = url_to_resolve_response(url, npm_resolver)?;
+  let resolve_response = url_to_node_resolution(url, npm_resolver)?;
   // TODO(bartlomieju): skipped checking errors for commonJS resolution and
   // "preserveSymlinksMain"/"preserveSymlinks" options.
   Ok(Some(resolve_response))
@@ -439,7 +446,7 @@ pub fn node_resolve_binary_export(
   pkg_req: &NpmPackageReq,
   bin_name: Option<&str>,
   npm_resolver: &GlobalNpmPackageResolver,
-) -> Result<ResolveResponse, AnyError> {
+) -> Result<NodeResolution, AnyError> {
   let pkg = npm_resolver.resolve_package_from_deno_module(pkg_req)?;
   let package_folder = pkg.folder_path;
   let package_json_path = package_folder.join("package.json");
@@ -489,7 +496,7 @@ pub fn node_resolve_binary_export(
   let url =
     ModuleSpecifier::from_file_path(package_folder.join(bin_entry)).unwrap();
 
-  let resolve_response = url_to_resolve_response(url, npm_resolver)?;
+  let resolve_response = url_to_node_resolution(url, npm_resolver)?;
   // TODO(bartlomieju): skipped checking errors for commonJS resolution and
   // "preserveSymlinksMain"/"preserveSymlinks" options.
   Ok(resolve_response)
@@ -546,23 +553,23 @@ fn package_config_resolve(
   Ok(package_dir.join(package_subpath))
 }
 
-fn url_to_resolve_response(
+fn url_to_node_resolution(
   url: ModuleSpecifier,
   npm_resolver: &dyn DenoDirNpmResolver,
-) -> Result<ResolveResponse, AnyError> {
+) -> Result<NodeResolution, AnyError> {
   Ok(if url.as_str().starts_with("http") {
-    ResolveResponse::Esm(url)
+    NodeResolution::Esm(url)
   } else if url.as_str().ends_with(".js") {
     let package_config = get_closest_package_json(&url, npm_resolver)?;
     if package_config.typ == "module" {
-      ResolveResponse::Esm(url)
+      NodeResolution::Esm(url)
     } else {
-      ResolveResponse::CommonJs(url)
+      NodeResolution::CommonJs(url)
     }
   } else if url.as_str().ends_with(".cjs") {
-    ResolveResponse::CommonJs(url)
+    NodeResolution::CommonJs(url)
   } else {
-    ResolveResponse::Esm(url)
+    NodeResolution::Esm(url)
   })
 }
 
@@ -570,16 +577,6 @@ fn finalize_resolution(
   resolved: ModuleSpecifier,
   base: &ModuleSpecifier,
 ) -> Result<ModuleSpecifier, AnyError> {
-  // TODO(bartlomieju): this is not part of Node resolution algorithm
-  // (as it doesn't support http/https); but I had to short circuit here
-  // for remote modules because they are mainly used to polyfill `node` built
-  // in modules. Another option would be to leave the resolved URLs
-  // as `node:<module_name>` and do the actual remapping to std's polyfill
-  // in module loader. I'm not sure which approach is better.
-  if resolved.scheme().starts_with("http") {
-    return Ok(resolved);
-  }
-
   // todo(dsherret): cache
   let encoded_sep_re = Regex::new(r"%2F|%2C").unwrap();
 

cli/proc_state.rs

@@ -21,6 +21,7 @@ use crate::http_cache;
 use crate::lockfile::as_maybe_locker;
 use crate::lockfile::Lockfile;
 use crate::node;
+use crate::node::NodeResolution;
 use crate::npm::GlobalNpmPackageResolver;
 use crate::npm::NpmPackageReference;
 use crate::npm::NpmPackageResolver;
@@ -461,17 +462,19 @@ impl ProcState {
 
   fn handle_node_resolve_result(
     &self,
-    result: Result<Option<ResolveResponse>, AnyError>,
+    result: Result<Option<node::NodeResolution>, AnyError>,
   ) -> Result<ModuleSpecifier, AnyError> {
     let response = match result? {
       Some(response) => response,
       None => bail!("Not found."),
     };
-    if let ResolveResponse::CommonJs(specifier) = &response {
+    if let NodeResolution::CommonJs(specifier) = &response {
       // remember that this was a common js resolution
       self.cjs_resolutions.lock().insert(specifier.clone());
+    } else if let NodeResolution::BuiltIn(specifier) = &response {
+      return node::resolve_builtin_node_module(specifier);
     }
-    response.to_result()
+    Ok(response.into_url())
   }
 
   pub fn resolve(

cli/worker.rs

@@ -9,7 +9,6 @@ use deno_core::futures::FutureExt;
 use deno_core::located_script_name;
 use deno_core::Extension;
 use deno_core::ModuleId;
-use deno_graph::source::ResolveResponse;
 use deno_runtime::colors;
 use deno_runtime::fmt_errors::format_js_error;
 use deno_runtime::ops::worker_host::CreateWebWorkerCb;
@@ -355,13 +354,14 @@ pub async fn create_main_worker(
       .await?;
     ps.npm_resolver.cache_packages().await?;
     ps.prepare_node_std_graph().await?;
-    let resolve_response = node::node_resolve_binary_export(
+    let node_resolution = node::node_resolve_binary_export(
       &package_ref.req,
       package_ref.sub_path.as_deref(),
       &ps.npm_resolver,
     )?;
-    let is_main_cjs = matches!(resolve_response, ResolveResponse::CommonJs(_));
-    (resolve_response.to_result()?, is_main_cjs)
+    let is_main_cjs =
+      matches!(node_resolution, node::NodeResolution::CommonJs(_));
+    (node_resolution.into_url(), is_main_cjs)
   } else {
     (main_module, false)
   };