Skip to content

Commit fcd78b0

Browse files
committed
fix(arborist): omit failed optional dependencies from installed deps, but keep them 'in the tree'
Fixes: #4828 Fixes: #7961 Replaces: #8127 Replaces: #8077 When optional dependencies fail, we don't want to install them, for whatever reason. The way this "uninstallation" is done right now is by simply removing the dependencies from the ideal tree during reification. Unfortunately, this means that what gets saved to the "hidden lockfile" is the edited ideal tree as well, and when NPM runs next, it'll use that when regenerating the "real" package-lock. This PR tags failed optional deps such that they're still added to the "trash list" (and thus have their directories cleaned up by the reifier), but prevents Arborist from removing them altogether from the ideal tree (which is usually done by setting their parent to `null`, making them unreachable, and letting them get GC'd).
1 parent 8b7bb12 commit fcd78b0

File tree

4 files changed

+16
-5
lines changed

4 files changed

+16
-5
lines changed

workspaces/arborist/lib/arborist/build-ideal-tree.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,8 @@ This is a one-time fix-up, please be patient...
809809
const crackOpen = this.#complete &&
810810
node !== this.idealTree &&
811811
node.resolved &&
812-
(hasBundle || hasShrinkwrap)
812+
(hasBundle || hasShrinkwrap) &&
813+
!node.ideallyInert
813814
if (crackOpen) {
814815
const Arborist = this.constructor
815816
const opt = { ...this.options }
@@ -1514,7 +1515,7 @@ This is a one-time fix-up, please be patient...
15141515
#idealTreePrune () {
15151516
for (const node of this.idealTree.inventory.values()) {
15161517
if (node.extraneous) {
1517-
node.parent = null
1518+
node.parent = false;
15181519
}
15191520
}
15201521
}
@@ -1527,7 +1528,7 @@ This is a one-time fix-up, please be patient...
15271528

15281529
const set = optionalSet(node)
15291530
for (const node of set) {
1530-
node.parent = null
1531+
node.ideallyInert = true;
15311532
}
15321533
}
15331534
}
@@ -1548,6 +1549,7 @@ This is a one-time fix-up, please be patient...
15481549
node.parent !== null
15491550
&& !node.isProjectRoot
15501551
&& !excludeNodes.has(node)
1552+
&& !node.ideallyInert
15511553
) {
15521554
this[_addNodeToTrashList](node)
15531555
}

workspaces/arborist/lib/arborist/isolated-reifier.js

+1
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ module.exports = cls => class IsolatedReifier extends cls {
100100
}
101101

102102
async externalProxy (result, node) {
103+
if (node.ideallyInert) { return; }
103104
await this.assignCommonProperties(node, result)
104105
if (node.hasShrinkwrap) {
105106
const dir = join(

workspaces/arborist/lib/arborist/reify.js

+7-2
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ module.exports = cls => class Reifier extends cls {
150150
for (const path of this[_trashList]) {
151151
const loc = relpath(this.idealTree.realpath, path)
152152
const node = this.idealTree.inventory.get(loc)
153-
if (node && node.root === this.idealTree) {
153+
if (node && node.root === this.idealTree && !node.ideallyInert) {
154154
node.parent = null
155155
}
156156
}
@@ -600,6 +600,7 @@ module.exports = cls => class Reifier extends cls {
600600
// retire the same path at the same time.
601601
const dirsChecked = new Set()
602602
return promiseAllRejectLate(leaves.map(async node => {
603+
if (node.ideallyInert) { return; }
603604
for (const d of walkUp(node.path)) {
604605
if (d === node.top.path) {
605606
break
@@ -744,6 +745,8 @@ module.exports = cls => class Reifier extends cls {
744745
}
745746

746747
async #extractOrLink (node) {
748+
if (node.ideallyInert) { return; }
749+
747750
const nm = resolve(node.parent.path, 'node_modules')
748751
await this.#validateNodeModules(nm)
749752

@@ -819,6 +822,7 @@ module.exports = cls => class Reifier extends cls {
819822
const set = optionalSet(node)
820823
for (node of set) {
821824
log.verbose('reify', 'failed optional dependency', node.path)
825+
node.ideallyInert = true;
822826
this[_addNodeToTrashList](node)
823827
}
824828
}) : p).then(() => node)
@@ -1151,6 +1155,7 @@ module.exports = cls => class Reifier extends cls {
11511155

11521156
this.#retiredUnchanged[retireFolder] = []
11531157
return promiseAllRejectLate(diff.unchanged.map(node => {
1158+
if (node.ideallyInert) { return; }
11541159
// no need to roll back links, since we'll just delete them anyway
11551160
if (node.isLink) {
11561161
return mkdir(dirname(node.path), { recursive: true, force: true })
@@ -1230,7 +1235,7 @@ module.exports = cls => class Reifier extends cls {
12301235
// skip links that only live within node_modules as they are most
12311236
// likely managed by packages we installed, we only want to rebuild
12321237
// unchanged links we directly manage
1233-
const linkedFromRoot = node.parent === tree || node.target.fsTop === tree
1238+
const linkedFromRoot = (node.parent === tree && !node.ideallyInert) || node.target.fsTop === tree
12341239
if (node.isLink && linkedFromRoot) {
12351240
nodes.push(node)
12361241
}

workspaces/arborist/lib/node.js

+3
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class Node {
103103
global = false,
104104
dummy = false,
105105
sourceReference = null,
106+
ideallyInert = false,
106107
} = options
107108
// this object gives querySelectorAll somewhere to stash context about a node
108109
// while processing a query
@@ -197,6 +198,8 @@ class Node {
197198
this.extraneous = false
198199
}
199200

201+
this.ideallyInert = ideallyInert
202+
200203
this.edgesIn = new Set()
201204
this.edgesOut = new CaseInsensitiveMap()
202205

0 commit comments

Comments
 (0)