Repository navigation
Conversation
) <!-- What / Why --> <!-- Describe the request in detail. What it does and why it's being changed. --> Move the existing no-applicable-override return in `Edge.satisfiedBy()` ahead of the bundle-membership check. 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. 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 `prettier` to `3.9.10`: ```sh node <npm-installation>/bin/npm-cli.js install prettier@3.9.10 \ --package-lock-only --ignore-scripts --audit=false --fund=false \ --timing --force --offline node <npm-installation>/bin/npm-cli.js install prettier@3.9.10 \ --package-lock-only --ignore-scripts --audit=false --fund=false \ --timing --dry-run=true --offline ``` 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. | Command | Median CPU: original -> patched | Median wall: original -> patched | | --- | --- | --- | | Lockfile writer | 9.277 -> 7.359 s (**20.7% lower**) | 8.080 -> 6.326 s (**21.7% lower**) | | Dry-run checker | 7.565 -> 5.853 s (**22.6% lower**) | 6.528 -> 4.824 s (**26.1% lower**) | 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 `inDepBundle` predicate. In both cases the change only moves the no-override guard ahead of the existing predicate. 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 example below — **passed: 128 valid edges in both variants; recursive bundle-traversal calls reduced from 49,408 to zero**. - The npm CLI-wide suite was not run. <details> <summary>Public synthetic reproduction</summary> 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. ```sh node <<'JS' const assert = require('node:assert/strict') const {resolve} = require('node:path') const Node = require(resolve('workspaces/arborist/lib/node.js')) const names = Array.from({length: 128}, (_, i) => `provider-${i}`) const root = new Node({ path: '/synthetic', pkg: { name: 'root', version: '1.0.0', dependencies: Object.fromEntries(names.map(name => [name, '*'])), }, }) const shared = new Node({ parent: root, pkg: {name: 'shared', version: '1.0.0'}, }) const providers = names.map(name => new Node({ parent: root, pkg: {name, version: '1.0.0', dependencies: {shared: '*'}}, })) let bundleTraversalCalls = 0 const getBundler = Node.prototype.getBundler Node.prototype.getBundler = function (...args) { bundleTraversalCalls++ return getBundler.apply(this, args) } try { for (const provider of providers) { assert.equal(provider.edgesOut.get('shared').satisfiedBy(shared), true) } } finally { Node.prototype.getBundler = getBundler } console.log({validatedEdges: providers.length, bundleTraversalCalls}) JS ``` </details> Prepared with assistance from GitHub Copilot. Validation results above were obtained locally. <!-- Examples: Related to #0 Depends on #0 Blocked by #0 Fixes #0 Closes #0 --> - Related to [#10127](#10127), but this branch is independent and does not include that change. That PR makes necessary traversals cheaper; this PR avoids traversals whose result is irrelevant. The benchmark savings should not be added together. - Retains the override/bundle semantics covered by [#9235](#9235). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nkolev92
marked this pull request as draft
November 6, 2026 18:12
nkolev92
marked this pull request as ready for review
November 6, 2026 19:51
jeffkl
approved these changes
Nov 10, 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.
Summary
Backports #10128 to
release/v11frombackport/v11/10128.Moves the no-applicable-override guard in
Edge.satisfiedBy()ahead of bundle/shrinkwrap checks to avoid unnecessary dependency-graph traversal. The production change retains v11's existing bundle/shrinkwrap predicate.