perf: skip npm install if graph has no new packages (denoland#24017)

cli/graph_util.rs

@@ -566,25 +566,31 @@ impl ModuleGraphBuilder {
 
  graph.build(roots, loader, options).await;
 
- 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?;
- }
+ 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?;
+ // 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;
  let has_jsr_package_mappings_changed =
  graph.packages.mappings().len() != initial_package_mappings_len;
- let has_npm_packages_changed =
- graph.npm_packages.len() != initial_npm_packages;
 
  if has_redirects_changed
  || has_jsr_package_deps_changed

tests/specs/npm/adding_npm_dep_in_dynamic_import/__test__.jsonc

@@ -0,0 +1,19 @@
+{
+ "tempDir": true,
+ "steps": [{
+ "args": "run --allow-read=. main.ts",
+ "output": "main.out"
+ }, {
+ "args": "task --quiet cat deno.lock",
+ "output": "lock.out"
+ }, {
+ "args": "task rm_node_modules",
+ "output": "[WILDCARD]"
+ }, {
+ "args": "run --allow-read=. main.ts",
+ "output": "main2.out"
+ }, {
+ "args": "task --quiet cat deno.lock",
+ "output": "lock.out"
+ }]
+}

tests/specs/npm/adding_npm_dep_in_dynamic_import/deno.json

@@ -0,0 +1,7 @@
+{
+ "nodeModulesDir": true,
+ "tasks": {
+ "cat": "cat",
+ "rm_node_modules": "rm -rf node_modules"
+ }
+}

tests/specs/npm/adding_npm_dep_in_dynamic_import/lock.out

@@ -0,0 +1,20 @@
+{
+ "version": "3",
+ "packages": {
+ "specifiers": {
+ "npm:@denotest/add": "npm:@denotest/[email protected]",
+ "npm:@denotest/subtract": "npm:@denotest/[email protected]"
+ },
+ "npm": {
+ "@denotest/[email protected]": {
+ "integrity": "[WILDLINE]",
+ "dependencies": {}
+ },
+ "@denotest/[email protected]": {
+ "integrity": "[WILDLINE]",
+ "dependencies": {}
+ }
+ }
+ },
+ "remote": {}
+}

tests/specs/npm/adding_npm_dep_in_dynamic_import/main.out

@@ -0,0 +1,8 @@
+Download http:https://localhost:4260/@denotest/add
+Download http:https://localhost:4260/@denotest/add/1.0.0.tgz
+Initialize @denotest/[email protected]
+3
+Download http:https://localhost:4260/@denotest/subtract
+Download http:https://localhost:4260/@denotest/subtract/1.0.0.tgz
+Initialize @denotest/[email protected]
+1

tests/specs/npm/adding_npm_dep_in_dynamic_import/main.ts

@@ -0,0 +1,8 @@
+import { add } from "npm:@denotest/add";
+
+console.log(add(1, 2));
+
+const fileName = "other.ts";
+const specifier = "./" + fileName; // non-analyzable
+const { subtract } = await import(specifier);
+console.log(subtract(3, 2));

tests/specs/npm/adding_npm_dep_in_dynamic_import/main2.out

@@ -0,0 +1,6 @@
+[UNORDERED_START]
+Initialize @denotest/[email protected]
+Initialize @denotest/[email protected]
+[UNORDERED_END]
+3
+1

tests/specs/npm/adding_npm_dep_in_dynamic_import/other.ts

@@ -0,0 +1 @@
+export * from "npm:@denotest/subtract";