Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
fix: respect link deps when calc peer dep sets
Browse files Browse the repository at this point in the history
* Show useful debug printing for virtual root nodes
* Respect hidden lockfiles when symlink deps present. As long as the
target of the link is in the shrinkwrap, and not newer.
* Respect link deps when calculating peerDep sets

Previously, we were not including link targets in the virtual trees
where peer dependency sets are calculated.  Additionally, we were still
using the path `/virtual-root` for the virtual node, even though this is
no longer load-bearing.  (And, as of the recent change to the Node
printable output, no longer necessary or particularly helpful for
debugging.)

As a result, a link dependency from the root node like `file:../../foo`
would get resolved against `/virtual-root`, resulting in `/foo`, which
of course does not match any Node in the virtual tree.  The outcome was
an ERESOLVE error where the `current` Node is shown as having no name
or version (because it is an unsatisfied Link).

The solution is two-part.

First, the path of the virtual tree root now matches the path of the
Node that it is sourced from.

Second, any Link children of the source node have their targets mirrored
in the virtual tree, resulting in them being matched appropriately.

The result is that a Link dependency can now properly satisfy a
peerDependency.  Test shows an example of using a Link to a local Node
as a workaround for a peerSet that otherwise would not be resolveable.

This can of course be abused to get around valid peerDep contracts, but
if they user takes it on themselves to use a local fork of a dependency,
we should respect that in buildIdealTree as we do elsewhere.

Fix: npm/cli#2199

PR-URL: #249
Credit: @isaacs
Close: #249
Reviewed-by: @ruyadorno
  • Loading branch information
isaacs authored and ruyadorno committed Mar 11, 2021
1 parent 2657dcb commit fa16e09
Show file tree
Hide file tree
Showing 16 changed files with 905 additions and 164 deletions.
16 changes: 15 additions & 1 deletion lib/arborist/build-ideal-tree.js
Expand Up @@ -883,6 +883,8 @@ This is a one-time fix-up, please be patient...
// 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 realParent = edge.peer ? edge.from.resolveParent : edge.from

Expand Down Expand Up @@ -936,11 +938,23 @@ This is a one-time fix-up, please be patient...
return this[_virtualRoots].get(node)

const vr = new Node({
path: '/virtual-root',
path: node.realpath,
sourceReference: node,
legacyPeerDeps: this.legacyPeerDeps,
})

// also need to set up any targets from any link deps, so that
// they are properly reflected in the virtual environment
for (const child of node.children.values()) {
if (child.isLink) {
new Node({
path: child.realpath,
sourceReference: child.target,
root: vr,
})
}
}

this[_virtualRoots].set(node, vr)
return vr
}
Expand Down
17 changes: 14 additions & 3 deletions lib/printable.js
Expand Up @@ -63,6 +63,13 @@ class ArboristNode {
}
}

class ArboristVirtualNode extends ArboristNode {
constructor (tree, path) {
super(tree, path)
this.sourceReference = printableTree(tree.sourceReference, path)
}
}

class ArboristLink extends ArboristNode {
constructor (tree, path) {
super(tree, path)
Expand Down Expand Up @@ -119,10 +126,14 @@ class EdgeIn extends Edge {
}

const printableTree = (tree, path = []) => {
if (path.includes(tree))
return { location: tree.location }
const Cls = tree.isLink ? ArboristLink
: tree.sourceReference ? ArboristVirtualNode
: ArboristNode
if (path.includes(tree)) {
const obj = Object.create(Cls.prototype)
return Object.assign(obj, { location: tree.location })
}
path.push(tree)
const Cls = tree.isLink ? ArboristLink : ArboristNode
return new Cls(tree, path)
}

Expand Down
18 changes: 14 additions & 4 deletions lib/shrinkwrap.js
Expand Up @@ -41,6 +41,7 @@ const readFile = promisify(fs.readFile)
const writeFile = promisify(fs.writeFile)
const stat = promisify(fs.stat)
const readdir_ = promisify(fs.readdir)
const readlink = promisify(fs.readlink)

// XXX remove when drop support for node v10
const lstat = promisify(fs.lstat)
Expand Down Expand Up @@ -176,10 +177,19 @@ const assertNoNewer = async (path, data, lockTime, dir = path, seen = null) => {
: readdir(parent, { withFileTypes: true })

return children.catch(() => [])
.then(ents => Promise.all(
ents.filter(ent => ent.isDirectory() && !/^\./.test(ent.name))
.map(ent => assertNoNewer(path, data, lockTime, resolve(parent, ent.name), seen))
)).then(() => {
.then(ents => Promise.all(ents.map(async ent => {
const child = resolve(parent, ent.name)
if (ent.isDirectory() && !/^\./.test(ent.name))
await assertNoNewer(path, data, lockTime, child, seen)
else if (ent.isSymbolicLink()) {
const target = resolve(parent, await readlink(child))
const tstat = await stat(target).catch(() => null)
seen.add(relpath(path, child))
if (tstat && tstat.isDirectory() && !seen.has(relpath(path, target)))
await assertNoNewer(path, data, lockTime, target, seen)
}
})))
.then(() => {
if (dir !== path)
return

Expand Down

0 comments on commit fa16e09

Please sign in to comment.