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

Commit

Permalink
fix: treat top-level global packages as "top" nodes
Browse files Browse the repository at this point in the history
When working with the global install space, such as
/usr/local/lib/node_modules, we create a "project root" at the parent of
the node_modules directory, and then more or less proceed normally, with
the globalStyle install strategy.

The main difference, of course, is that in such a case, we should not
ever place *peers* of a top-level global dependency in that same global
space.  This is part of the root cause of npm/cli#3457, where a peer
dependency of the globally installed package was placed as a top-level
globally installed package, instead of being nested under its dependent.

Note that this _also_ could result in an `ERESOLVE` if multiple
globally-installed packages depended upon conflicting versions of a peer
dep, or worse yet, quietly replacing that peerDep in such a way that it
breaks a previously-installed globally installed package.

With this change, peer dependencies of a global top-level package will
always be nested under their dependent, removing any chance that they
might collide with anything else in the top level of the global install
space.

Fix: npm/cli#3457
  • Loading branch information
isaacs committed Aug 19, 2021
1 parent b46cca1 commit 2a4525a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 6 deletions.
9 changes: 9 additions & 0 deletions lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,15 @@ This is a one-time fix-up, please be patient...
// keep track of the thing that caused this node to be included.
const src = parent.sourceReference
this[_peerSetSource].set(node, src)

// do not load the peers along with the set if this is a global top pkg
// otherwise we'll be tempted to put peers as other top-level installed
// things, potentially clobbering what's there already, which is not
// what we want. the missing edges will be picked up on the next pass.
if (this[_global] && edge.from.isProjectRoot)
return node

// otherwise, we have to make sure that our peers can go along with us.
return this[_loadPeerSet](node, required)
}

Expand Down
2 changes: 1 addition & 1 deletion lib/deepest-nesting-target.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
const deepestNestingTarget = (start, name) => {
for (const target of start.ancestry()) {
// note: this will skip past the first target if edge is peer
if (target.isProjectRoot || !target.resolveParent)
if (target.isProjectRoot || !target.resolveParent || target.globalTop)
return target
const targetEdge = target.edgesOut.get(name)
if (!targetEdge || !targetEdge.peer)
Expand Down
19 changes: 14 additions & 5 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ class Node {

// true for packages installed directly in the global node_modules folder
get globalTop () {
return this.global && this.parent.isProjectRoot
return this.global && this.parent && this.parent.isProjectRoot
}

get workspaces () {
Expand Down Expand Up @@ -479,6 +479,9 @@ class Node {
}

get isProjectRoot () {
// only treat as project root if it's the actual link that is the root,
// or the target of the root link, but NOT if it's another link to the
// same root that happens to be somewhere else.
return this === this.root || this === this.root.target
}

Expand Down Expand Up @@ -773,9 +776,15 @@ class Node {
this[_loadDepType](this.package.dependencies, 'prod')
this[_loadDepType](this.package.optionalDependencies, 'optional')

const { isTop, path, sourceReference } = this
const { isTop: srcTop, path: srcPath } = sourceReference || {}
if (isTop && path && (!sourceReference || srcTop && srcPath))
const { globalTop, isTop, path, sourceReference } = this
const {
globalTop: srcGlobalTop,
isTop: srcTop,
path: srcPath,
} = sourceReference || {}
const thisDev = isTop && !globalTop && path
const srcDev = !sourceReference || srcTop && !srcGlobalTop && srcPath
if (thisDev && srcDev)
this[_loadDepType](this.package.devDependencies, 'dev')
}

Expand Down Expand Up @@ -1223,7 +1232,7 @@ class Node {
}

get isTop () {
return !this.parent
return !this.parent || this.globalTop
}

get top () {
Expand Down

0 comments on commit 2a4525a

Please sign in to comment.