diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js index 32e523cbc83ca..83a9c18cb6aac 100644 --- a/workspaces/arborist/lib/edge.js +++ b/workspaces/arborist/lib/edge.js @@ -275,7 +275,7 @@ class Edge { this.#error = 'PEER LOCAL' } else if (!this.satisfiedBy(this.#to)) { this.#error = 'INVALID' - } else if (this.overrides && this.#to.edgesOut.size && OverrideSet.doOverrideSetsConflict(this.overrides, this.#to.overrides)) { + } else if (this.overrides && this.#to.overrides && this.#to.edgesOut.size && OverrideSet.doOverrideSetsConflict(this.overrides, this.#to.overrides)) { // Check for conflicts between the edge's override set and the target node's override set. // This catches cases where different parts of the tree have genuinely incompatible // version requirements for the same package. diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js index b4a11ba589df7..7dc876d44c922 100644 --- a/workspaces/arborist/lib/override-set.js +++ b/workspaces/arborist/lib/override-set.js @@ -200,6 +200,11 @@ class OverrideSet { } static doOverrideSetsConflict (first, second) { + // If either override set is missing, there's no conflict + if (!first || !second) { + return false + } + // If override sets contain one another then we can try to use the more specific one. // If neither one is more specific, check for semantic conflicts. const specificSet = this.findSpecificOverrideSet(first, second) diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index cf5e2e3326539..e3f6204da9556 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -1244,3 +1244,91 @@ t.test('override fallback to local when root missing dependency with from.overri t.equal(edge.spec, '^1.2.3', 'should fall back to local package version from devDependencies') t.end() }) + +t.test('edge with overrides should not crash when target has no overrides', t => { + const root = { + name: 'root', + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => 'root', + package: { + name: 'root', + version: '1.0.0', + overrides: { + react: '^18.3.1', + 'react-dom': '^18.3.1', + }, + }, + get version () { + return this.package.version + }, + isTop: true, + parent: null, + overrides: new OverrideSet({ + overrides: { + react: '^18.3.1', + 'react-dom': '^18.3.1', + }, + }), + resolve (n) { + return n === 'dep' ? dep : null + }, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, + } + + // Target node with dependencies but NO overrides + const dep = { + name: 'dep', + edgesOut: new Map([ + // Has dependencies so edgesOut.size > 0 + ['some-lib', { name: 'some-lib', peer: false }], + ]), + edgesIn: new Set(), + explain: () => 'dep', + package: { name: 'dep', version: '1.0.0' }, + get version () { + return this.package.version + }, + parent: root, + root, + // NO overrides property - this causes this.#to.overrides to be undefined + resolve () { + return null + }, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, + } + + // Create an edge from root to dep + // Root has overrides, dep has dependencies but no overrides + const edge = new Edge({ + from: root, + type: 'prod', + spec: '^1.0.0', + name: 'dep', + overrides: root.overrides, + }) + + t.doesNotThrow(() => { + const err = edge.error + t.equal(err, null, 'edge should be valid (no error)') + }, 'should not crash when target node has no overrides') + + t.ok(edge.valid, 'edge should be valid') + t.end() +}) diff --git a/workspaces/arborist/test/override-set.js b/workspaces/arborist/test/override-set.js index 8b8be32c9bb81..85265e645ef0e 100644 --- a/workspaces/arborist/test/override-set.js +++ b/workspaces/arborist/test/override-set.js @@ -558,4 +558,24 @@ t.test('coverage for isEqual edge cases', async t => { // AND parent.isEqual(...) returns false t.notOk(childC.isEqual(childD), 'two children with different parents are not equal') }) + + t.test('should handle undefined/null second parameter gracefully', async (t) => { + const overrides = new OverrideSet({ + overrides: { + react: '^18.3.1', + 'react-dom': '^18.3.1', + }, + }) + + t.doesNotThrow(() => { + OverrideSet.doOverrideSetsConflict(overrides, undefined) + }, 'should not throw when second parameter is undefined') + + t.doesNotThrow(() => { + OverrideSet.doOverrideSetsConflict(overrides, null) + }, 'should not throw when second parameter is null') + + t.equal(OverrideSet.doOverrideSetsConflict(overrides, undefined), false, 'should return false for undefined') + t.equal(OverrideSet.doOverrideSetsConflict(overrides, null), false, 'should return false for null') + }) })