feat: wire up ext/node to the Node compatibility layer (denoland#17785)

This PR changes Node.js/npm compatibility layer to use polyfills for
built-in Node.js
embedded in the snapshot (that are coming from "ext/node" extension).

As a result loading `std/node`, either from
"https://deno.land/std@<latest>/" or
from "DENO_NODE_COMPAT_URL" env variable were removed. All code that is
imported via "npm:" specifiers now uses code embedded in the snapshot.

Several fixes were applied to various modules in "ext/node" to make
tests pass.

---------

Co-authored-by: Yoshiya Hinosawa <[email protected]>
Co-authored-by: Divy Srivastava <[email protected]>

cli/cache/mod.rs

@@ -114,18 +114,14 @@ impl Loader for FetchCacher {
 
     let specifier =
       if let Some(module_name) = specifier.as_str().strip_prefix("node:") {
-        if module_name == "module" {
-          // the source code for "node:module" is built-in rather than
-          // being from deno_std like the other modules
-          return Box::pin(futures::future::ready(Ok(Some(
-            deno_graph::source::LoadResponse::External {
-              specifier: specifier.clone(),
-            },
-          ))));
-        }
-
+        // Built-in Node modules are embedded in the Deno binary (in V8 snapshot)
+        // so we don't want them to be loaded by the "deno graph".
         match crate::node::resolve_builtin_node_module(module_name) {
-          Ok(specifier) => specifier,
+          Ok(specifier) => {
+            return Box::pin(futures::future::ready(Ok(Some(
+              deno_graph::source::LoadResponse::External { specifier },
+            ))))
+          }
           Err(err) => return Box::pin(futures::future::ready(Err(err))),
         }
       } else {

cli/module_loader.rs

@@ -77,11 +77,10 @@ impl CliModuleLoader {
     specifier: &ModuleSpecifier,
     maybe_referrer: Option<ModuleSpecifier>,
   ) -> Result<ModuleCodeSource, AnyError> {
-    // TODO(bartlomieju): uncomment, when all `node:` module have been
-    // snapshotted
-    // if specifier.scheme() == "node" {
-    //   unreachable!("Node built-in modules should be handled internally.");
-    // }
+    if specifier.scheme() == "node" {
+      unreachable!("Node built-in modules should be handled internally.");
+    }
+
     let graph = self.ps.graph();
     match graph.get(specifier) {
       Some(deno_graph::Module {

cli/node/mod.rs

@@ -27,7 +27,6 @@ use deno_runtime::deno_node::package_imports_resolve;
 use deno_runtime::deno_node::package_resolve;
 use deno_runtime::deno_node::path_to_declaration_path;
 use deno_runtime::deno_node::NodeModuleKind;
-use deno_runtime::deno_node::NodeModulePolyfillSpecifier;
 use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::PackageJson;
@@ -39,7 +38,6 @@ use once_cell::sync::Lazy;
 use regex::Regex;
 
 use crate::cache::NodeAnalysisCache;
-use crate::deno_std::CURRENT_STD_URL;
 use crate::file_fetcher::FileFetcher;
 use crate::npm::NpmPackageResolver;
 
@@ -106,33 +104,10 @@ impl NodeResolution {
   }
 }
 
-static NODE_COMPAT_URL: Lazy<Url> = Lazy::new(|| {
-  if let Ok(url_str) = std::env::var("DENO_NODE_COMPAT_URL") {
-    let url = Url::parse(&url_str).expect(
-      "Malformed DENO_NODE_COMPAT_URL value, make sure it's a file URL ending with a slash"
-    );
-    return url;
-  }
-
-  CURRENT_STD_URL.clone()
-});
-
-pub static MODULE_ALL_URL: Lazy<Url> =
-  Lazy::new(|| NODE_COMPAT_URL.join("node/module_all.ts").unwrap());
-
+// TODO(bartlomieju): seems super wasteful to parse specified each time
 pub fn resolve_builtin_node_module(specifier: &str) -> Result<Url, AnyError> {
   if let Some(module) = find_builtin_node_module(specifier) {
-    match module.specifier {
-      // We will load the source code from the `std/node` polyfill.
-      NodeModulePolyfillSpecifier::StdNode(specifier) => {
-        let module_url = NODE_COMPAT_URL.join(specifier).unwrap();
-        return Ok(module_url);
-      }
-      // The module has already been snapshotted and is present in the binary.
-      NodeModulePolyfillSpecifier::Embedded(specifier) => {
-        return Ok(ModuleSpecifier::parse(specifier).unwrap());
-      }
-    }
+    return Ok(ModuleSpecifier::parse(module.specifier).unwrap());
   }
 
   Err(generic_error(format!(

cli/proc_state.rs

@@ -63,8 +63,6 @@ use std::collections::HashMap;
 use std::collections::HashSet;
 use std::ops::Deref;
 use std::path::PathBuf;
-use std::sync::atomic::AtomicBool;
-use std::sync::atomic::Ordering;
 use std::sync::Arc;
 
 /// This structure represents state of single "deno" program.
@@ -98,7 +96,6 @@ pub struct Inner {
   pub npm_resolver: NpmPackageResolver,
   pub cjs_resolutions: Mutex<HashSet<ModuleSpecifier>>,
   progress_bar: ProgressBar,
-  node_std_graph_prepared: AtomicBool,
 }
 
 impl Deref for ProcState {
@@ -160,7 +157,6 @@ impl ProcState {
       npm_resolver: self.npm_resolver.clone(),
       cjs_resolutions: Default::default(),
       progress_bar: self.progress_bar.clone(),
-      node_std_graph_prepared: AtomicBool::new(false),
     });
     self.init_watcher();
   }
@@ -293,7 +289,6 @@ impl ProcState {
       npm_resolver,
       cjs_resolutions: Default::default(),
       progress_bar,
-      node_std_graph_prepared: AtomicBool::new(false),
     })))
   }
 
@@ -369,7 +364,6 @@ impl ProcState {
 
     if !npm_package_reqs.is_empty() {
       self.npm_resolver.add_package_reqs(npm_package_reqs).await?;
-      self.prepare_node_std_graph().await?;
     }
 
     if has_node_builtin_specifier
@@ -384,9 +378,7 @@ impl ProcState {
     drop(_pb_clear_guard);
 
     // type check if necessary
-    let is_std_node = roots.len() == 1 && roots[0] == *node::MODULE_ALL_URL;
     if self.options.type_check_mode() != TypeCheckMode::None
-      && !is_std_node
       && !self.graph_data.read().is_type_checked(&roots, lib)
     {
       log::debug!("Type checking.");
@@ -456,31 +448,6 @@ impl ProcState {
       .await
   }
 
-  /// Add the builtin node modules to the graph data.
-  pub async fn prepare_node_std_graph(&self) -> Result<(), AnyError> {
-    if self.node_std_graph_prepared.load(Ordering::Relaxed) {
-      return Ok(());
-    }
-
-    let mut graph = self.graph_data.read().graph_inner_clone();
-    let mut loader = self.create_graph_loader();
-    let analyzer = self.parsed_source_cache.as_analyzer();
-    graph
-      .build(
-        vec![node::MODULE_ALL_URL.clone()],
-        &mut loader,
-        deno_graph::BuildOptions {
-          module_analyzer: Some(&*analyzer),
-          ..Default::default()
-        },
-      )
-      .await;
-
-    self.graph_data.write().update_graph(Arc::new(graph));
-    self.node_std_graph_prepared.store(true, Ordering::Relaxed);
-    Ok(())
-  }
-
   fn handle_node_resolve_result(
     &self,
     result: Result<Option<node::NodeResolution>, AnyError>,
@@ -712,6 +679,10 @@ impl ProcState {
   pub fn graph(&self) -> Arc<ModuleGraph> {
     self.graph_data.read().graph.clone()
   }
+
+  pub fn has_node_builtin_specifier(&self) -> bool {
+    self.graph_data.read().has_node_builtin_specifier
+  }
 }
 
 #[derive(Clone, Debug)]

cli/tests/integration/run_tests.rs

@@ -3830,14 +3830,14 @@ fn stdio_streams_are_locked_in_permission_prompt() {
 }
 
 itest!(node_builtin_modules_ts {
-  args: "run --quiet run/node_builtin_modules/mod.ts",
+  args: "run --quiet --allow-read run/node_builtin_modules/mod.ts hello there",
   output: "run/node_builtin_modules/mod.ts.out",
   envs: env_vars_for_npm_tests_no_sync_download(),
   exit_code: 0,
 });
 
 itest!(node_builtin_modules_js {
-  args: "run --quiet run/node_builtin_modules/mod.js",
+  args: "run --quiet --allow-read run/node_builtin_modules/mod.js hello there",
   output: "run/node_builtin_modules/mod.js.out",
   envs: env_vars_for_npm_tests_no_sync_download(),
   exit_code: 0,

cli/tests/testdata/run/node_builtin_modules/mod.js

@@ -2,3 +2,4 @@ import { createRequire } from "node:module";
 console.log(createRequire);
 import process from "node:process";
 console.log(process.version);
+console.log(process.argv);

cli/tests/testdata/run/node_builtin_modules/mod.js.out

@@ -1,2 +1,8 @@
 [Function: createRequire]
 v[WILDCARD].[WILDCARD].[WILDCARD]
+[
+  "[WILDCARD]",
+  "[WILDCARD]mod.js",
+  "hello",
+  "there"
+]

cli/tests/testdata/run/node_builtin_modules/mod.ts

@@ -2,3 +2,4 @@ import { createRequire } from "node:module";
 console.log(createRequire);
 import process from "node:process";
 console.log(process.version);
+console.log(process.argv);

cli/tests/testdata/run/node_builtin_modules/mod.ts.out

@@ -1,2 +1,8 @@
 [Function: createRequire]
 v[WILDCARD].[WILDCARD].[WILDCARD]
+[
+  "[WILDCARD]",
+  "[WILDCARD]mod.ts",
+  "hello",
+  "there"
+]

cli/tools/repl/session.rs

@@ -461,10 +461,8 @@ impl ReplSession {
       resolved_imports.iter().any(|url| url.scheme() == "node");
     if !npm_imports.is_empty() || has_node_specifier {
       if !self.has_initialized_node_runtime {
-        self.proc_state.prepare_node_std_graph().await?;
         deno_node::initialize_runtime(
           &mut self.worker.js_runtime,
-          crate::node::MODULE_ALL_URL.as_str(),
           self.proc_state.options.node_modules_dir(),
         )
         .await?;

cli/worker.rs

@@ -67,7 +67,6 @@ impl CliMainWorker {
     log::debug!("main_module {}", self.main_module);
 
     if self.is_main_cjs {
-      self.ps.prepare_node_std_graph().await?;
       self.initialize_main_module_for_node().await?;
       deno_node::load_cjs_module(
         &mut self.worker.js_runtime,
@@ -279,9 +278,6 @@ impl CliMainWorker {
   async fn execute_main_module_possibly_with_npm(
     &mut self,
   ) -> Result<(), AnyError> {
-    if self.ps.npm_resolver.has_packages() {
-      self.ps.prepare_node_std_graph().await?;
-    }
     let id = self.worker.preload_main_module(&self.main_module).await?;
     self.evaluate_module_possibly_with_npm(id).await
   }
@@ -297,17 +293,17 @@ impl CliMainWorker {
     &mut self,
     id: ModuleId,
   ) -> Result<(), AnyError> {
-    if self.ps.npm_resolver.has_packages() {
+    if self.ps.npm_resolver.has_packages()
+      || self.ps.has_node_builtin_specifier()
+    {
       self.initialize_main_module_for_node().await?;
     }
     self.worker.evaluate_module(id).await
   }
 
   async fn initialize_main_module_for_node(&mut self) -> Result<(), AnyError> {
-    self.ps.prepare_node_std_graph().await?;
     deno_node::initialize_runtime(
       &mut self.worker.js_runtime,
-      node::MODULE_ALL_URL.as_str(),
       self.ps.options.node_modules_dir(),
     )
     .await?;
@@ -630,7 +626,6 @@ fn create_web_worker_pre_execute_module_callback(
       if ps.npm_resolver.has_packages() {
         deno_node::initialize_runtime(
           &mut worker.js_runtime,
-          node::MODULE_ALL_URL.as_str(),
           ps.options.node_modules_dir(),
         )
         .await?;

core/runtime.rs

@@ -841,7 +841,14 @@ impl JsRuntime {
           )
           .await?;
         let receiver = runtime.mod_evaluate(id);
-        runtime.run_event_loop(false).await?;
+        poll_fn(|cx| {
+          let r = runtime.poll_event_loop(cx, false);
+          // TODO(bartlomieju): some code in readable-stream polyfill in `ext/node`
+          // is calling `nextTick()` during snapshotting, which causes infinite loop
+          runtime.state.borrow_mut().has_tick_scheduled = false;
+          r
+        })
+        .await?;
         receiver.await?
       })
       .with_context(|| format!("Couldn't execute '{}'", file_source.specifier))
@@ -2532,7 +2539,6 @@ impl JsRuntime {
       let tc_scope = &mut v8::TryCatch::new(scope);
       let this = v8::undefined(tc_scope).into();
       js_nexttick_cb.call(tc_scope, this, &[]);
-
       if let Some(exception) = tc_scope.exception() {
         return exception_to_err_result(tc_scope, exception, false);
       }

ext/node/01_node.js

@@ -109,6 +109,8 @@ function initialize(nodeModules, nodeGlobalThisName) {
     writable: false,
     value: nodeGlobalThis,
   });
+  // FIXME(bartlomieju): not nice to depend on `Deno` namespace here
+  internals.__bootstrapNodeProcess(Deno.args);
 }
 
 internals.node = {

ext/node/lib.rs

@@ -26,7 +26,6 @@ pub use path::PathClean;
 pub use polyfill::find_builtin_node_module;
 pub use polyfill::is_builtin_node_module;
 pub use polyfill::NodeModulePolyfill;
-pub use polyfill::NodeModulePolyfillSpecifier;
 pub use polyfill::SUPPORTED_BUILTIN_NODE_MODULES;
 pub use resolution::get_closest_package_json;
 pub use resolution::get_package_scope_config;
@@ -462,18 +461,15 @@ pub fn init<P: NodePermissions + 'static>(
 
 pub async fn initialize_runtime(
   js_runtime: &mut JsRuntime,
-  module_all_url: &str,
   uses_local_node_modules_dir: bool,
 ) -> Result<(), AnyError> {
   let source_code = &format!(
-    r#"(async function loadBuiltinNodeModules(moduleAllUrl, nodeGlobalThisName, usesLocalNodeModulesDir) {{
-      const moduleAll = await import(moduleAllUrl);
-      Deno[Deno.internal].node.initialize(moduleAll.default, nodeGlobalThisName);
+    r#"(async function loadBuiltinNodeModules(nodeGlobalThisName, usesLocalNodeModulesDir) {{
+      Deno[Deno.internal].node.initialize(Deno[Deno.internal].nodeModuleAll, nodeGlobalThisName);
       if (usesLocalNodeModulesDir) {{
         Deno[Deno.internal].require.setUsesLocalNodeModulesDir();
       }}
-    }})('{}', '{}', {});"#,
-    module_all_url,
+    }})('{}', {});"#,
     NODE_GLOBAL_THIS_NAME.as_str(),
     uses_local_node_modules_dir,
   );