Skip to content

Commit d389614

Browse files
owlstronautowlstronaut
committed
fix: corrects peer dependency flag propagation
1 parent 7a09902 commit d389614

File tree

12 files changed

+1519
-462
lines changed

12 files changed

+1519
-462
lines changed

‎package-lock.json‎

Lines changed: 30 additions & 192 deletions
Large diffs are not rendered by default.

‎tap-snapshots/workspaces/arborist/test/calc-dep-flags.js.test.cjs‎

Lines changed: 809 additions & 0 deletions
Large diffs are not rendered by default.

‎workspaces/arborist/lib/calc-dep-flags.js‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const calcDepFlagsStep = (node) => {
2222
// or normal dependency graphs overlap deep in the dep graph.
2323
// Since we're only walking through deps that are not already flagged
2424
// as non-dev/non-optional, it's typically a very shallow traversal
25+
2526
node.extraneous = false
2627
resetParents(node, 'extraneous')
2728
resetParents(node, 'dev')
@@ -47,10 +48,16 @@ const calcDepFlagsStep = (node) => {
4748
if (!to) {
4849
return
4950
}
50-
5151
// everything with any kind of edge into it is not extraneous
5252
to.extraneous = false
5353

54+
// If this is a peer edge, mark the target as peer
55+
if (peer) {
56+
to.peer = true
57+
} else if (to.peer && !hasIncomingPeerEdge(to)) {
58+
unsetFlag(to, 'peer')
59+
}
60+
5461
// devOptional is the *overlap* of the dev and optional tree.
5562
// however, for convenience and to save an extra rewalk, we leave
5663
// it set when we are in *either* tree, and then omit it from the
@@ -61,11 +68,6 @@ const calcDepFlagsStep = (node) => {
6168
// either the dev or opt trees
6269
const unsetDev = unsetDevOpt || !node.dev && !dev
6370
const unsetOpt = unsetDevOpt || !node.optional && !optional
64-
const unsetPeer = !node.peer && !peer
65-
66-
if (unsetPeer) {
67-
unsetFlag(to, 'peer')
68-
}
6971

7072
if (unsetDevOpt) {
7173
unsetFlag(to, 'devOptional')
@@ -83,6 +85,16 @@ const calcDepFlagsStep = (node) => {
8385
return node
8486
}
8587

88+
const hasIncomingPeerEdge = (node) => {
89+
const target = node.isLink && node.target ? node.target : node
90+
for (const edge of target.edgesIn) {
91+
if (edge.type === 'peer') {
92+
return true
93+
}
94+
}
95+
return false
96+
}
97+
8698
const resetParents = (node, flag) => {
8799
if (node[flag]) {
88100
return
@@ -109,12 +121,19 @@ const unsetFlag = (node, flag) => {
109121
const children = []
110122
const targetNode = node.isLink && node.target ? node.target : node
111123
for (const edge of targetNode.edgesOut.values()) {
112-
if (
113-
edge.to &&
114-
edge.to[flag] &&
115-
((flag !== 'peer' && edge.type === 'peer') || edge.type === 'prod')
116-
) {
117-
children.push(edge.to)
124+
if (edge.to?.[flag]) {
125+
// For the peer flag, only follow peer edges to unset the flag
126+
// Don't propagate peer flag through prod/dev/optional edges
127+
if (flag === 'peer') {
128+
if (edge.type === 'peer') {
129+
children.push(edge.to)
130+
}
131+
} else {
132+
// For other flags, follow prod edges (and peer edges for non-peer flags)
133+
if (edge.type === 'prod' || edge.type === 'peer') {
134+
children.push(edge.to)
135+
}
136+
}
118137
}
119138
}
120139
return children

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

Lines changed: 168 additions & 250 deletions
Large diffs are not rendered by default.

‎workspaces/arborist/tap-snapshots/test/arborist/load-actual.js.test.cjs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ ArboristNode {
280280
"location": "node_modules/@scope/y",
281281
"name": "@scope/y",
282282
"path": "root/node_modules/@scope/y",
283+
"peer": true,
283284
"version": "1.2.3",
284285
},
285286
"foo" => ArboristNode {
@@ -869,6 +870,7 @@ ArboristLink {
869870
"location": "node_modules/@scope/y",
870871
"name": "@scope/y",
871872
"path": "root/node_modules/@scope/y",
873+
"peer": true,
872874
"version": "1.2.3",
873875
},
874876
"foo" => ArboristNode {
@@ -2699,6 +2701,7 @@ ArboristLink {
26992701
"location": "node_modules/@scope/y",
27002702
"name": "@scope/y",
27012703
"path": "root/node_modules/@scope/y",
2704+
"peer": true,
27022705
"version": "1.2.3",
27032706
},
27042707
"foo" => ArboristNode {
@@ -4428,6 +4431,7 @@ ArboristNode {
44284431
"location": "node_modules/@scope/y",
44294432
"name": "@scope/y",
44304433
"path": "root/node_modules/@scope/y",
4434+
"peer": true,
44314435
"version": "1.2.3",
44324436
},
44334437
"foo" => ArboristNode {
@@ -6072,6 +6076,7 @@ ArboristNode {
60726076
"location": "node_modules/@scope/y",
60736077
"name": "@scope/y",
60746078
"path": "root/node_modules/@scope/y",
6079+
"peer": true,
60756080
"version": "1.2.3",
60766081
},
60776082
"foo" => ArboristNode {

‎workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,6 @@ ArboristNode {
303303
"location": "node_modules/wrappy",
304304
"name": "wrappy",
305305
"path": "{CWD}/test/fixtures/edit-package-json/changed/node_modules/wrappy",
306-
"peer": true,
307306
"resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz",
308307
"version": "1.0.2",
309308
},

‎workspaces/arborist/tap-snapshots/test/arborist/pruner.js.test.cjs‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,99 @@
55
* Make sure to inspect the output below. Do not ignore changes!
66
*/
77
'use strict'
8+
exports[`test/arborist/pruner.js TAP do not prune dependencies that are optional but not peer > must match snapshot 1`] = `
9+
ArboristNode {
10+
"children": Map {
11+
"optional-dep" => ArboristNode {
12+
"edgesIn": Set {
13+
EdgeIn {
14+
"from": "node_modules/peer-pkg",
15+
"name": "optional-dep",
16+
"spec": "1.0.0",
17+
"type": "optional",
18+
},
19+
},
20+
"location": "node_modules/optional-dep",
21+
"name": "optional-dep",
22+
"optional": true,
23+
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer/node_modules/optional-dep",
24+
"version": "1.0.0",
25+
},
26+
"peer-pkg" => ArboristNode {
27+
"edgesIn": Set {
28+
EdgeIn {
29+
"from": "",
30+
"name": "peer-pkg",
31+
"spec": "1.0.0",
32+
"type": "peer",
33+
},
34+
EdgeIn {
35+
"from": "node_modules/pkg-a",
36+
"name": "peer-pkg",
37+
"spec": "1.0.0",
38+
"type": "peer",
39+
},
40+
},
41+
"edgesOut": Map {
42+
"optional-dep" => EdgeOut {
43+
"name": "optional-dep",
44+
"spec": "1.0.0",
45+
"to": "node_modules/optional-dep",
46+
"type": "optional",
47+
},
48+
},
49+
"location": "node_modules/peer-pkg",
50+
"name": "peer-pkg",
51+
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer/node_modules/peer-pkg",
52+
"peer": true,
53+
"version": "1.0.0",
54+
},
55+
"pkg-a" => ArboristNode {
56+
"edgesIn": Set {
57+
EdgeIn {
58+
"from": "",
59+
"name": "pkg-a",
60+
"spec": "1.0.0",
61+
"type": "prod",
62+
},
63+
},
64+
"edgesOut": Map {
65+
"peer-pkg" => EdgeOut {
66+
"name": "peer-pkg",
67+
"spec": "1.0.0",
68+
"to": "node_modules/peer-pkg",
69+
"type": "peer",
70+
},
71+
},
72+
"location": "node_modules/pkg-a",
73+
"name": "pkg-a",
74+
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer/node_modules/pkg-a",
75+
"version": "1.0.0",
76+
},
77+
},
78+
"edgesOut": Map {
79+
"peer-pkg" => EdgeOut {
80+
"name": "peer-pkg",
81+
"spec": "1.0.0",
82+
"to": "node_modules/peer-pkg",
83+
"type": "peer",
84+
},
85+
"pkg-a" => EdgeOut {
86+
"name": "pkg-a",
87+
"spec": "1.0.0",
88+
"to": "node_modules/pkg-a",
89+
"type": "prod",
90+
},
91+
},
92+
"isProjectRoot": true,
93+
"location": "",
94+
"name": "tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer",
95+
"packageName": "peer-optional-test",
96+
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer",
97+
"version": "1.0.0",
98+
}
99+
`
100+
8101
exports[`test/arborist/pruner.js TAP prune with actual tree > must match snapshot 1`] = `
9102
ArboristNode {
10103
"isProjectRoot": true,

‎workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10483,6 +10483,7 @@ ArboristNode {
1048310483
"location": "node_modules/react",
1048410484
"name": "react",
1048510485
"path": "{CWD}/test/arborist/tap-testdir-reify-multiple-bundles-at-the-same-level/node_modules/react",
10486+
"peer": true,
1048610487
"resolved": "https://registry.npmjs.org/react/-/react-16.12.0.tgz",
1048710488
"version": "16.12.0",
1048810489
},
@@ -11876,6 +11877,7 @@ ArboristNode {
1187611877
"location": "node_modules/tap/node_modules/@babel/core",
1187711878
"name": "@babel/core",
1187811879
"path": "{CWD}/test/arborist/tap-testdir-reify-multiple-bundles-at-the-same-level/node_modules/tap/node_modules/@babel/core",
11880+
"peer": true,
1187911881
"version": "7.7.5",
1188011882
},
1188111883
"@babel/generator" => ArboristNode {
@@ -12770,6 +12772,7 @@ ArboristNode {
1277012772
"location": "node_modules/tap/node_modules/@types/react",
1277112773
"name": "@types/react",
1277212774
"path": "{CWD}/test/arborist/tap-testdir-reify-multiple-bundles-at-the-same-level/node_modules/tap/node_modules/@types/react",
12775+
"peer": true,
1277312776
"version": "16.9.16",
1277412777
},
1277512778
"ansi-escapes" => ArboristNode {
@@ -18724,6 +18727,7 @@ ArboristNode {
1872418727
"location": "node_modules/ajv",
1872518728
"name": "ajv",
1872618729
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/ajv",
18730+
"peer": true,
1872718731
"resolved": "https://registry.npmjs.org/ajv/-/ajv-4.11.2.tgz",
1872818732
"version": "4.11.2",
1872918733
},
@@ -21590,6 +21594,7 @@ ArboristNode {
2159021594
"location": "node_modules/eslint",
2159121595
"name": "eslint",
2159221596
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint",
21597+
"peer": true,
2159321598
"resolved": "https://registry.npmjs.org/eslint/-/eslint-3.10.2.tgz",
2159421599
"version": "3.10.2",
2159521600
},
@@ -21678,6 +21683,7 @@ ArboristNode {
2167821683
"location": "node_modules/eslint-plugin-promise",
2167921684
"name": "eslint-plugin-promise",
2168021685
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint-plugin-promise",
21686+
"peer": true,
2168121687
"resolved": "https://registry.npmjs.org/eslint-plugin-promise/-/eslint-plugin-promise-3.4.1.tgz",
2168221688
"version": "3.4.1",
2168321689
},
@@ -21720,6 +21726,7 @@ ArboristNode {
2172021726
"location": "node_modules/eslint-plugin-react",
2172121727
"name": "eslint-plugin-react",
2172221728
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint-plugin-react",
21729+
"peer": true,
2172321730
"resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-6.7.1.tgz",
2172421731
"version": "6.7.1",
2172521732
},
@@ -21750,6 +21757,7 @@ ArboristNode {
2175021757
"location": "node_modules/eslint-plugin-standard",
2175121758
"name": "eslint-plugin-standard",
2175221759
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint-plugin-standard",
21760+
"peer": true,
2175321761
"resolved": "https://registry.npmjs.org/eslint-plugin-standard/-/eslint-plugin-standard-2.0.1.tgz",
2175421762
"version": "2.0.1",
2175521763
},
@@ -34342,6 +34350,7 @@ ArboristNode {
3434234350
"location": "node_modules/@isaacs/testing-peer-deps-b",
3434334351
"name": "@isaacs/testing-peer-deps-b",
3434434352
"path": "{CWD}/test/arborist/tap-testdir-reify-testing-peer-deps-nested-with-update/node_modules/@isaacs/testing-peer-deps-b",
34353+
"peer": true,
3434534354
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-b/-/testing-peer-deps-b-2.0.1.tgz",
3434634355
"version": "2.0.1",
3434734356
},
@@ -40618,6 +40627,7 @@ ArboristNode {
4061840627
"location": "node_modules/react",
4061940628
"name": "react",
4062040629
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/react",
40630+
"peer": true,
4062140631
"resolved": "https://registry.npmjs.org/react/-/react-15.6.2.tgz",
4062240632
"version": "15.6.2",
4062340633
},
@@ -41818,6 +41828,7 @@ ArboristNode {
4181841828
"location": "node_modules/tap/node_modules/@babel/core",
4181941829
"name": "@babel/core",
4182041830
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/tap/node_modules/@babel/core",
41831+
"peer": true,
4182141832
"version": "7.7.5",
4182241833
},
4182341834
"@babel/generator" => ArboristNode {
@@ -42712,6 +42723,7 @@ ArboristNode {
4271242723
"location": "node_modules/tap/node_modules/@types/react",
4271342724
"name": "@types/react",
4271442725
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/tap/node_modules/@types/react",
42726+
"peer": true,
4271542727
"version": "16.9.16",
4271642728
},
4271742729
"ansi-escapes" => ArboristNode {
@@ -44438,6 +44450,7 @@ ArboristNode {
4443844450
"location": "node_modules/tap/node_modules/react",
4443944451
"name": "react",
4444044452
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/tap/node_modules/react",
44453+
"peer": true,
4444144454
"resolved": "https://registry.npmjs.org/react/-/react-16.12.0.tgz",
4444244455
"version": "16.12.0",
4444344456
},
@@ -46277,6 +46290,7 @@ ArboristNode {
4627746290
"location": "node_modules/typescript",
4627846291
"name": "typescript",
4627946292
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/typescript",
46293+
"peer": true,
4628046294
"resolved": "https://registry.npmjs.org/typescript/-/typescript-3.7.4.tgz",
4628146295
"version": "3.7.4",
4628246296
},

0 commit comments

Comments
 (0)