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

fix(arborist): pass the edge to fromPath in order to determine correct path #5233

Merged
merged 1 commit into from Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 1 addition & 2 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
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
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
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
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