Skip to content
This repository was archived by the owner on Jan 20, 2022. It is now read-only.

Commit 9410d16

Browse files
isaacsisaacs
committed
Fix additional peerOptional conflict cases
case D ``` root -> (x, z@1) x -> PEEROPTIONAL(y) y -> PEER(z@2) ``` case E ``` root -> (x, z@1) x -> PEEROPTIONAL(y) PEER(z@1) y -> PEER(z@2) ``` case F ``` root -> (x, z@1) x -> PEEROPTIONAL(y) PEER(z@1) PEER(w@1) w -> PEER(z@1) y -> PEER(z@2) ``` Case F properly reproduces the failure seen by installing ng-packagr@11.1.3 alongside typescript@4. Fix: #236 cc: @kyliau
1 parent 7d5cb3d commit 9410d16

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+1370
-3
lines changed

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -1121,8 +1121,11 @@ This is a one-time fix-up, please be patient...
11211121
// we don't like it. always fail strictly, always allow forcibly or
11221122
// in non-strict mode if it's not our fault. don't warn here, because
11231123
// we are going to warn again when we place the deps, if we end up
1124-
// overriding for something else.
1125-
if (conflictOK)
1124+
// overriding for something else. If the thing that has this dep
1125+
// isn't also required, then there's a good chance we won't need it,
1126+
// so allow it for now and let it conflict if it turns out to actually
1127+
// be necessary for the installation.
1128+
if (conflictOK || !required.has(edge.from))
11261129
continue
11271130

11281131
// ok, it's the root, or we're in unforced strict mode, so this is bad

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

