Skip to content

Commit 8a1e79d

Browse files
committed
fix: undefined override set conflicts shouldn't error
1 parent 958b10e commit 8a1e79d

File tree

4 files changed

+114
-1
lines changed

4 files changed

+114
-1
lines changed

workspaces/arborist/lib/edge.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class Edge {
275275
this.#error = 'PEER LOCAL'
276276
} else if (!this.satisfiedBy(this.#to)) {
277277
this.#error = 'INVALID'
278-
} else if (this.overrides && this.#to.edgesOut.size && OverrideSet.doOverrideSetsConflict(this.overrides, this.#to.overrides)) {
278+
} else if (this.overrides && this.#to.overrides && this.#to.edgesOut.size && OverrideSet.doOverrideSetsConflict(this.overrides, this.#to.overrides)) {
279279
// Check for conflicts between the edge's override set and the target node's override set.
280280
// This catches cases where different parts of the tree have genuinely incompatible
281281
// version requirements for the same package.

workspaces/arborist/lib/override-set.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ class OverrideSet {
200200
}
201201

202202
static doOverrideSetsConflict (first, second) {
203+
// If either override set is missing, there's no conflict
204+
if (!first || !second) {
205+
return false
206+
}
207+
203208
// If override sets contain one another then we can try to use the more specific one.
204209
// If neither one is more specific, check for semantic conflicts.
205210
const specificSet = this.findSpecificOverrideSet(first, second)

workspaces/arborist/test/edge.js

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,3 +1244,91 @@ t.test('override fallback to local when root missing dependency with from.overri
12441244
t.equal(edge.spec, '^1.2.3', 'should fall back to local package version from devDependencies')
12451245
t.end()
12461246
})
1247+
1248+
t.test('edge with overrides should not crash when target has no overrides', t => {
1249+
const root = {
1250+
name: 'root',
1251+
edgesOut: new Map(),
1252+
edgesIn: new Set(),
1253+
explain: () => 'root',
1254+
package: {
1255+
name: 'root',
1256+
version: '1.0.0',
1257+
overrides: {
1258+
react: '^18.3.1',
1259+
'react-dom': '^18.3.1',
1260+
},
1261+
},
1262+
get version () {
1263+
return this.package.version
1264+
},
1265+
isTop: true,
1266+
parent: null,
1267+
overrides: new OverrideSet({
1268+
overrides: {
1269+
react: '^18.3.1',
1270+
'react-dom': '^18.3.1',
1271+
},
1272+
}),
1273+
resolve (n) {
1274+
return n === 'dep' ? dep : null
1275+
},
1276+
addEdgeOut (edge) {
1277+
this.edgesOut.set(edge.name, edge)
1278+
},
1279+
addEdgeIn (edge) {
1280+
this.edgesIn.add(edge)
1281+
},
1282+
deleteEdgeIn (edge) {
1283+
this.edgesIn.delete(edge)
1284+
},
1285+
}
1286+
1287+
// Target node with dependencies but NO overrides
1288+
const dep = {
1289+
name: 'dep',
1290+
edgesOut: new Map([
1291+
// Has dependencies so edgesOut.size > 0
1292+
['some-lib', { name: 'some-lib', peer: false }],
1293+
]),
1294+
edgesIn: new Set(),
1295+
explain: () => 'dep',
1296+
package: { name: 'dep', version: '1.0.0' },
1297+
get version () {
1298+
return this.package.version
1299+
},
1300+
parent: root,
1301+
root,
1302+
// NO overrides property - this causes this.#to.overrides to be undefined
1303+
resolve () {
1304+
return null
1305+
},
1306+
addEdgeOut (edge) {
1307+
this.edgesOut.set(edge.name, edge)
1308+
},
1309+
addEdgeIn (edge) {
1310+
this.edgesIn.add(edge)
1311+
},
1312+
deleteEdgeIn (edge) {
1313+
this.edgesIn.delete(edge)
1314+
},
1315+
}
1316+
1317+
// Create an edge from root to dep
1318+
// Root has overrides, dep has dependencies but no overrides
1319+
const edge = new Edge({
1320+
from: root,
1321+
type: 'prod',
1322+
spec: '^1.0.0',
1323+
name: 'dep',
1324+
overrides: root.overrides,
1325+
})
1326+
1327+
t.doesNotThrow(() => {
1328+
const err = edge.error
1329+
t.equal(err, null, 'edge should be valid (no error)')
1330+
}, 'should not crash when target node has no overrides')
1331+
1332+
t.ok(edge.valid, 'edge should be valid')
1333+
t.end()
1334+
})

workspaces/arborist/test/override-set.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,4 +558,24 @@ t.test('coverage for isEqual edge cases', async t => {
558558
// AND parent.isEqual(...) returns false
559559
t.notOk(childC.isEqual(childD), 'two children with different parents are not equal')
560560
})
561+
562+
t.test('should handle undefined/null second parameter gracefully', async (t) => {
563+
const overrides = new OverrideSet({
564+
overrides: {
565+
react: '^18.3.1',
566+
'react-dom': '^18.3.1',
567+
},
568+
})
569+
570+
t.doesNotThrow(() => {
571+
OverrideSet.doOverrideSetsConflict(overrides, undefined)
572+
}, 'should not throw when second parameter is undefined')
573+
574+
t.doesNotThrow(() => {
575+
OverrideSet.doOverrideSetsConflict(overrides, null)
576+
}, 'should not throw when second parameter is null')
577+
578+
t.equal(OverrideSet.doOverrideSetsConflict(overrides, undefined), false, 'should return false for undefined')
579+
t.equal(OverrideSet.doOverrideSetsConflict(overrides, null), false, 'should return false for null')
580+
})
561581
})

0 commit comments

Comments
 (0)