fix(npm): resolve dynamic npm imports individually

cli/Cargo.toml

@@ -69,10 +69,10 @@ deno_config = "=0.16.4"
 deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
 deno_doc = { version = "=0.139.0", features = ["html", "syntect"] }
 deno_emit = "=0.42.0"
-deno_graph = { version = "=0.78.0", features = ["tokio_executor"] }
+deno_graph = { version = "=0.78.1", features = ["tokio_executor"] }
 deno_lint = { version = "=0.60.0", features = ["docs"] }
 deno_lockfile.workspace = true
-deno_npm = "=0.21.3"
+deno_npm = "=0.21.4"
 deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
 deno_semver = "=0.5.4"
 deno_task_shell = "=0.16.1"

cli/factory.rs

@@ -35,7 +35,6 @@ use crate::npm::CliNpmResolver;
 use crate::npm::CliNpmResolverByonmCreateOptions;
 use crate::npm::CliNpmResolverCreateOptions;
 use crate::npm::CliNpmResolverManagedCreateOptions;
-use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption;
 use crate::npm::CliNpmResolverManagedSnapshotOption;
 use crate::resolver::CjsResolutionStore;
 use crate::resolver::CliGraphResolver;
@@ -441,10 +440,8 @@ impl CliFactory {
             cache_setting: self.options.cache_setting(),
             text_only_progress_bar: self.text_only_progress_bar().clone(),
             maybe_node_modules_path: self.options.node_modules_dir_path().cloned(),
-            package_json_installer:
-              CliNpmResolverManagedPackageJsonInstallerOption::ConditionalInstall(
+            package_json_deps_provider:
                 self.package_json_deps_provider().clone(),
-              ),
             npm_system_info: self.options.npm_system_info(),
             npmrc: self.options.npmrc().clone()
           })

cli/graph_util.rs

@@ -160,6 +160,10 @@ pub fn graph_valid(
   if let Some(error) = errors.next() {
     Err(error)
   } else {
+    // finally surface the npm resolution result
+    if let Err(err) = &graph.npm_dep_graph_result {
+      return Err(custom_error(get_error_class_name(err), format!("{}", err)));
+    }
     Ok(())
   }
 }
@@ -562,30 +566,9 @@ impl ModuleGraphBuilder {
     let initial_redirects_len = graph.redirects.len();
     let initial_package_deps_len = graph.packages.package_deps_sum();
     let initial_package_mappings_len = graph.packages.mappings().len();
-    let initial_npm_packages = graph.npm_packages.len();
 
     graph.build(roots, loader, options).await;
 
-    let has_npm_packages_changed =
-      graph.npm_packages.len() != initial_npm_packages;
-    // skip installing npm packages if we don't have to
-    if is_first_execution
-      && self.npm_resolver.root_node_modules_path().is_some()
-      || has_npm_packages_changed
-    {
-      if let Some(npm_resolver) = self.npm_resolver.as_managed() {
-        // ensure that the top level package.json is installed if a
-        // specifier was matched in the package.json
-        if self.resolver.found_package_json_dep() {
-          npm_resolver.ensure_top_level_package_json_install().await?;
-        }
-
-        // resolve the dependencies of any pending dependencies
-        // that were inserted by building the graph
-        npm_resolver.resolve_pending().await?;
-      }
-    }
-
     let has_redirects_changed = graph.redirects.len() != initial_redirects_len;
     let has_jsr_package_deps_changed =
       graph.packages.package_deps_sum() != initial_package_deps_len;

cli/lsp/resolver.rs

@@ -12,7 +12,6 @@ use crate::npm::CliNpmResolver;
 use crate::npm::CliNpmResolverByonmCreateOptions;
 use crate::npm::CliNpmResolverCreateOptions;
 use crate::npm::CliNpmResolverManagedCreateOptions;
-use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption;
 use crate::npm::CliNpmResolverManagedSnapshotOption;
 use crate::npm::ManagedCliNpmResolver;
 use crate::resolver::CliGraphResolver;
@@ -347,9 +346,11 @@ async fn create_npm_resolver(
       cache_setting: CacheSetting::Only,
       text_only_progress_bar: ProgressBar::new(ProgressBarStyle::TextOnly),
       maybe_node_modules_path: config_data.node_modules_dir.clone(),
-      // do not install while resolving in the lsp—leave that to the cache command
-      package_json_installer:
-        CliNpmResolverManagedPackageJsonInstallerOption::NoInstall,
+      package_json_deps_provider: Arc::new(PackageJsonDepsProvider::new(
+        config_data.package_json.as_ref().map(|package_json| {
+          package_json::get_local_package_json_version_reqs(package_json)
+        }),
+      )),
       npmrc: config_data
         .npmrc
         .clone()

cli/module_loader.rs

@@ -75,7 +75,6 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> {
   let npm_resolver = factory.npm_resolver().await?;
   if let Some(npm_resolver) = npm_resolver.as_managed() {
     npm_resolver.ensure_top_level_package_json_install().await?;
-    npm_resolver.resolve_pending().await?;
   }
   // cache as many entries in the import map as we can
   if let Some(import_map) = factory.maybe_import_map().await? {