refactor: use deno_graph for npm specifiers (#17858)

This changes npm specifiers to be handled by deno_graph and resolved to
an npm package name and version when the specifier is encountered. It
also slightly changes how npm specifier resolution occurs—previously it
would collect all the npm specifiers and resolve them all at once, but
now it resolves them on the fly as they are encountered in the module
graph.

https://github.com/denoland/deno_graph/pull/232

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

1 files changed, 67 insertions, 47 deletions

diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index 77dc3011c..ef1e0f59a 100644
--- a/cli/graph_util.rs
+++ b/cli/graph_util.rs
@@ -8,7 +8,7 @@ use crate::cache;
 use crate::cache::TypeCheckCache;
 use crate::colors;
 use crate::errors::get_error_class_name;
-use crate::npm::resolve_graph_npm_info;
+use crate::npm::NpmPackageResolver;
 use crate::proc_state::ProcState;
 use crate::resolver::CliGraphResolver;
 use crate::tools::check;
@@ -17,6 +17,7 @@ use deno_core::anyhow::bail;
 use deno_core::error::custom_error;
 use deno_core::error::AnyError;
 use deno_core::ModuleSpecifier;
+use deno_graph::Module;
 use deno_graph::ModuleGraph;
 use deno_graph::ModuleGraphError;
 use deno_graph::ResolutionError;
@@ -121,20 +122,23 @@ pub fn graph_valid(
 /// Checks the lockfile against the graph and and exits on errors.
 pub fn graph_lock_or_exit(graph: &ModuleGraph, lockfile: &mut Lockfile) {
   for module in graph.modules() {
-    if let Some(source) = &module.maybe_source {
-      if !lockfile.check_or_insert_remote(module.specifier.as_str(), source) {
-        let err = format!(
-          concat!(
-            "The source code is invalid, as it does not match the expected hash in the lock file.\n",
-            "  Specifier: {}\n",
-            "  Lock file: {}",
-          ),
-          module.specifier,
-          lockfile.filename.display(),
-        );
-        log::error!("{} {}", colors::red("error:"), err);
-        std::process::exit(10);
-      }
+    let source = match module {
+      Module::Esm(module) => &module.source,
+      Module::Json(module) => &module.source,
+      Module::Node(_) | Module::Npm(_) | Module::External(_) => continue,
+    };
+    if !lockfile.check_or_insert_remote(module.specifier().as_str(), source) {
+      let err = format!(
+        concat!(
+          "The source code is invalid, as it does not match the expected hash in the lock file.\n",
+          "  Specifier: {}\n",
+          "  Lock file: {}",
+        ),
+        module.specifier(),
+        lockfile.filename.display(),
+      );
+      log::error!("{} {}", colors::red("error:"), err);
+      std::process::exit(10);
     }
   }
 }
@@ -154,30 +158,33 @@ pub async fn create_graph_and_maybe_check(
   let cli_resolver = CliGraphResolver::new(
     ps.options.to_maybe_jsx_import_source_config(),
     ps.maybe_import_map.clone(),
+    ps.options.no_npm(),
+    ps.npm_resolver.api().clone(),
+    ps.npm_resolver.resolution().clone(),
     maybe_package_json_deps,
   );
   let graph_resolver = cli_resolver.as_graph_resolver();
+  let graph_npm_resolver = cli_resolver.as_graph_npm_resolver();
   let analyzer = ps.parsed_source_cache.as_analyzer();
   let mut graph = ModuleGraph::default();
-  graph
-    .build(
-      vec![root],
-      &mut cache,
-      deno_graph::BuildOptions {
-        is_dynamic: false,
-        imports: maybe_imports,
-        resolver: Some(graph_resolver),
-        module_analyzer: Some(&*analyzer),
-        reporter: None,
-      },
-    )
-    .await;
+  build_graph_with_npm_resolution(
+    &mut graph,
+    &ps.npm_resolver,
+    vec![root],
+    &mut cache,
+    deno_graph::BuildOptions {
+      is_dynamic: false,
+      imports: maybe_imports,
+      resolver: Some(graph_resolver),
+      npm_resolver: Some(graph_npm_resolver),
+      module_analyzer: Some(&*analyzer),
+      reporter: None,
+    },
+  )
+  .await?;
+
   graph_valid_with_cli_options(&graph, &graph.roots, &ps.options)?;
   let graph = Arc::new(graph);
-  let npm_graph_info = resolve_graph_npm_info(&graph);
-  ps.npm_resolver
-    .add_package_reqs(npm_graph_info.package_reqs)
-    .await?;
   if let Some(lockfile) = &ps.lockfile {
     graph_lock_or_exit(&graph, &mut lockfile.lock());
   }
@@ -185,7 +192,7 @@ pub async fn create_graph_and_maybe_check(
   if ps.options.type_check_mode() != TypeCheckMode::None {
     // node built-in specifiers use the @types/node package to determine
     // types, so inject that now after the lockfile has been written
-    if npm_graph_info.has_node_builtin_specifier {
+    if graph.has_node_specifier {
       ps.npm_resolver
         .inject_synthetic_types_node_package()
         .await?;
@@ -211,7 +218,6 @@ pub async fn create_graph_and_maybe_check(
         ts_config: ts_config_result.ts_config,
         log_checks: true,
         reload: ps.options.reload_flag(),
-        has_node_builtin_specifier: npm_graph_info.has_node_builtin_specifier,
       },
     )?;
     log::debug!("{}", check_result.stats);
@@ -223,23 +229,37 @@ pub async fn create_graph_and_maybe_check(
   Ok(graph)
 }
 
+pub async fn build_graph_with_npm_resolution<'a>(
+  graph: &mut ModuleGraph,
+  npm_resolver: &NpmPackageResolver,
+  roots: Vec<ModuleSpecifier>,
+  loader: &mut dyn deno_graph::source::Loader,
+  options: deno_graph::BuildOptions<'a>,
+) -> Result<(), AnyError> {
+  graph.build(roots, loader, options).await;
+
+  // resolve the dependencies of any pending dependencies
+  // that were inserted by building the graph
+  npm_resolver.resolve_pending().await?;
+
+  Ok(())
+}
+
 pub fn error_for_any_npm_specifier(
-  graph: &deno_graph::ModuleGraph,
+  graph: &ModuleGraph,
 ) -> Result<(), AnyError> {
-  let first_npm_specifier = graph
-    .specifiers()
-    .filter_map(|(_, r)| match r {
-      Ok(module) if module.kind == deno_graph::ModuleKind::External => {
-        Some(&module.specifier)
+  for module in graph.modules() {
+    match module {
+      Module::Npm(module) => {
+        bail!("npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: {}", module.specifier)
       }
-      _ => None,
-    })
-    .next();
-  if let Some(npm_specifier) = first_npm_specifier {
-    bail!("npm specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: {}", npm_specifier)
-  } else {
-    Ok(())
+      Module::Node(module) => {
+        bail!("Node specifiers have not yet been implemented for this sub command (https://github.com/denoland/deno/issues/15960). Found: node:{}", module.module_name)
+      }
+      Module::Esm(_) | Module::Json(_) | Module::External(_) => {}
+    }
   }
+  Ok(())
 }
 
 /// Adds more explanatory information to a resolution error.