Repository navigation
fix(arborist): skip bundle checks without an applicable override - #10128
Open
jeffwidman wants to merge 1 commit into
Open
jeffwidman wants to merge 1 commit into
jeffwidman wants to merge 1 commit into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jeffwidman
marked this pull request as ready for review
October 15, 2026 05:19
martinrrm
approved these changes
Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move the existing no-applicable-override return in
Edge.satisfiedBy()ahead of the bundle-membership check.Why this removes work
Bundle membership determines whether dependency validation should use an overridden spec or the original spec. Without an applicable override, that distinction cannot affect the result, but the current ordering still asks
node.inDepBundle, potentially traversing the dependency graph.Checking the existing cheap condition first avoids that irrelevant traversal. This is a guard reorder, not a new cache, a skipped dependency-validation step, or a public API change. When an override applies, the existing bundle handling and validation logic are unchanged.
The regression test covers both absent override state and an inherited override set with no applicable rule. It checks both satisfying and unsatisfying versions and fails if bundle membership is queried.
Local measurements
I applied only this guard reorder to an isolated npm 11.17.0 installation using Node 24.22.0, then compared it with an unmodified installation. Neither arm includes the separate visited-set optimization.
The workload was a large workspace snapshot with 5,405 original lockfile entries, updating
prettierto3.9.10:Each invocation started from fresh copies of the same inputs and a warm metadata cache, in a four-CPU/12-GB container with networking disabled. One warmup per arm was excluded, followed by three measured alternating pairs. Timed runs used patched source directly, without preload hooks or profiling.
All package manifests and lockfiles were byte-identical across variants. npm output matched after removing timing information; every invocation exited successfully.
A tiny-project control with one local dependency used seven alternating measured pairs after warmups. CPU was 0.346975 -> 0.348489 s and wall was 0.271516 -> 0.272622 s, both about 0.4% higher, with identical output. Those small differences do not establish a material regression.
These are local native-command measurements, not a claim about every project or complete dependency-update jobs. The large fixture is not included. The synthetic example below independently demonstrates the eliminated work.
The benchmark runtime uses the older bundle/shrinkwrap predicate; this PR uses current upstream's
inDepBundlepredicate. In both cases the change only moves the no-override guard ahead of the existing predicate.Validation
On this standalone branch based on
latest, using Node 24.22.0:node bin/npm-cli.js test --workspace=@npmcli/arborist --ignore-scripts -- --jobs=4— passed: 59 test files; 100% statements, branches, functions, and lines.node node_modules/eslint/bin/eslint.js workspaces/arborist/lib/edge.js workspaces/arborist/test/edge.js— passed.Public synthetic reproduction
Run from the repository root after installing development dependencies. This counts work, not elapsed time. The graph has no overrides, so bundle membership cannot affect these edge-satisfaction results.
Prepared with assistance from GitHub Copilot. Validation results above were obtained locally.
References