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

Commit

Permalink
Do not ERESOLVE on peerOptionals not added to tree
Browse files Browse the repository at this point in the history
For these dependency graphs:

    # case a
    root -> (x, y@1)
    x -> PEEROPTIONAL(z)
    z -> PEER(y@2)

    # case b
    root -> (x) PEEROPTIONAL(y@1)
    x -> PEEROPTIONAL(y@2)

    # case c
    root -> (x) PEEROPTIONAL(y@1)
    x -> PEER(z)
    z -> PEEROPTIONAL(y@2)

The peerOptional dependency is included in the peerSet, which would
raise an ERESOLVE conflict at the peerSet generation stage, even though
the peerOptional dependencies will not actually be added to the tree.

To address this, this commit tracks the nodes which are actually
required in the peerSet generation phase, by virtue of being
non-optionally depended upon by a required node in the peerSet.

If a conflict occurs on a node which is not in the required set during
the peerSet generation phase, we ignore it in much the same way that we
would ignore peerSet errors in metadependencies or when --force is used.

Of course, if the peerOptional dependency is _actually_ required, to
avoid a conflict with an invalid resolution present in the tree already,
and there is no suitable placement for it, then ERESOLVE will still be
raised.

Fix: #223
Fix: npm/cli#2667
  • Loading branch information