+252
Original file line numberDiff line numberDiff line change
@@ -13531,6 +13531,258 @@ ArboristNode {
1353113531
}
1353213532
`
1353313533

13534+
exports[`test/arborist/build-ideal-tree.js TAP do not ERESOLVE on peerOptionals that are ignored anyway case d > must match snapshot 1`] = `
13535+
ArboristNode {
13536+
"children": Map {
13537+
"@isaacs/testing-peer-optional-conflict-d-x" => ArboristNode {
13538+
"edgesIn": Set {
13539+
EdgeIn {
13540+
"from": "",
13541+
"name": "@isaacs/testing-peer-optional-conflict-d-x",
13542+
"spec": "1",
13543+
"type": "prod",
13544+
},
13545+
},
13546+
"edgesOut": Map {
13547+
"@isaacs/testing-peer-optional-conflict-d-y" => EdgeOut {
13548+
"name": "@isaacs/testing-peer-optional-conflict-d-y",
13549+
"spec": "1",
13550+
"to": null,
13551+
"type": "peerOptional",
13552+
},
13553+
},
13554+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-d-x",
13555+
"name": "@isaacs/testing-peer-optional-conflict-d-x",
13556+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/d/node_modules/@isaacs/testing-peer-optional-conflict-d-x",
13557+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-d-x/-/testing-peer-optional-conflict-d-x-1.0.0.tgz",
13558+
"version": "1.0.0",
13559+
},
13560+
"@isaacs/testing-peer-optional-conflict-d-z" => ArboristNode {
13561+
"edgesIn": Set {
13562+
EdgeIn {
13563+
"from": "",
13564+
"name": "@isaacs/testing-peer-optional-conflict-d-z",
13565+
"spec": "1",
13566+
"type": "prod",
13567+
},
13568+
},
13569+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-d-z",
13570+
"name": "@isaacs/testing-peer-optional-conflict-d-z",
13571+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/d/node_modules/@isaacs/testing-peer-optional-conflict-d-z",
13572+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-d-z/-/testing-peer-optional-conflict-d-z-1.0.0.tgz",
13573+
"version": "1.0.0",
13574+
},
13575+
},
13576+
"edgesOut": Map {
13577+
"@isaacs/testing-peer-optional-conflict-d-x" => EdgeOut {
13578+
"name": "@isaacs/testing-peer-optional-conflict-d-x",
13579+
"spec": "1",
13580+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-d-x",
13581+
"type": "prod",
13582+
},
13583+
"@isaacs/testing-peer-optional-conflict-d-z" => EdgeOut {
13584+
"name": "@isaacs/testing-peer-optional-conflict-d-z",
13585+
"spec": "1",
13586+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-d-z",
13587+
"type": "prod",
13588+
},
13589+
},
13590+
"location": "",
13591+
"name": "d",
13592+
"packageName": "@isaacs/testing-peer-optional-conflict-d",
13593+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/d",
13594+
"version": "1.0.0",
13595+
}
13596+
`
13597+
13598+
exports[`test/arborist/build-ideal-tree.js TAP do not ERESOLVE on peerOptionals that are ignored anyway case e > must match snapshot 1`] = `
13599+
ArboristNode {
13600+
"children": Map {
13601+
"@isaacs/testing-peer-optional-conflict-e-x" => ArboristNode {
13602+
"edgesIn": Set {
13603+
EdgeIn {
13604+
"from": "",
13605+
"name": "@isaacs/testing-peer-optional-conflict-e-x",
13606+
"spec": "1",
13607+
"type": "prod",
13608+
},
13609+
},
13610+
"edgesOut": Map {
13611+
"@isaacs/testing-peer-optional-conflict-e-y" => EdgeOut {
13612+
"name": "@isaacs/testing-peer-optional-conflict-e-y",
13613+
"spec": "1",
13614+
"to": null,
13615+
"type": "peerOptional",
13616+
},
13617+
"@isaacs/testing-peer-optional-conflict-e-z" => EdgeOut {
13618+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
13619+
"spec": "1",
13620+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-e-z",
13621+
"type": "peer",
13622+
},
13623+
},
13624+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-e-x",
13625+
"name": "@isaacs/testing-peer-optional-conflict-e-x",
13626+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/e/node_modules/@isaacs/testing-peer-optional-conflict-e-x",
13627+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-e-x/-/testing-peer-optional-conflict-e-x-1.0.0.tgz",
13628+
"version": "1.0.0",
13629+
},
13630+
"@isaacs/testing-peer-optional-conflict-e-z" => ArboristNode {
13631+
"edgesIn": Set {
13632+
EdgeIn {
13633+
"from": "",
13634+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
13635+
"spec": "1",
13636+
"type": "prod",
13637+
},
13638+
EdgeIn {
13639+
"from": "node_modules/@isaacs/testing-peer-optional-conflict-e-x",
13640+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
13641+
"spec": "1",
13642+
"type": "peer",
13643+
},
13644+
},
13645+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-e-z",
13646+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
13647+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/e/node_modules/@isaacs/testing-peer-optional-conflict-e-z",
13648+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-e-z/-/testing-peer-optional-conflict-e-z-1.0.0.tgz",
13649+
"version": "1.0.0",
13650+
},
13651+
},
13652+
"edgesOut": Map {
13653+
"@isaacs/testing-peer-optional-conflict-e-x" => EdgeOut {
13654+
"name": "@isaacs/testing-peer-optional-conflict-e-x",
13655+
"spec": "1",
13656+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-e-x",
13657+
"type": "prod",
13658+
},
13659+
"@isaacs/testing-peer-optional-conflict-e-z" => EdgeOut {
13660+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
13661+
"spec": "1",
13662+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-e-z",
13663+
"type": "prod",
13664+
},
13665+
},
13666+
"location": "",
13667+
"name": "e",
13668+
"packageName": "@isaacs/testing-peer-optional-conflict-e",
13669+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/e",
13670+
"version": "1.0.0",
13671+
}
13672+
`
13673+
13674+
exports[`test/arborist/build-ideal-tree.js TAP do not ERESOLVE on peerOptionals that are ignored anyway case f > must match snapshot 1`] = `
13675+
ArboristNode {
13676+
"children": Map {
13677+
"@isaacs/testing-peer-optional-conflict-f-w" => ArboristNode {
13678+
"edgesIn": Set {
13679+
EdgeIn {
13680+
"from": "node_modules/@isaacs/testing-peer-optional-conflict-f-x",
13681+
"name": "@isaacs/testing-peer-optional-conflict-f-w",
13682+
"spec": "1",
13683+
"type": "peer",
13684+
},
13685+
},
13686+
"edgesOut": Map {
13687+
"@isaacs/testing-peer-optional-conflict-f-z" => EdgeOut {
13688+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13689+
"spec": "1",
13690+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-f-z",
13691+
"type": "peer",
13692+
},
13693+
},
13694+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-f-w",
13695+
"name": "@isaacs/testing-peer-optional-conflict-f-w",
13696+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/f/node_modules/@isaacs/testing-peer-optional-conflict-f-w",
13697+
"peer": true,
13698+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-f-w/-/testing-peer-optional-conflict-f-w-1.0.0.tgz",
13699+
"version": "1.0.0",
13700+
},
13701+
"@isaacs/testing-peer-optional-conflict-f-x" => ArboristNode {
13702+
"edgesIn": Set {
13703+
EdgeIn {
13704+
"from": "",
13705+
"name": "@isaacs/testing-peer-optional-conflict-f-x",
13706+
"spec": "1",
13707+
"type": "prod",
13708+
},
13709+
},
13710+
"edgesOut": Map {
13711+
"@isaacs/testing-peer-optional-conflict-f-w" => EdgeOut {
13712+
"name": "@isaacs/testing-peer-optional-conflict-f-w",
13713+
"spec": "1",
13714+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-f-w",
13715+
"type": "peer",
13716+
},
13717+
"@isaacs/testing-peer-optional-conflict-f-y" => EdgeOut {
13718+
"name": "@isaacs/testing-peer-optional-conflict-f-y",
13719+
"spec": "1",
13720+
"to": null,
13721+
"type": "peerOptional",
13722+
},
13723+
"@isaacs/testing-peer-optional-conflict-f-z" => EdgeOut {
13724+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13725+
"spec": "1",
13726+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-f-z",
13727+
"type": "peer",
13728+
},
13729+
},
13730+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-f-x",
13731+
"name": "@isaacs/testing-peer-optional-conflict-f-x",
13732+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/f/node_modules/@isaacs/testing-peer-optional-conflict-f-x",
13733+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-f-x/-/testing-peer-optional-conflict-f-x-1.0.0.tgz",
13734+
"version": "1.0.0",
13735+
},
13736+
"@isaacs/testing-peer-optional-conflict-f-z" => ArboristNode {
13737+
"edgesIn": Set {
13738+
EdgeIn {
13739+
"from": "",
13740+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13741+
"spec": "1",
13742+
"type": "prod",
13743+
},
13744+
EdgeIn {
13745+
"from": "node_modules/@isaacs/testing-peer-optional-conflict-f-w",
13746+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13747+
"spec": "1",
13748+
"type": "peer",
13749+
},
13750+
EdgeIn {
13751+
"from": "node_modules/@isaacs/testing-peer-optional-conflict-f-x",
13752+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13753+
"spec": "1",
13754+
"type": "peer",
13755+
},
13756+
},
13757+
"location": "node_modules/@isaacs/testing-peer-optional-conflict-f-z",
13758+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13759+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/f/node_modules/@isaacs/testing-peer-optional-conflict-f-z",
13760+
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-f-z/-/testing-peer-optional-conflict-f-z-1.0.0.tgz",
13761+
"version": "1.0.0",
13762+
},
13763+
},
13764+
"edgesOut": Map {
13765+
"@isaacs/testing-peer-optional-conflict-f-x" => EdgeOut {
13766+
"name": "@isaacs/testing-peer-optional-conflict-f-x",
13767+
"spec": "1",
13768+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-f-x",
13769+
"type": "prod",
13770+
},
13771+
"@isaacs/testing-peer-optional-conflict-f-z" => EdgeOut {
13772+
"name": "@isaacs/testing-peer-optional-conflict-f-z",
13773+
"spec": "1",
13774+
"to": "node_modules/@isaacs/testing-peer-optional-conflict-f-z",
13775+
"type": "prod",
13776+
},
13777+
},
13778+
"location": "",
13779+
"name": "f",
13780+
"packageName": "@isaacs/testing-peer-optional-conflict-f",
13781+
"path": "{CWD}/test/fixtures/peer-optional-eresolve/f",
13782+
"version": "1.0.0",
13783+
}
13784+
`
13785+
1353413786
exports[`test/arborist/build-ideal-tree.js TAP do not add shrinkwrapped deps > expect resolving Promise 1`] = `
1353513787
ArboristNode {
1353613788
"children": Map {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -2508,7 +2508,7 @@ t.test('do not ERESOLVE on peerOptionals that are ignored anyway', t => {
25082508
// this simulates three cases where a conflict occurs during the peerSet
25092509
// generation phase, but will not manifest in the tree building phase.
25102510
const base = resolve(fixtures, 'peer-optional-eresolve')
2511-
const cases = ['a', 'b', 'c']
2511+
const cases = ['a', 'b', 'c', 'd', 'e', 'f']
25122512
t.plan(cases.length)
25132513
for (const c of cases) {
25142514
t.test(`case ${c}`, async t => {

‎test/fixtures/peer-optional-eresolve/d/README.md

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# peer optional failure D
2+
3+
```
4+
root -> (x, z@1)
5+
x -> PEEROPTIONAL(y)
6+
y -> PEER(z@2)
7+
```
8+
9+
[npm/arborist#223](https://github.com/npm/arborist/issues/223)
10+
11+
[npm/arborist#236](https://github.com/npm/arborist/issues/236)
12+
13+
Subtly similar to case A, but note y and z swapped, so that they are
14+
resolved in the reverse order (because deps are alphabetically sorted for
15+
consistency). This is relevant, because it ensures that there's no dep
16+
present in the peerSet until _after_ the conflict is encountered, and that
17+
code path was not being hit.

‎test/fixtures/peer-optional-eresolve/d/package.json

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-d",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"@isaacs/testing-peer-optional-conflict-d-x": "1",
6+
"@isaacs/testing-peer-optional-conflict-d-z": "1"
7+
}
8+
}

‎test/fixtures/peer-optional-eresolve/d/x/package.json

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-d-x",
3+
"version": "1.0.0",
4+
"peerDependencies": {
5+
"@isaacs/testing-peer-optional-conflict-d-y": "1"
6+
},
7+
"peerDependenciesMeta": {
8+
"@isaacs/testing-peer-optional-conflict-d-y": {
9+
"optional": true
10+
}
11+
}
12+
}

‎test/fixtures/peer-optional-eresolve/d/y/package.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-d-y",
3+
"version": "1.0.0",
4+
"peerDependencies": {
5+
"@isaacs/testing-peer-optional-conflict-d-z": "2"
6+
}
7+
}

‎test/fixtures/peer-optional-eresolve/d/z/1/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-d-z",
3+
"version": "1.0.0"
4+
}

‎test/fixtures/peer-optional-eresolve/d/z/2/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-d-z",
3+
"version": "2.0.0"
4+
}

‎test/fixtures/peer-optional-eresolve/e/README.md

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# peer optional failure E
2+
3+
```
4+
root -> (x, z@1)
5+
x -> PEEROPTIONAL(y) PEER(z@1)
6+
y -> PEER(z@2)
7+
```
8+
9+
[npm/arborist#223](https://github.com/npm/arborist/issues/223)
10+
11+
[npm/arborist#236](https://github.com/npm/arborist/issues/236)
12+
13+
The same as case D, but with the addition of a `x->z` peerDep.

‎test/fixtures/peer-optional-eresolve/e/package.json

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-e",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"@isaacs/testing-peer-optional-conflict-e-x": "1",
6+
"@isaacs/testing-peer-optional-conflict-e-z": "1"
7+
}
8+
}

‎test/fixtures/peer-optional-eresolve/e/x/package.json

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-e-x",
3+
"version": "1.0.0",
4+
"peerDependencies": {
5+
"@isaacs/testing-peer-optional-conflict-e-y": "1",
6+
"@isaacs/testing-peer-optional-conflict-e-z": "1"
7+
},
8+
"peerDependenciesMeta": {
9+
"@isaacs/testing-peer-optional-conflict-e-y": {
10+
"optional": true
11+
}
12+
}
13+
}

‎test/fixtures/peer-optional-eresolve/e/y/package.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-e-y",
3+
"version": "1.0.0",
4+
"peerDependencies": {
5+
"@isaacs/testing-peer-optional-conflict-e-z": "2"
6+
}
7+
}

‎test/fixtures/peer-optional-eresolve/e/z/1/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
3+
"version": "1.0.0"
4+
}

‎test/fixtures/peer-optional-eresolve/e/z/2/package.json

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "@isaacs/testing-peer-optional-conflict-e-z",
3+
"version": "2.0.0"
4+
}

0 commit comments

Comments
 (0)