Skip to content

Commit

Permalink
fix(arborist): pass the edge to fromPath in order to determine correc…
Browse files Browse the repository at this point in the history 
…t path (#5233)

by passing in the edge we can determine if the edge is overridden, and if it is the path we want to return is the project root since that's what user's will have define their overrides relative to
  • Loading branch information
@nlf
nlf committed Aug 1, 2022
1 parent 47cc95d commit 050284d
Show file tree
Hide file tree
Showing 7 changed files with 266 additions and 32 deletions.
3 changes: 1 addition & 2 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
View file
Expand Up @@ -1076,9 +1076,8 @@ This is a one-time fix-up, please be patient...
// if it fails at this point, though, dont' worry because it
// may well be an optional dep that has gone missing. it'll
// fail later anyway.
const from = fromPath(placed)
promises.push(...this[_problemEdges](placed).map(e =>
this[_fetchManifest](npa.resolve(e.name, e.spec, from))
this[_fetchManifest](npa.resolve(e.name, e.spec, fromPath(placed, e)))
.catch(er => null)))
},
})
Expand Down
2 changes: 1 addition & 1 deletion workspaces/arborist/lib/dep-valid.js
View file
Expand Up @@ -20,7 +20,7 @@ const depValid = (child, requested, requestor) => {
// file: deps that depend on other files/dirs, we must resolve the
// location based on the *requestor* file/dir, not where it ends up.
// '' is equivalent to '*'
requested = npa.resolve(child.name, requested || '*', fromPath(requestor))
requested = npa.resolve(child.name, requested || '*', fromPath(requestor, requestor.edgesOut.get(child.name)))
} catch (er) {
// Not invalid because the child doesn't match, but because
// the spec itself is not supported. Nothing would match,
Expand Down
19 changes: 15 additions & 4 deletions workspaces/arborist/lib/from-path.js
View file
Expand Up @@ -6,8 +6,19 @@
const { dirname } = require('path')
const npa = require('npm-package-arg')

const fromPath = (node, spec) =>
spec && spec.type === 'file' ? dirname(spec.fetchSpec)
: node.realpath
const fromPath = (node, spec, edge) => {
if (edge && edge.overrides && edge.overrides.name === edge.name && edge.overrides.value) {
// fromPath could be called with a node that has a virtual root, if that happens
// we want to make sure we get the real root node when overrides are in use. this
// is to allow things like overriding a dependency with a tarball file that's a
// relative path from the project root
return node.sourceReference
? node.sourceReference.root.realpath
: node.root.realpath
}

module.exports = node => fromPath(node, node.resolved && npa(node.resolved))
return spec && spec.type === 'file' ? dirname(spec.fetchSpec)
: node.realpath
}

module.exports = (node, edge) => fromPath(node, node.resolved && npa(node.resolved), edge)
189 changes: 189 additions & 0 deletions workspaces/arborist/tap-snapshots/test/edge.js.test.cjs
View file
Expand Up @@ -88,6 +88,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"invalid": false,
Expand Down Expand Up @@ -188,6 +209,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"invalid": true,
Expand Down Expand Up @@ -345,6 +387,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"resolve": Function resolve(n),
Expand Down Expand Up @@ -414,6 +477,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"resolve": Function resolve(n),
Expand Down Expand Up @@ -614,6 +698,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"resolve": Function resolve(n),
Expand Down Expand Up @@ -697,6 +802,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"a" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"type": "peer",
Expand Down Expand Up @@ -748,6 +874,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"invalid": false,
Expand Down Expand Up @@ -814,6 +961,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"resolve": Function resolve(n),
Expand Down Expand Up @@ -868,6 +1036,27 @@ Edge {
"version": "1.2.3",
},
"resolve": Function resolve(n),
"root": Object {
"addEdgeIn": Function addEdgeIn(edge),
"addEdgeOut": Function addEdgeOut(edge),
"edgesIn": Set {},
"edgesOut": Map {
"missing" => Edge {
"peerConflicted": false,
},
},
"explain": Function explain(),
"isTop": true,
"name": "top",
"package": Object {
"name": "top",
"version": "1.2.3",
},
"packageName": "top",
"parent": null,
"resolve": Function resolve(n),
"version": "1.2.3",
},
"version": "1.2.3",
},
"invalid": false,
Expand Down

0 comments on commit 050284d

Please sign in to comment.