Skip to content

Commit 208c06e

Browse files
milaninfymilaninfy
authored
fix: peer edge crash due to no parent or detached node (#8448)
`node.parent` is resulting in `null` value somewhere in recursive/looped calls to `#loadPeerSet` method in building ideal tree. When circular peer ref of a top level dependency replaced/resolved with different compatible version, it makes top level dependency to be removed from it's parent ( node.parent = null ) since it's been replaced, so no longer need to proceed with further peer set exploration. fixes: #8261
1 parent 9e5abf1 commit 208c06e

File tree

12 files changed

+661
-0
lines changed

12 files changed

+661
-0
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,12 @@ This is a one-time fix-up, please be patient...
13191319
.sort(({ name: a }, { name: b }) => localeCompare(a, b))
13201320

13211321
for (const edge of peerEdges) {
1322+
// node.parent gets mutated during loop execution due to recursive #nodeFromEdge calls.
1323+
// When a compatible peer is found (e.g. a@1.1.0 replaces a@1.2.0), the original node loses its parent.
1324+
// if node is detached/removed from the tree, or has no parent, so no need to check remaining edgesOut for that node.
1325+
if (!node.parent) {
1326+
break
1327+
}
13221328
// already placed this one, and we're happy with it.
13231329
if (edge.valid && edge.to) {
13241330
continue

‎workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74672,6 +74672,315 @@ exports[`test/arborist/build-ideal-tree.js TAP more peer dep conflicts metadeps
7467274672
Array []
7467374673
`
7467474674

74675+
exports[`test/arborist/build-ideal-tree.js TAP more peer dep conflicts peerDep replacement of top level dep with different version resulting detached top level dep > default result 1`] = `
74676+
ArboristNode {
74677+
"children": Map {
74678+
"@test/a" => ArboristNode {
74679+
"dev": true,
74680+
"edgesIn": Set {
74681+
EdgeIn {
74682+
"from": "",
74683+
"name": "@test/a",
74684+
"spec": "^1.1.0",
74685+
"type": "dev",
74686+
},
74687+
EdgeIn {
74688+
"from": "node_modules/@test/b",
74689+
"name": "@test/a",
74690+
"spec": "1.1.0",
74691+
"type": "peer",
74692+
},
74693+
},
74694+
"edgesOut": Map {
74695+
"@test/b" => EdgeOut {
74696+
"name": "@test/b",
74697+
"spec": "1.1.0",
74698+
"to": "node_modules/@test/b",
74699+
"type": "peerOptional",
74700+
},
74701+
"@test/c" => EdgeOut {
74702+
"name": "@test/c",
74703+
"spec": "1.1.0",
74704+
"to": null,
74705+
"type": "peerOptional",
74706+
},
74707+
"lodash" => EdgeOut {
74708+
"name": "lodash",
74709+
"spec": "^4.17.0",
74710+
"to": null,
74711+
"type": "peerOptional",
74712+
},
74713+
"uniq" => EdgeOut {
74714+
"name": "uniq",
74715+
"spec": "^1.0.0",
74716+
"to": null,
74717+
"type": "peerOptional",
74718+
},
74719+
},
74720+
"location": "node_modules/@test/a",
74721+
"name": "@test/a",
74722+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep/node_modules/@test/a",
74723+
"resolved": "http://localhost:4873/@test/a/-/a-1.1.0.tgz",
74724+
"version": "1.1.0",
74725+
},
74726+
"@test/b" => ArboristNode {
74727+
"dev": true,
74728+
"edgesIn": Set {
74729+
EdgeIn {
74730+
"from": "",
74731+
"name": "@test/b",
74732+
"spec": "1.1.0",
74733+
"type": "dev",
74734+
},
74735+
EdgeIn {
74736+
"from": "node_modules/@test/a",
74737+
"name": "@test/b",
74738+
"spec": "1.1.0",
74739+
"type": "peerOptional",
74740+
},
74741+
},
74742+
"edgesOut": Map {
74743+
"@test/a" => EdgeOut {
74744+
"name": "@test/a",
74745+
"spec": "1.1.0",
74746+
"to": "node_modules/@test/a",
74747+
"type": "peer",
74748+
},
74749+
},
74750+
"location": "node_modules/@test/b",
74751+
"name": "@test/b",
74752+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep/node_modules/@test/b",
74753+
"resolved": "http://localhost:4873/@test/b/-/b-1.1.0.tgz",
74754+
"version": "1.1.0",
74755+
},
74756+
},
74757+
"edgesOut": Map {
74758+
"@test/a" => EdgeOut {
74759+
"name": "@test/a",
74760+
"spec": "^1.1.0",
74761+
"to": "node_modules/@test/a",
74762+
"type": "dev",
74763+
},
74764+
"@test/b" => EdgeOut {
74765+
"name": "@test/b",
74766+
"spec": "1.1.0",
74767+
"to": "node_modules/@test/b",
74768+
"type": "dev",
74769+
},
74770+
},
74771+
"isProjectRoot": true,
74772+
"location": "",
74773+
"name": "tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep",
74774+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep",
74775+
}
74776+
`
74777+
74778+
exports[`test/arborist/build-ideal-tree.js TAP more peer dep conflicts peerDep replacement of top level dep with different version resulting detached top level dep > force result 1`] = `
74779+
ArboristNode {
74780+
"children": Map {
74781+
"@test/a" => ArboristNode {
74782+
"dev": true,
74783+
"edgesIn": Set {
74784+
EdgeIn {
74785+
"from": "",
74786+
"name": "@test/a",
74787+
"spec": "^1.1.0",
74788+
"type": "dev",
74789+
},
74790+
EdgeIn {
74791+
"from": "node_modules/@test/b",
74792+
"name": "@test/a",
74793+
"spec": "1.1.0",
74794+
"type": "peer",
74795+
},
74796+
},
74797+
"edgesOut": Map {
74798+
"@test/b" => EdgeOut {
74799+
"name": "@test/b",
74800+
"spec": "1.1.0",
74801+
"to": "node_modules/@test/b",
74802+
"type": "peerOptional",
74803+
},
74804+
"@test/c" => EdgeOut {
74805+
"name": "@test/c",
74806+
"spec": "1.1.0",
74807+
"to": null,
74808+
"type": "peerOptional",
74809+
},
74810+
"lodash" => EdgeOut {
74811+
"name": "lodash",
74812+
"spec": "^4.17.0",
74813+
"to": null,
74814+
"type": "peerOptional",
74815+
},
74816+
"uniq" => EdgeOut {
74817+
"name": "uniq",
74818+
"spec": "^1.0.0",
74819+
"to": null,
74820+
"type": "peerOptional",
74821+
},
74822+
},
74823+
"location": "node_modules/@test/a",
74824+
"name": "@test/a",
74825+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep/node_modules/@test/a",
74826+
"resolved": "http://localhost:4873/@test/a/-/a-1.1.0.tgz",
74827+
"version": "1.1.0",
74828+
},
74829+
"@test/b" => ArboristNode {
74830+
"dev": true,
74831+
"edgesIn": Set {
74832+
EdgeIn {
74833+
"from": "",
74834+
"name": "@test/b",
74835+
"spec": "1.1.0",
74836+
"type": "dev",
74837+
},
74838+
EdgeIn {
74839+
"from": "node_modules/@test/a",
74840+
"name": "@test/b",
74841+
"spec": "1.1.0",
74842+
"type": "peerOptional",
74843+
},
74844+
},
74845+
"edgesOut": Map {
74846+
"@test/a" => EdgeOut {
74847+
"name": "@test/a",
74848+
"spec": "1.1.0",
74849+
"to": "node_modules/@test/a",
74850+
"type": "peer",
74851+
},
74852+
},
74853+
"location": "node_modules/@test/b",
74854+
"name": "@test/b",
74855+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep/node_modules/@test/b",
74856+
"resolved": "http://localhost:4873/@test/b/-/b-1.1.0.tgz",
74857+
"version": "1.1.0",
74858+
},
74859+
},
74860+
"edgesOut": Map {
74861+
"@test/a" => EdgeOut {
74862+
"name": "@test/a",
74863+
"spec": "^1.1.0",
74864+
"to": "node_modules/@test/a",
74865+
"type": "dev",
74866+
},
74867+
"@test/b" => EdgeOut {
74868+
"name": "@test/b",
74869+
"spec": "1.1.0",
74870+
"to": "node_modules/@test/b",
74871+
"type": "dev",
74872+
},
74873+
},
74874+
"isProjectRoot": true,
74875+
"location": "",
74876+
"name": "tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep",
74877+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep",
74878+
}
74879+
`
74880+
74881+
exports[`test/arborist/build-ideal-tree.js TAP more peer dep conflicts peerDep replacement of top level dep with different version resulting detached top level dep > strict result 1`] = `
74882+
ArboristNode {
74883+
"children": Map {
74884+
"@test/a" => ArboristNode {
74885+
"dev": true,
74886+
"edgesIn": Set {
74887+
EdgeIn {
74888+
"from": "",
74889+
"name": "@test/a",
74890+
"spec": "^1.1.0",
74891+
"type": "dev",
74892+
},
74893+
EdgeIn {
74894+
"from": "node_modules/@test/b",
74895+
"name": "@test/a",
74896+
"spec": "1.1.0",
74897+
"type": "peer",
74898+
},
74899+
},
74900+
"edgesOut": Map {
74901+
"@test/b" => EdgeOut {
74902+
"name": "@test/b",
74903+
"spec": "1.1.0",
74904+
"to": "node_modules/@test/b",
74905+
"type": "peerOptional",
74906+
},
74907+
"@test/c" => EdgeOut {
74908+
"name": "@test/c",
74909+
"spec": "1.1.0",
74910+
"to": null,
74911+
"type": "peerOptional",
74912+
},
74913+
"lodash" => EdgeOut {
74914+
"name": "lodash",
74915+
"spec": "^4.17.0",
74916+
"to": null,
74917+
"type": "peerOptional",
74918+
},
74919+
"uniq" => EdgeOut {
74920+
"name": "uniq",
74921+
"spec": "^1.0.0",
74922+
"to": null,
74923+
"type": "peerOptional",
74924+
},
74925+
},
74926+
"location": "node_modules/@test/a",
74927+
"name": "@test/a",
74928+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep/node_modules/@test/a",
74929+
"resolved": "http://localhost:4873/@test/a/-/a-1.1.0.tgz",
74930+
"version": "1.1.0",
74931+
},
74932+
"@test/b" => ArboristNode {
74933+
"dev": true,
74934+
"edgesIn": Set {
74935+
EdgeIn {
74936+
"from": "",
74937+
"name": "@test/b",
74938+
"spec": "1.1.0",
74939+
"type": "dev",
74940+
},
74941+
EdgeIn {
74942+
"from": "node_modules/@test/a",
74943+
"name": "@test/b",
74944+
"spec": "1.1.0",
74945+
"type": "peerOptional",
74946+
},
74947+
},
74948+
"edgesOut": Map {
74949+
"@test/a" => EdgeOut {
74950+
"name": "@test/a",
74951+
"spec": "1.1.0",
74952+
"to": "node_modules/@test/a",
74953+
"type": "peer",
74954+
},
74955+
},
74956+
"location": "node_modules/@test/b",
74957+
"name": "@test/b",
74958+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep/node_modules/@test/b",
74959+
"resolved": "http://localhost:4873/@test/b/-/b-1.1.0.tgz",
74960+
"version": "1.1.0",
74961+
},
74962+
},
74963+
"edgesOut": Map {
74964+
"@test/a" => EdgeOut {
74965+
"name": "@test/a",
74966+
"spec": "^1.1.0",
74967+
"to": "node_modules/@test/a",
74968+
"type": "dev",
74969+
},
74970+
"@test/b" => EdgeOut {
74971+
"name": "@test/b",
74972+
"spec": "1.1.0",
74973+
"to": "node_modules/@test/b",
74974+
"type": "dev",
74975+
},
74976+
},
74977+
"isProjectRoot": true,
74978+
"location": "",
74979+
"name": "tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep",
74980+
"path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-more-peer-dep-conflicts-peerDep-replacement-of-top-level-dep-with-different-version-resulting-detached-top-level-dep",
74981+
}
74982+
`
74983+
7467574984
exports[`test/arborist/build-ideal-tree.js TAP more peer dep conflicts prod dep directly on conflicted peer, full peer set, newer > force result 1`] = `
7467674985
ArboristNode {
7467774986
"children": Map {

‎workspaces/arborist/test/arborist/build-ideal-tree.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,6 +1655,16 @@ t.test('more peer dep conflicts', async t => {
16551655
error: false,
16561656
resolvable: true,
16571657
},
1658+
'peerDep replacement of top level dep with different version resulting detached top level dep': {
1659+
pkg: {
1660+
description: 'a@ -> (PeerOptional(b, c, dep, dep)) b -> ( Peer(a) ) c -> ( Peer(a) )',
1661+
devDependencies: {
1662+
'@test/a': '^1.1.0',
1663+
'@test/b': '1.1.0',
1664+
},
1665+
},
1666+
error: false,
1667+
resolvable: true },
16581668
})
16591669

16601670
createRegistry(t, true)

0 commit comments

Comments
 (0)