isaacs committed Feb 11, 2021
1 parent ffb80ee commit 40277a0
Show file tree
Hide file tree
Showing 36 changed files with 1,332 additions and 0 deletions.
388 changes: 388 additions & 0 deletions tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12476,6 +12476,394 @@ ArboristNode {
}
`

exports[`test/arborist/build-ideal-tree.js TAP do not ERESOLVE on peerOptionals that are ignored anyway case a > must match snapshot 1`] = `
&ref_1 Node {
"children": Map {
"@isaacs/testing-peer-optional-conflict-a-x" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-a-z" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-GYIjOCQ5BFWl0caylWvjYOLrs0JZq0SkAY0WhNegXPXehob14/DjVD3LxwUWIKzgbhfr345ig46H0jFEmdV66Q==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-a-x",
"name": "@isaacs/testing-peer-optional-conflict-a-x",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-x",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-x",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-a-x/-/testing-peer-optional-conflict-a-x-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
"@isaacs/testing-peer-optional-conflict-a-y" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-v8/s8BZTLE28MF+s+Mh9IgGsnM68xkZm1Gy3GjXwEoMmUKmhZdVq+s4AgyE5zvMjKwNXStLzfLlI0fm5mbmUUw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-a-y",
"name": "@isaacs/testing-peer-optional-conflict-a-y",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-y",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-y",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-a-y/-/testing-peer-optional-conflict-a-y-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-a-x" => Edge {},
"@isaacs/testing-peer-optional-conflict-a-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": null,
"inventory": Inventory {
"" => <*ref_1>,
"node_modules/@isaacs/testing-peer-optional-conflict-a-x" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-a-z" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-GYIjOCQ5BFWl0caylWvjYOLrs0JZq0SkAY0WhNegXPXehob14/DjVD3LxwUWIKzgbhfr345ig46H0jFEmdV66Q==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-a-x",
"name": "@isaacs/testing-peer-optional-conflict-a-x",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-x",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-x",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-a-x/-/testing-peer-optional-conflict-a-x-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
"node_modules/@isaacs/testing-peer-optional-conflict-a-y" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-v8/s8BZTLE28MF+s+Mh9IgGsnM68xkZm1Gy3GjXwEoMmUKmhZdVq+s4AgyE5zvMjKwNXStLzfLlI0fm5mbmUUw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-a-y",
"name": "@isaacs/testing-peer-optional-conflict-a-y",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-y",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/a/node_modules/@isaacs/testing-peer-optional-conflict-a-y",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-a-y/-/testing-peer-optional-conflict-a-y-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "",
"name": "a",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/a",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/a",
"resolved": null,
"sourceReference": null,
"tops": Set {},
}
`

exports[`test/arborist/build-ideal-tree.js TAP do not ERESOLVE on peerOptionals that are ignored anyway case b > must match snapshot 1`] = `
&ref_1 Node {
"children": Map {
"@isaacs/testing-peer-optional-conflict-b-x" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-b-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-19feHJAqq5HG2RrN2zHXiPXujXqHgJuGyZaKFolghgYKyncZYRpBwxboSDgMp8SxyrGx9EF74x6yarrVXRo9hw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-b-x",
"name": "@isaacs/testing-peer-optional-conflict-b-x",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/b/node_modules/@isaacs/testing-peer-optional-conflict-b-x",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/b/node_modules/@isaacs/testing-peer-optional-conflict-b-x",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-b-x/-/testing-peer-optional-conflict-b-x-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-b-x" => Edge {},
"@isaacs/testing-peer-optional-conflict-b-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": null,
"inventory": Inventory {
"" => <*ref_1>,
"node_modules/@isaacs/testing-peer-optional-conflict-b-x" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-b-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-19feHJAqq5HG2RrN2zHXiPXujXqHgJuGyZaKFolghgYKyncZYRpBwxboSDgMp8SxyrGx9EF74x6yarrVXRo9hw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-b-x",
"name": "@isaacs/testing-peer-optional-conflict-b-x",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/b/node_modules/@isaacs/testing-peer-optional-conflict-b-x",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/b/node_modules/@isaacs/testing-peer-optional-conflict-b-x",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-b-x/-/testing-peer-optional-conflict-b-x-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "",
"name": "b",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/b",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/b",
"resolved": null,
"sourceReference": null,
"tops": Set {},
}
`

exports[`test/arborist/build-ideal-tree.js TAP do not ERESOLVE on peerOptionals that are ignored anyway case c > must match snapshot 1`] = `
&ref_1 Node {
"children": Map {
"@isaacs/testing-peer-optional-conflict-c-x" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-c-z" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-wvByt0plqaMZBxfIgbqUSc/RRnfk83Zn5I/OpuV/NrZ89Bo0mqao22CSM3I1v72czur4B91tC/vTZyzLYl1Anw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-c-x",
"name": "@isaacs/testing-peer-optional-conflict-c-x",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-x",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-x",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-c-x/-/testing-peer-optional-conflict-c-x-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
"@isaacs/testing-peer-optional-conflict-c-z" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-c-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-pgnul6UIJ7QYIfchXrZ9WwYhqpXvPwote3j7emH7LZyDeXyUbSWPUYR6Cc0NlBZIfB20Ia3J6KcxdZBOG5y6zw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-c-z",
"name": "@isaacs/testing-peer-optional-conflict-c-z",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-z",
"peer": true,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-z",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-c-z/-/testing-peer-optional-conflict-c-z-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-c-x" => Edge {},
"@isaacs/testing-peer-optional-conflict-c-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": null,
"inventory": Inventory {
"" => <*ref_1>,
"node_modules/@isaacs/testing-peer-optional-conflict-c-x" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-c-z" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-wvByt0plqaMZBxfIgbqUSc/RRnfk83Zn5I/OpuV/NrZ89Bo0mqao22CSM3I1v72czur4B91tC/vTZyzLYl1Anw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-c-x",
"name": "@isaacs/testing-peer-optional-conflict-c-x",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-x",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-x",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-c-x/-/testing-peer-optional-conflict-c-x-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
"node_modules/@isaacs/testing-peer-optional-conflict-c-z" => Node {
"children": Map {},
"dev": false,
"devOptional": false,
"dummy": false,
"edgesIn": Set {
Edge {},
},
"edgesOut": Map {
"@isaacs/testing-peer-optional-conflict-c-y" => Edge {},
},
"errors": Array [],
"extraneous": false,
"fsChildren": Set {},
"hasShrinkwrap": false,
"integrity": "sha512-pgnul6UIJ7QYIfchXrZ9WwYhqpXvPwote3j7emH7LZyDeXyUbSWPUYR6Cc0NlBZIfB20Ia3J6KcxdZBOG5y6zw==",
"inventory": Inventory {},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "node_modules/@isaacs/testing-peer-optional-conflict-c-z",
"name": "@isaacs/testing-peer-optional-conflict-c-z",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-z",
"peer": true,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/c/node_modules/@isaacs/testing-peer-optional-conflict-c-z",
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-optional-conflict-c-z/-/testing-peer-optional-conflict-c-z-1.0.0.tgz",
"sourceReference": null,
"tops": Set {},
},
},
"legacyPeerDeps": false,
"linksIn": Set {},
"location": "",
"name": "c",
"optional": false,
"path": "{CWD}/test/fixtures/peer-optional-eresolve/c",
"peer": false,
"realpath": "{CWD}/test/fixtures/peer-optional-eresolve/c",
"resolved": null,
"sourceReference": null,
"tops": Set {},
}
`

exports[`test/arborist/build-ideal-tree.js TAP do not add shrinkwrapped deps > expect resolving Promise 1`] = `
ArboristNode {
"children": Map {
Expand Down
Loading

0 comments on commit 40277a0

Please sign in to comment.