Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 24 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
perf: skip npm install if graph has no new packages
  • Loading branch information
dsherret committed May 28, 2024
commit 16c881eb60f5ceb82ad3d7d5be68036f93e6838b
27 changes: 16 additions & 11 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,25 +565,30 @@ 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;
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
Expand Down
19 changes: 19 additions & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -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"
}]
}
7 changes: 7 additions & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"nodeModulesDir": true,
"tasks": {
"cat": "cat",
"rm_node_modules": "rm -rf node_modules"
}
}
20 changes: 20 additions & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/lock.out
Original file line number Diff line number Diff line change
@@ -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": "sha512-KgwwXAkEQYoTmHAWGukagTXTISHua3DPnKtUwd2ju4/+IFtgkztqku1ZZg9y6mfKOCrkja+yZyky/ktQB7/9ZQ==",
"dependencies": {}
},
"@denotest/[email protected]": {
"integrity": "sha512-JCkFYOt0KisCeT7+TtQaGPEqAPSs2tNlv5GZDCbSxPw1YVzWBbjj1YbUORAgtaXBZK0d7aUMvaIcuNKFDb9GIQ==",
"dependencies": {}
}
}
},
"remote": {}
}
8 changes: 8 additions & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/main.out
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/main.ts
Original file line number Diff line number Diff line change
@@ -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));
6 changes: 6 additions & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/main2.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[UNORDERED_START]
Initialize @denotest/[email protected]
Initialize @denotest/[email protected]
[UNORDERED_END]
3
1
1 change: 1 addition & 0 deletions tests/specs/npm/adding_npm_dep_in_dynamic_import/other.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "npm:@denotest/subtract";