From 69da5d8290fda4797af5e3b3e5e7bf3c0141f203 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 28 May 2024 16:24:07 -0400 Subject: [PATCH] perf: skip npm install if graph has no new packages (#24017) --- cli/graph_util.rs | 28 +++++++++++-------- .../__test__.jsonc | 19 +++++++++++++ .../deno.json | 7 +++++ .../adding_npm_dep_in_dynamic_import/lock.out | 20 +++++++++++++ .../adding_npm_dep_in_dynamic_import/main.out | 8 ++++++ .../adding_npm_dep_in_dynamic_import/main.ts | 8 ++++++ .../main2.out | 6 ++++ .../adding_npm_dep_in_dynamic_import/other.ts | 1 + 8 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/__test__.jsonc create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/deno.json create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/lock.out create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/main.out create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/main.ts create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/main2.out create mode 100644 tests/specs/npm/adding_npm_dep_in_dynamic_import/other.ts diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 017d6b35d6eae..b7e3240b53e67 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -566,16 +566,24 @@ 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; @@ -583,8 +591,6 @@ impl ModuleGraphBuilder { 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 diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/__test__.jsonc b/tests/specs/npm/adding_npm_dep_in_dynamic_import/__test__.jsonc new file mode 100644 index 0000000000000..4dcedd92f09ef --- /dev/null +++ b/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" + }] +} diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/deno.json b/tests/specs/npm/adding_npm_dep_in_dynamic_import/deno.json new file mode 100644 index 0000000000000..38bbf89a0c650 --- /dev/null +++ b/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" + } +} diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/lock.out b/tests/specs/npm/adding_npm_dep_in_dynamic_import/lock.out new file mode 100644 index 0000000000000..c67be75c936a5 --- /dev/null +++ b/tests/specs/npm/adding_npm_dep_in_dynamic_import/lock.out @@ -0,0 +1,20 @@ +{ + "version": "3", + "packages": { + "specifiers": { + "npm:@denotest/add": "npm:@denotest/add@1.0.0", + "npm:@denotest/subtract": "npm:@denotest/subtract@1.0.0" + }, + "npm": { + "@denotest/add@1.0.0": { + "integrity": "[WILDLINE]", + "dependencies": {} + }, + "@denotest/subtract@1.0.0": { + "integrity": "[WILDLINE]", + "dependencies": {} + } + } + }, + "remote": {} +} diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/main.out b/tests/specs/npm/adding_npm_dep_in_dynamic_import/main.out new file mode 100644 index 0000000000000..fe612aa3c164e --- /dev/null +++ b/tests/specs/npm/adding_npm_dep_in_dynamic_import/main.out @@ -0,0 +1,8 @@ +Download http://localhost:4260/@denotest/add +Download http://localhost:4260/@denotest/add/1.0.0.tgz +Initialize @denotest/add@1.0.0 +3 +Download http://localhost:4260/@denotest/subtract +Download http://localhost:4260/@denotest/subtract/1.0.0.tgz +Initialize @denotest/subtract@1.0.0 +1 diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/main.ts b/tests/specs/npm/adding_npm_dep_in_dynamic_import/main.ts new file mode 100644 index 0000000000000..75e9b91999442 --- /dev/null +++ b/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)); diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/main2.out b/tests/specs/npm/adding_npm_dep_in_dynamic_import/main2.out new file mode 100644 index 0000000000000..7f1598ed0c1e9 --- /dev/null +++ b/tests/specs/npm/adding_npm_dep_in_dynamic_import/main2.out @@ -0,0 +1,6 @@ +[UNORDERED_START] +Initialize @denotest/add@1.0.0 +Initialize @denotest/subtract@1.0.0 +[UNORDERED_END] +3 +1 diff --git a/tests/specs/npm/adding_npm_dep_in_dynamic_import/other.ts b/tests/specs/npm/adding_npm_dep_in_dynamic_import/other.ts new file mode 100644 index 0000000000000..b8487fa51bade --- /dev/null +++ b/tests/specs/npm/adding_npm_dep_in_dynamic_import/other.ts @@ -0,0 +1 @@ +export * from "npm:@denotest/subtract";