fix(npm): resolve dynamic npm imports individually

denoland · dsherret · Jun 11, 2024 · Jun 10, 2024 · Jun 10, 2024 · Jun 10, 2024
commit a9c712a6bd60b65ac22672d608df47262e247c4f
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -369,3 369,7 @@ opt-level = 3
 opt-level = 3
 [profile.release.package.base64-simd]
 opt-level = 3
 
 [patch.crates-io]
 deno_graph = { path = "../deno_graph" }
 deno_npm = { path = "../deno_npm" }
diff --git a/cli/factory.rs b/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()
  })

diff --git a/cli/graph_util.rs b/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_resolution_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;

diff --git a/cli/lsp/resolver.rs b/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()

diff --git a/cli/module_loader.rs b/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? {

diff --git a/cli/npm/managed/installer.rs b/cli/npm/managed/installer.rs