Skip to content

Commit 5b7c0cc

Browse files
fix(arborist): exclude store nodes from :root > * in linked strategy (#9096)
1 parent 3b70a9d commit 5b7c0cc

2 files changed

Lines changed: 94 additions & 2 deletions

File tree

workspaces/arborist/lib/query-selector-all.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -793,9 +793,14 @@ const hasParent = (node, compareNodes) => {
793793
compareNode = compareNode.target
794794
}
795795

796-
// follows logical parent for link ancestors
796+
// Follows logical parent for link ancestors (e.g. workspaces whose target lives outside node_modules).
797+
// Only match if the node has a link whose parent is the compareNode. Without this check, nodes deep in the store (linked strategy) would incorrectly match as children of root via their fsParent chain.
797798
if (node.isTop && (node.resolveParent === compareNode)) {
798-
return true
799+
for (const link of node.linksIn) {
800+
if (link.parent === compareNode) {
801+
return true
802+
}
803+
}
799804
}
800805
// follows edges-in to check if they match a possible parent
801806
for (const edge of node.edgesIn) {

workspaces/arborist/test/query-selector-all.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,3 +1036,90 @@ t.test('query-selector-all', async t => {
10361036
['#b *', ['a@1.0.0', 'bar@2.0.0', 'baz@1.0.0', 'lorem@1.0.0', 'moo@3.0.0']],
10371037
])
10381038
})
1039+
1040+
// Simulates the linked install strategy layout where packages live in node_modules/.store/ and are symlinked from node_modules/.
1041+
t.test('linked strategy: :root > * excludes transitive deps and store nodes', async t => {
1042+
/*
1043+
fixture tree (linked strategy layout):
1044+
1045+
linked-test@1.0.0
1046+
├── nopt@7.2.1 (symlink -> .store/nopt-hash/node_modules/nopt)
1047+
├── ini@4.1.3 (symlink -> .store/ini-hash/node_modules/ini)
1048+
└── .store/
1049+
├── nopt-hash/
1050+
│ └── node_modules/
1051+
│ ├── nopt@7.2.1 (actual)
1052+
│ └── abbrev (symlink -> ../../abbrev-hash/node_modules/abbrev)
1053+
├── ini-hash/
1054+
│ └── node_modules/
1055+
│ └── ini@4.1.3 (actual)
1056+
└── abbrev-hash/
1057+
└── node_modules/
1058+
└── abbrev@2.0.0 (actual)
1059+
*/
1060+
1061+
const path = t.testdir({
1062+
node_modules: {
1063+
nopt: t.fixture('symlink', '.store/nopt-hash/node_modules/nopt'),
1064+
ini: t.fixture('symlink', '.store/ini-hash/node_modules/ini'),
1065+
'.store': {
1066+
'nopt-hash': {
1067+
node_modules: {
1068+
nopt: {
1069+
'package.json': JSON.stringify({
1070+
name: 'nopt',
1071+
version: '7.2.1',
1072+
dependencies: { abbrev: '^2.0.0' },
1073+
}),
1074+
},
1075+
abbrev: t.fixture('symlink', '../../abbrev-hash/node_modules/abbrev'),
1076+
},
1077+
},
1078+
'ini-hash': {
1079+
node_modules: {
1080+
ini: {
1081+
'package.json': JSON.stringify({
1082+
name: 'ini',
1083+
version: '4.1.3',
1084+
}),
1085+
},
1086+
},
1087+
},
1088+
'abbrev-hash': {
1089+
node_modules: {
1090+
abbrev: {
1091+
'package.json': JSON.stringify({
1092+
name: 'abbrev',
1093+
version: '2.0.0',
1094+
}),
1095+
},
1096+
},
1097+
},
1098+
},
1099+
},
1100+
'package.json': JSON.stringify({
1101+
name: 'linked-test',
1102+
version: '1.0.0',
1103+
dependencies: {
1104+
nopt: '^7.0.0',
1105+
ini: '^4.0.0',
1106+
},
1107+
}),
1108+
})
1109+
1110+
const arb = new Arborist({ path })
1111+
const tree = await arb.loadActual()
1112+
1113+
const rootChildren = await querySelectorAll(tree, ':root > *')
1114+
t.same(rootChildren, [
1115+
'ini@4.1.3',
1116+
'nopt@7.2.1',
1117+
], ':root > * should only return direct dependencies, not transitive deps or store nodes')
1118+
1119+
const rootDescendants = await querySelectorAll(tree, ':root *')
1120+
t.same(rootDescendants, [
1121+
'abbrev@2.0.0',
1122+
'ini@4.1.3',
1123+
'nopt@7.2.1',
1124+
], ':root * should return all descendants including transitive deps')
1125+
})

0 commit comments

Comments
 (0)