From 23d9832f465fc3742b71439dd02a747237dad9e6 Mon Sep 17 00:00:00 2001 From: Michael Garvin Date: Wed, 8 Apr 2026 10:00:41 -0700 Subject: [PATCH] fix: pass _isRoot context where missing fixes: https://github.com/npm/cli/issues/9189 --- .../arborist/lib/arborist/build-ideal-tree.js | 48 +++++++------------ .../arborist/lib/arborist/isolated-reifier.js | 1 + workspaces/arborist/lib/arborist/reify.js | 1 + 3 files changed, 20 insertions(+), 30 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index a256b79049eba..fdbbd4679bd80 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -950,7 +950,7 @@ This is a one-time fix-up, please be patient... tree: pd, getChildren: pd => pd.children, visit: pd => { - const { placed, edge, canPlace: cpd } = pd + const { placed, edge, canPlace: cpd, parent } = pd // if we didn't place anything, nothing to do here if (!placed) { return @@ -1011,8 +1011,7 @@ This is a one-time fix-up, please be patient... return } - // lastly, also check for the missing deps of the node we placed, - // and any holes created by pruning out conflicted peer sets. + // lastly, also check for the missing deps of the node we placed, and any holes created by pruning out conflicted peer sets. this.#depsQueue.push(placed) for (const dep of pd.needEvaluation) { this.#depsSeen.delete(dep) @@ -1020,16 +1019,14 @@ This is a one-time fix-up, please be patient... } // pre-fetch any problem edges, since we'll need these soon - // if it fails at this point, though, don't worry because it - // may well be an optional dep that has gone missing. it'll - // fail later anyway. + // if it fails at this point, though, don't worry because it may well be an optional dep that has gone missing + // it'll fail later anyway for (const e of this.#problemEdges(placed)) { - // XXX This is somehow load bearing. This makes tests that print - // the ideal tree of a tree with tarball dependencies fail. This - // can't be changed or removed till we figure out why + // XXX This is somehow load bearing. This makes tests that print the ideal tree of a tree with tarball dependencies fail + // This can't be changed or removed till we figure out why // The test is named "tarball deps with transitive tarball deps" promises.push(() => - this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e))) + this.#fetchManifest(npa.resolve(e.name, e.spec, fromPath(placed, e)), parent) .catch(() => null) ) } @@ -1047,26 +1044,18 @@ This is a one-time fix-up, please be patient... return this.#buildDepStep() } - // loads a node from an edge, and then loads its peer deps (and their - // peer deps, on down the line) into a virtual root parent. + // loads a node from an edge, and then loads its peer deps (and their peer deps, on down the line) into a virtual root parent. async #nodeFromEdge (edge, parent_, secondEdge, required) { - // create a virtual root node with the same deps as the node that - // is requesting this one, so that we can get all the peer deps in - // a context where they're likely to be resolvable. - // Note that the virtual root will also have virtual copies of the - // targets of any child Links, so that they resolve appropriately. + // create a virtual root node with the same deps as the node that is requesting this one, so that we can get all the peer deps in a context where they're likely to be resolvable. + // Note that the virtual root will also have virtual copies of the targets of any child Links, so that they resolve appropriately. const parent = parent_ || this.#virtualRoot(edge.from) const spec = npa.resolve(edge.name, edge.spec, edge.from.path) const first = await this.#nodeFromSpec(edge.name, spec, parent, edge) - // we might have a case where the parent has a peer dependency on - // `foo@*` which resolves to v2, but another dep in the set has a - // peerDependency on `foo@1`. In that case, if we force it to be v2, - // we're unnecessarily triggering an ERESOLVE. - // If we have a second edge to worry about, and it's not satisfied - // by the first node, try a second and see if that satisfies the - // original edge here. + // we might have a case where the parent has a peer dependency on `foo@*` which resolves to v2, but another dep in the set has a peerDependency on `foo@1`. + // In that case, if we force it to be v2, we're unnecessarily triggering an ERESOLVE. + // If we have a second edge to worry about, and it's not satisfied by the first node, try a second and see if that satisfies the original edge here. const spec2 = secondEdge && npa.resolve( edge.name, secondEdge.spec, @@ -1210,11 +1199,12 @@ This is a one-time fix-up, please be patient... return problems } - async #fetchManifest (spec) { + async #fetchManifest (spec, parent) { const options = { ...this.options, avoid: this.#avoidRange(spec.name), fullMetadata: true, + _isRoot: parent?.isProjectRoot || parent?.isWorkspace, } // get the intended spec and stored metadata from yarn.lock file, // if available and valid. @@ -1231,10 +1221,8 @@ This is a one-time fix-up, please be patient... } async #nodeFromSpec (name, spec, parent, edge) { - // pacote will slap integrity on its options, so we have to clone - // the object so it doesn't get mutated. - // Don't bother to load the manifest for link deps, because the target - // might be within another package that doesn't exist yet. + // pacote will slap integrity on its options, so we have to clone the object so it doesn't get mutated. + // Don't bother to load the manifest for link deps, because the target might be within another package that doesn't exist yet. const { installLinks, legacyPeerDeps } = this const isWorkspace = this.idealTree.workspaces && this.idealTree.workspaces.has(spec.name) @@ -1287,7 +1275,7 @@ This is a one-time fix-up, please be patient... // spec isn't a directory, and either isn't a workspace or the workspace we have // doesn't satisfy the edge. try to fetch a manifest and build a node from that. - return this.#fetchManifest(spec) + return this.#fetchManifest(spec, parent) .then(pkg => new Node({ name, pkg, parent, installLinks, legacyPeerDeps }), error => { error.requiredBy = edge.from.location || '.' diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index 121ad0effc8f6..c0f4eccc5f43e 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -157,6 +157,7 @@ module.exports = cls => class IsolatedReifier extends cls { ...this.options, resolved: node.resolved, integrity: node.integrity, + // TODO _isRoot }) const Arborist = this.constructor const arb = new Arborist({ ...this.options, path: dir }) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 44ac7cd34dcbc..26ad0016be3a9 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -737,6 +737,7 @@ module.exports = cls => class Reifier extends cls { ...this.options, resolved: node.resolved, integrity: node.integrity, + _isRoot: node.parent?.isProjectRoot || node.parent?.isWorkspace, }) // store nodes don't use Node class so node.package doesn't get updated if (node.isInStore) {