refactor(npm): reorganize initialization of compat layer (#16471)

Ensures that "std/node" graph is analyzed only once.

3 files changed, 27 insertions, 23 deletions

diff --git a/cli/main.rs b/cli/main.rs
index af0b532c0..9444ac2bf 100644
--- a/cli/main.rs
+++ b/cli/main.rs
@@ -292,17 +292,6 @@ async fn load_and_type_check(
   for file in files {
     let specifier = resolve_url_or_path(file)?;
 
-    // TODO(bartlomieju): in the future (after all relevant deno subcommands
-    // have support for npm: specifiers), it would be good to unify this code
-    // in `ProcState::prepare_module_load`.
-    if let Ok(package_ref) = NpmPackageReference::from_specifier(&specifier) {
-      ps.npm_resolver
-        .add_package_reqs(vec![package_ref.req.clone()])
-        .await?;
-      ps.prepare_node_std_graph().await?;
-      continue;
-    }
-
     ps.prepare_module_load(
       vec![specifier],
       false,
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index 7fc28b553..0410fd3d9 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -61,6 +61,8 @@ use log::warn;
 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.
@@ -93,6 +95,7 @@ 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 {
@@ -280,6 +283,7 @@ impl ProcState {
       npm_resolver,
       cjs_resolutions: Default::default(),
       progress_bar,
+      node_std_graph_prepared: AtomicBool::new(false),
     })))
   }
 
@@ -297,12 +301,20 @@ impl ProcState {
     reload_on_watch: bool,
   ) -> Result<(), AnyError> {
     let _pb_clear_guard = self.progress_bar.clear_guard();
+    let mut npm_package_reqs = vec![];
+
+    for root in &roots {
+      if let Ok(package_ref) = NpmPackageReference::from_specifier(root) {
+        npm_package_reqs.push(package_ref.req);
+      }
+    }
+
     let roots = roots
       .into_iter()
       .map(|s| (s, ModuleKind::Esm))
       .collect::<Vec<_>>();
 
-    if !reload_on_watch {
+    if !reload_on_watch && npm_package_reqs.is_empty() {
       let graph_data = self.graph_data.read();
       if self.options.type_check_mode() == TypeCheckMode::None
         || graph_data.is_type_checked(&roots, &lib)
@@ -397,7 +409,7 @@ impl ProcState {
       graph_data.entries().map(|(s, _)| s).cloned().collect()
     };
 
-    let npm_package_references = {
+    {
       let mut graph_data = self.graph_data.write();
       graph_data.add_graph(&graph, reload_on_watch);
       let check_js = self.options.check_js();
@@ -408,14 +420,11 @@ impl ProcState {
           check_js,
         )
         .unwrap()?;
-      graph_data.npm_package_reqs()
+      npm_package_reqs.extend(graph_data.npm_package_reqs());
     };
 
-    if !npm_package_references.is_empty() {
-      self
-        .npm_resolver
-        .add_package_reqs(npm_package_references)
-        .await?;
+    if !npm_package_reqs.is_empty() {
+      self.npm_resolver.add_package_reqs(npm_package_reqs).await?;
       self.prepare_node_std_graph().await?;
     }
 
@@ -471,10 +480,15 @@ impl ProcState {
   // FIXME(bartlomieju): appears this function can be called more than once
   // if we have npm imports
   pub async fn prepare_node_std_graph(&self) -> Result<(), AnyError> {
+    if self.node_std_graph_prepared.load(Ordering::Relaxed) {
+      return Ok(());
+    }
+
     let node_std_graph = self
       .create_graph(vec![(node::MODULE_ALL_URL.clone(), ModuleKind::Esm)])
       .await?;
     self.graph_data.write().add_graph(&node_std_graph, false);
+    self.node_std_graph_prepared.store(true, Ordering::Relaxed);
     Ok(())
   }
 
diff --git a/cli/worker.rs b/cli/worker.rs
index 10fe5b36f..1aa2abdd9 100644
--- a/cli/worker.rs
+++ b/cli/worker.rs
@@ -60,6 +60,7 @@ 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?;
       node::load_cjs_module_from_ext_node(
         &mut self.worker.js_runtime,
@@ -280,6 +281,9 @@ 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
   }
@@ -302,6 +306,7 @@ impl CliMainWorker {
   }
 
   async fn initialize_main_module_for_node(&mut self) -> Result<(), AnyError> {
+    self.ps.prepare_node_std_graph().await?;
     node::initialize_runtime(&mut self.worker.js_runtime).await?;
     if let DenoSubcommand::Run(flags) = self.ps.options.sub_command() {
       if let Ok(pkg_ref) = NpmPackageReference::from_str(&flags.script) {
@@ -372,10 +377,6 @@ pub async fn create_main_worker(
     (main_module, false)
   };
 
-  if ps.npm_resolver.has_packages() {
-    ps.prepare_node_std_graph().await?;
-  }
-
   let module_loader = CliModuleLoader::new(ps.clone());
 
   let maybe_inspector_server = ps.maybe_inspector_server.clone();