Skip to content

Instantly share code, notes, and snippets.

@isaacs

isaacs/gist:96a4b2c79060117ff9bfd38f06dc9e59

Created October 13, 2021 21:29
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save isaacs/96a4b2c79060117ff9bfd38f06dc9e59 to your computer and use it in GitHub Desktop.
Save isaacs/96a4b2c79060117ff9bfd38f06dc9e59 to your computer and use it in GitHub Desktop.
Download ZIP
Raw
gistfile.patch
diff --git a/lib/can-place-dep.js b/lib/can-place-dep.js
index 6be59093..3291b6fb 100644
--- a/lib/can-place-dep.js
+++ b/lib/can-place-dep.js
@@ -145,7 +145,9 @@ class CanPlaceDep {
return CONFLICT
}
- if (targetEdge && !dep.satisfies(targetEdge) && targetEdge !== this.edge) {
+ // skip this test if there's a current node, because we might be able
+ // to dedupe against it anyway
+ if (!current && targetEdge && !dep.satisfies(targetEdge) && targetEdge !== this.edge) {
return CONFLICT
}
@@ -167,10 +169,10 @@ class CanPlaceDep {
const { version: newVer } = dep
const tryReplace = curVer && newVer && semver.gte(newVer, curVer)
if (tryReplace && dep.canReplace(current)) {
- /* XXX-istanbul ignore else - It's extremely rare that a replaceable
- * node would be a conflict, if the current one wasn't a conflict,
- * but it is theoretically possible if peer deps are pinned. In
- * that case we treat it like any other conflict, and keep trying */
+ // It's extremely rare that a replaceable node would be a conflict, if
+ // the current one wasn't a conflict, but it is theoretically possible
+ // if peer deps are pinned. In that case we treat it like any other
+ // conflict, and keep trying.
const cpp = this.canPlacePeers(REPLACE)
if (cpp !== CONFLICT) {
return cpp
diff --git a/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs b/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
index b5363796..6c11017a 100644
--- a/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
+++ b/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
@@ -56865,21 +56865,21 @@ ArboristNode {
"type": "peer",
},
EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-import",
+ "from": "node_modules/standard/node_modules/eslint-plugin-es",
"name": "eslint",
- "spec": "2.x - 6.x",
+ "spec": ">=4.19.1",
"type": "peer",
},
EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-node",
+ "from": "node_modules/standard/node_modules/eslint-plugin-import",
"name": "eslint",
- "spec": ">=5.16.0",
+ "spec": "2.x - 6.x",
"type": "peer",
},
EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/eslint-plugin-es",
+ "from": "node_modules/standard/node_modules/eslint-plugin-node",
"name": "eslint",
- "spec": ">=4.19.1",
+ "spec": ">=5.16.0",
"type": "peer",
},
EdgeIn {
@@ -57197,6 +57197,60 @@ ArboristNode {
"resolved": "https://registry.npmjs.org/eslint-config-standard-jsx/-/eslint-config-standard-jsx-8.1.0.tgz",
"version": "8.1.0",
},
+ "eslint-plugin-es" => ArboristNode {
+ "children": Map {
+ "regexpp" => ArboristNode {
+ "dev": true,
+ "edgesIn": Set {
+ EdgeIn {
+ "from": "node_modules/standard/node_modules/eslint-plugin-es",
+ "name": "regexpp",
+ "spec": "^3.0.0",
+ "type": "prod",
+ },
+ },
+ "location": "node_modules/standard/node_modules/eslint-plugin-es/node_modules/regexpp",
+ "name": "regexpp",
+ "path": "{CWD}/test/fixtures/yargs/node_modules/standard/node_modules/eslint-plugin-es/node_modules/regexpp",
+ "resolved": "https://registry.npmjs.org/regexpp/-/regexpp-3.1.0.tgz",
+ "version": "3.1.0",
+ },
+ },
+ "dev": true,
+ "edgesIn": Set {
+ EdgeIn {
+ "from": "node_modules/standard/node_modules/eslint-plugin-node",
+ "name": "eslint-plugin-es",
+ "spec": "^2.0.0",
+ "type": "prod",
+ },
+ },
+ "edgesOut": Map {
+ "eslint" => EdgeOut {
+ "name": "eslint",
+ "spec": ">=4.19.1",
+ "to": "node_modules/standard/node_modules/eslint",
+ "type": "peer",
+ },
+ "eslint-utils" => EdgeOut {
+ "name": "eslint-utils",
+ "spec": "^1.4.2",
+ "to": "node_modules/standard/node_modules/eslint-utils",
+ "type": "prod",
+ },
+ "regexpp" => EdgeOut {
+ "name": "regexpp",
+ "spec": "^3.0.0",
+ "to": "node_modules/standard/node_modules/eslint-plugin-es/node_modules/regexpp",
+ "type": "prod",
+ },
+ },
+ "location": "node_modules/standard/node_modules/eslint-plugin-es",
+ "name": "eslint-plugin-es",
+ "path": "{CWD}/test/fixtures/yargs/node_modules/standard/node_modules/eslint-plugin-es",
+ "resolved": "https://registry.npmjs.org/eslint-plugin-es/-/eslint-plugin-es-2.0.0.tgz",
+ "version": "2.0.0",
+ },
"eslint-plugin-import" => ArboristNode {
"children": Map {
"debug" => ArboristNode {
@@ -57351,42 +57405,6 @@ ArboristNode {
},
"eslint-plugin-node" => ArboristNode {
"children": Map {
- "eslint-plugin-es" => ArboristNode {
- "dev": true,
- "edgesIn": Set {
- EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-node",
- "name": "eslint-plugin-es",
- "spec": "^2.0.0",
- "type": "prod",
- },
- },
- "edgesOut": Map {
- "eslint" => EdgeOut {
- "name": "eslint",
- "spec": ">=4.19.1",
- "to": "node_modules/standard/node_modules/eslint",
- "type": "peer",
- },
- "eslint-utils" => EdgeOut {
- "name": "eslint-utils",
- "spec": "^1.4.2",
- "to": "node_modules/standard/node_modules/eslint-utils",
- "type": "prod",
- },
- "regexpp" => EdgeOut {
- "name": "regexpp",
- "spec": "^3.0.0",
- "to": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/regexpp",
- "type": "prod",
- },
- },
- "location": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/eslint-plugin-es",
- "name": "eslint-plugin-es",
- "path": "{CWD}/test/fixtures/yargs/node_modules/standard/node_modules/eslint-plugin-node/node_modules/eslint-plugin-es",
- "resolved": "https://registry.npmjs.org/eslint-plugin-es/-/eslint-plugin-es-2.0.0.tgz",
- "version": "2.0.0",
- },
"ignore" => ArboristNode {
"dev": true,
"edgesIn": Set {
@@ -57403,22 +57421,6 @@ ArboristNode {
"resolved": "https://registry.npmjs.org/ignore/-/ignore-5.1.8.tgz",
"version": "5.1.8",
},
- "regexpp" => ArboristNode {
- "dev": true,
- "edgesIn": Set {
- EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/eslint-plugin-es",
- "name": "regexpp",
- "spec": "^3.0.0",
- "type": "prod",
- },
- },
- "location": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/regexpp",
- "name": "regexpp",
- "path": "{CWD}/test/fixtures/yargs/node_modules/standard/node_modules/eslint-plugin-node/node_modules/regexpp",
- "resolved": "https://registry.npmjs.org/regexpp/-/regexpp-3.1.0.tgz",
- "version": "3.1.0",
- },
"semver" => ArboristNode {
"dev": true,
"edgesIn": Set {
@@ -57461,7 +57463,7 @@ ArboristNode {
"eslint-plugin-es" => EdgeOut {
"name": "eslint-plugin-es",
"spec": "^2.0.0",
- "to": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/eslint-plugin-es",
+ "to": "node_modules/standard/node_modules/eslint-plugin-es",
"type": "prod",
},
"eslint-utils" => EdgeOut {
@@ -57621,13 +57623,13 @@ ArboristNode {
"type": "prod",
},
EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-node",
+ "from": "node_modules/standard/node_modules/eslint-plugin-es",
"name": "eslint-utils",
"spec": "^1.4.2",
"type": "prod",
},
EdgeIn {
- "from": "node_modules/standard/node_modules/eslint-plugin-node/node_modules/eslint-plugin-es",
+ "from": "node_modules/standard/node_modules/eslint-plugin-node",
"name": "eslint-utils",
"spec": "^1.4.2",
"type": "prod",
diff --git a/tap-snapshots/test/can-place-dep.js.test.cjs b/tap-snapshots/test/can-place-dep.js.test.cjs
index 2bca9535..73d87f6d 100644
--- a/tap-snapshots/test/can-place-dep.js.test.cjs
+++ b/tap-snapshots/test/can-place-dep.js.test.cjs
@@ -13,6 +13,10 @@ exports[`test/can-place-dep.js TAP basic placement check tests basic placement w
Array []
`
+exports[`test/can-place-dep.js TAP basic placement check tests can dedupe, cannot place peer > conflict children 1`] = `
+Array []
+`
+
exports[`test/can-place-dep.js TAP basic placement check tests cannot place peer inside of dependent > conflict children 1`] = `
Array []
`
diff --git a/test/can-place-dep.js b/test/can-place-dep.js
index 2f6bb812..cb1de7f5 100644
--- a/test/can-place-dep.js
+++ b/test/can-place-dep.js
@@ -1356,6 +1356,35 @@ t.test('basic placement check tests', t => {
expect: OK,
})
+ // root -> (k, y@1)
+ // k -> (x)
+ // x -> PEER(y@1||2)
+ //
+ // root
+ // +-- y@1
+ // +-- k@1
+ //
+ // place x in root with y@2 in peerset
+ runTest('can dedupe, cannot place peer', {
+ tree: new Node({
+ path,
+ pkg: { dependencies: { k: '1', y: '1' }},
+ children: [
+ { pkg: { name: 'y', version: '1.0.0' }},
+ { pkg: { name: 'k', version: '1.0.0', dependencies: { x: '' }}},
+ ],
+ }),
+ dep: new Node({
+ pkg: { name: 'x', version: '1.0.0', peerDependencies: { y: '1||2' }},
+ }),
+ peerSet: [
+ { pkg: { name: 'y', version: '2.0.0' }},
+ ],
+ targetLoc: '',
+ nodeLoc: 'node_modules/k',
+ expect: OK,
+ })
+
t.end()
})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment