diff --git a/workspaces/arborist/lib/dep-valid.js b/workspaces/arborist/lib/dep-valid.js index 4afb5e47cf111..4fab2d3ece835 100644 --- a/workspaces/arborist/lib/dep-valid.js +++ b/workspaces/arborist/lib/dep-valid.js @@ -101,7 +101,7 @@ const depValid = (child, requested, requestor) => { }) } - default: // unpossible, just being cautious + default: // impossible, just being cautious break } diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js index cc9698ad6cae7..77f351ed45789 100644 --- a/workspaces/arborist/lib/edge.js +++ b/workspaces/arborist/lib/edge.js @@ -4,6 +4,7 @@ const util = require('util') const npa = require('npm-package-arg') const depValid = require('./dep-valid.js') +const OverrideSet = require('./override-set.js') class ArboristEdge { constructor (edge) { @@ -103,7 +104,7 @@ class Edge { } satisfiedBy (node) { - if (node.name !== this.#name) { + if (node.name !== this.#name || !this.#from) { return false } @@ -112,7 +113,31 @@ class Edge { if (node.hasShrinkwrap || node.inShrinkwrap || node.inBundle) { return depValid(node, this.rawSpec, this.#accept, this.#from) } - return depValid(node, this.spec, this.#accept, this.#from) + + // If there's no override we just use the spec. + if (!this.overrides?.keySpec) { + return depValid(node, this.spec, this.#accept, this.#from) + } + // There's some override. If the target node satisfies the overriding spec + // then it's okay. + if (depValid(node, this.spec, this.#accept, this.#from)) { + return true + } + // If it doesn't, then it should at least satisfy the original spec. + if (!depValid(node, this.rawSpec, this.#accept, this.#from)) { + return false + } + // It satisfies the original spec, not the overriding spec. We need to make + // sure it doesn't use the overridden spec. + // For example: + // we might have an ^8.0.0 rawSpec, and an override that makes + // keySpec=8.23.0 and the override value spec=9.0.0. + // If the node is 9.0.0, then it's okay because it's consistent with spec. + // If the node is 8.24.0, then it's okay because it's consistent with the rawSpec. + // If the node is 8.23.0, then it's not okay because even though it's consistent + // with the rawSpec, it's also consistent with the keySpec. + // So we're looking for ^8.0.0 or 9.0.0 and not 8.23.0. + return !depValid(node, this.overrides.keySpec, this.#accept, this.#from) } // return the edge data, and an explanation of how that edge came to be here @@ -179,13 +204,18 @@ class Edge { get spec () { if (this.overrides?.value && this.overrides.value !== '*' && this.overrides.name === this.#name) { + // If this edge has the same overrides field as the source, then we're not applying an override for this edge. + if (this.overrides === this.#from?.overrides) { + return this.#spec + } + if (this.overrides.value.startsWith('$')) { const ref = this.overrides.value.slice(1) // we may be a virtual root, if we are we want to resolve reference overrides // from the real root, not the virtual one - const pkg = this.#from.sourceReference - ? this.#from.sourceReference.root.package - : this.#from.root.package + const pkg = this.#from?.sourceReference + ? this.#from?.sourceReference.root.package + : this.#from?.root?.package if (pkg.devDependencies?.[ref]) { return pkg.devDependencies[ref] } @@ -234,10 +264,15 @@ class Edge { } else { this.#error = 'MISSING' } - } else if (this.peer && this.#from === this.#to.parent && !this.#from.isTop) { + } else if (this.peer && this.#from === this.#to.parent && !this.#from?.isTop) { this.#error = 'PEER LOCAL' } else if (!this.satisfiedBy(this.#to)) { this.#error = 'INVALID' + } else if (this.overrides && this.#to.edgesOut.size && OverrideSet.doOverrideSetsConflict(this.overrides, this.#to.overrides)) { + // Any inconsistency between the edge's override set and the target's override set is potentially problematic. + // But we only say the edge is in error if the override sets are plainly conflicting. + // Note that if the target doesn't have any dependencies of their own, then this inconsistency is irrelevant. + this.#error = 'INVALID' } else { this.#error = 'OK' } @@ -250,15 +285,26 @@ class Edge { reload (hard = false) { this.#explanation = null - if (this.#from.overrides) { - this.overrides = this.#from.overrides.getEdgeRule(this) + + let needToUpdateOverrideSet = false + let newOverrideSet + let oldOverrideSet + if (this.#from?.overrides) { + newOverrideSet = this.#from.overrides.getEdgeRule(this) + if (newOverrideSet && !newOverrideSet.isEqual(this.overrides)) { + // If there's a new different override set we need to propagate it to the nodes. + // If we're deleting the override set then there's no point propagating it right now since it will be filled with another value later. + needToUpdateOverrideSet = true + oldOverrideSet = this.overrides + this.overrides = newOverrideSet + } } else { delete this.overrides } - const newTo = this.#from.resolve(this.#name) + const newTo = this.#from?.resolve(this.#name) if (newTo !== this.#to) { if (this.#to) { - this.#to.edgesIn.delete(this) + this.#to.deleteEdgeIn(this) } this.#to = newTo this.#error = null @@ -268,14 +314,19 @@ class Edge { } else if (hard) { this.#error = null } + else if (needToUpdateOverrideSet) { + // Propagate the new override set to the target node. + this.#to.updateOverridesEdgeInRemoved(oldOverrideSet) + this.#to.updateOverridesEdgeInAdded(newOverrideSet) + } } detach () { this.#explanation = null if (this.#to) { - this.#to.edgesIn.delete(this) + this.#to.deleteEdgeIn(this) } - this.#from.edgesOut.delete(this.#name) + this.#from?.edgesOut.delete(this.#name) this.#to = null this.#error = 'DETACHED' this.#from = null diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js index bdc021b7c12a9..0640c3c0f441d 100644 --- a/workspaces/arborist/lib/node.js +++ b/workspaces/arborist/lib/node.js @@ -65,6 +65,8 @@ const CaseInsensitiveMap = require('./case-insensitive-map.js') const querySelectorAll = require('./query-selector-all.js') +const log = require('proc-log') + class Node { #global #meta @@ -342,7 +344,28 @@ class Node { } get overridden () { - return !!(this.overrides && this.overrides.value && this.overrides.name === this.name) + if (!this.overrides) { + return false + } + if (!this.overrides.value) { + return false + } + if (this.overrides.name !== this.name) { + return false + } + + // The overrides rule is for a package with this name, but some override rules only apply to specific + // versions. To make sure this package was actually overridden, we check whether any edge going in + // had the rule applied to it, in which case its overrides set is different than its source node. + for (const edge of this.edgesIn) { + if (edge.overrides && edge.overrides.name === this.name && edge.overrides.value === this.version) { + if (!edge.overrides.isEqual(edge.from.overrides)) { + return true + } + } + } + + return false } get package () { @@ -820,9 +843,6 @@ class Node { target.root = root } - if (!this.overrides && this.parent && this.parent.overrides) { - this.overrides = this.parent.overrides.getNodeRule(this) - } // tree should always be valid upon root setter completion. treeCheck(this) if (this !== root) { @@ -1004,10 +1024,21 @@ class Node { return false } - // XXX need to check for two root nodes? - if (node.overrides !== this.overrides) { - return false + // If this node has no dependencies, then it's irrelevant to check the override + // rules of the replacement node. + if (this.edgesOut.size) { + // XXX need to check for two root nodes? + if (node.overrides) { + if (!node.overrides.isEqual(this.overrides)) { + return false + } + } else { + if (this.overrides) { + return false + } + } } + ignorePeers = new Set(ignorePeers) // gather up all the deps of this node and that are only depended @@ -1075,8 +1106,13 @@ class Node { return false } - // if we prefer dedupe, or if the version is greater/equal, take the other - if (preferDedupe || semver.gte(other.version, this.version)) { + // if we prefer dedupe, or if the version is equal, take the other + if (preferDedupe || semver.eq(other.version, this.version)) { + return true + } + + // if our current version isn't the result of an override, then prefer to take the greater version + if (!this.overridden && semver.gt(other.version, this.version)) { return true } @@ -1247,10 +1283,6 @@ class Node { this[_changePath](newPath) } - if (parent.overrides) { - this.overrides = parent.overrides.getNodeRule(this) - } - // clobbers anything at that path, resets all appropriate references this.root = parent.root } @@ -1344,9 +1376,87 @@ class Node { this.edgesOut.set(edge.name, edge) } - addEdgeIn (edge) { + recalculateOutEdgesOverrides () { + // For each edge out propogate the new overrides through. + for (const edge of this.edgesOut.values()) { + edge.reload(true) + if (edge.to) { + edge.to.updateOverridesEdgeInAdded(edge.overrides) + } + } + } + + updateOverridesEdgeInRemoved (otherOverrideSet) { + // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later. + if (!this.overrides || !this.overrides.isEqual(otherOverrideSet)) { + return false + } + let newOverrideSet + for (const edge of this.edgesIn) { + if (newOverrideSet && edge.overrides) { + newOverrideSet = OverrideSet.findSpecificOverrideSet(edge.overrides, newOverrideSet) + } else { + newOverrideSet = edge.overrides + } + } + if (this.overrides.isEqual(newOverrideSet)) { + return false + } + this.overrides = newOverrideSet + if (this.overrides) { + // Optimization: if there's any override set at all, then no non-extraneous node has an empty override set. So if we temporarily have no + // override set (for example, we removed all the edges in), there's no use updating all the edges out right now. Let's just wait until + // we have an actual override set later. + this.recalculateOutEdgesOverrides() + } + return true + } + + // This logic isn't perfect either. When we have two edges in that have different override sets, then we have to decide which set is correct. + // This function assumes the more specific override set is applicable, so if we have dependencies A->B->C and A->C + // and an override set that specifies what happens for C under A->B, this will work even if the new A->C edge comes along and tries to change + // the override set. + // The strictly correct logic is not to allow two edges with different overrides to point to the same node, because even if this node can satisfy + // both, one of its dependencies might need to be different depending on the edge leading to it. + // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen. + updateOverridesEdgeInAdded (otherOverrideSet) { + if (!otherOverrideSet) { + // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree. + // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field. + return false + } + if (!this.overrides) { + this.overrides = otherOverrideSet + this.recalculateOutEdgesOverrides() + return true + } + if (this.overrides.isEqual(otherOverrideSet)) { + return false + } + const newOverrideSet = OverrideSet.findSpecificOverrideSet(this.overrides, otherOverrideSet) + if (newOverrideSet) { + if (!this.overrides.isEqual(newOverrideSet)) { + this.overrides = newOverrideSet + this.recalculateOutEdgesOverrides() + return true + } + return false + } + // This is an error condition. We can only get here if the new override set is in conflict with the existing. + log.silly('Conflicting override sets', this.name) + } + + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) if (edge.overrides) { - this.overrides = edge.overrides + this.updateOverridesEdgeInRemoved(edge.overrides) + } + } + + addEdgeIn (edge) { + // We need to handle the case where the new edge in has an overrides field which is different from the current value. + if (!this.overrides || !this.overrides.isEqual(edge.overrides)) { + this.updateOverridesEdgeInAdded(edge.overrides) } this.edgesIn.add(edge) diff --git a/workspaces/arborist/lib/override-set.js b/workspaces/arborist/lib/override-set.js index bfc5a5d7906ee..13c5982b44414 100644 --- a/workspaces/arborist/lib/override-set.js +++ b/workspaces/arborist/lib/override-set.js @@ -1,5 +1,6 @@ const npa = require('npm-package-arg') const semver = require('semver') +const log = require('proc-log') class OverrideSet { constructor ({ overrides, key, parent }) { @@ -44,6 +45,43 @@ class OverrideSet { } } + childrenAreEqual (other) { + if (this.children.size !== other.children.size) { + return false + } + for (const [key] of this.children) { + if (!other.children.has(key)) { + return false + } + if (this.children.get(key).value !== other.children.get(key).value) { + return false + } + if (!this.children.get(key).childrenAreEqual(other.children.get(key))) { + return false + } + } + return true + } + + isEqual (other) { + if (this === other) { + return true + } + if (!other) { + return false + } + if (this.key !== other.key || this.value !== other.value) { + return false + } + if (!this.childrenAreEqual(other)) { + return false + } + if (!this.parent) { + return !other.parent + } + return this.parent.isEqual(other.parent) + } + getEdgeRule (edge) { for (const rule of this.ruleset.values()) { if (rule.name !== edge.name) { @@ -55,7 +93,8 @@ class OverrideSet { return rule } - let spec = npa(`${edge.name}@${edge.spec}`) + // We need to use the rawSpec here, because the spec has the overrides applied to it already. + let spec = npa(`${edge.name}@${edge.rawSpec}`) if (spec.type === 'alias') { spec = spec.subSpec } @@ -142,6 +181,28 @@ class OverrideSet { return ruleset } + + static findSpecificOverrideSet (first, second) { + for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) { + if (overrideSet.isEqual(first)) { + return second + } + } + for (let overrideSet = first; overrideSet; overrideSet = overrideSet.parent) { + if (overrideSet.isEqual(second)) { + return first + } + } + + // The override sets are incomparable. Neither one contains the other. + log.silly('Conflicting override sets', first, second) + } + + static doOverrideSetsConflict (first, second) { + // If override sets contain one another then we can try to use the more specific one. + // If neither one is more specific, then we consider them to be in conflict. + return (this.findSpecificOverrideSet(first, second) === undefined) + } } module.exports = OverrideSet diff --git a/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs b/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs index 17dc0b0c9fb0b..7b28779165d56 100644 --- a/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/edge.js.test.cjs @@ -52,6 +52,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "b" => Edge { @@ -69,6 +70,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -91,6 +93,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -122,6 +125,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -139,6 +143,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -173,6 +178,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "b" => Edge { @@ -190,6 +196,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -212,6 +219,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -243,6 +251,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -260,6 +269,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -294,6 +304,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -334,6 +345,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "aa" => Edge { @@ -351,6 +363,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "b" => Edge { @@ -368,6 +381,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -390,6 +404,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -424,6 +439,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -441,6 +457,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "b" => Edge { @@ -458,6 +475,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -480,6 +498,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -516,6 +535,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "aa" => Edge { @@ -533,6 +553,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -576,6 +597,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "aa" => Edge { @@ -593,6 +615,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -610,6 +633,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -645,6 +669,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -662,6 +687,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "b" => Edge { @@ -679,6 +705,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -701,6 +728,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -737,6 +765,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "a" => Edge { @@ -766,6 +795,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -783,6 +813,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "a" => Edge { @@ -805,6 +836,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "a" => Edge { @@ -838,6 +870,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "aa" => Edge { @@ -855,6 +888,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -877,6 +911,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -908,6 +943,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -925,6 +961,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "aa" => Edge { @@ -942,6 +979,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -964,6 +1002,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -1000,6 +1039,7 @@ Edge { "from": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "b" => Edge { @@ -1017,6 +1057,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -1039,6 +1080,7 @@ Edge { "root": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { @@ -1070,6 +1112,7 @@ Edge { "to": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set { Edge { "peerConflicted": false, @@ -1087,6 +1130,7 @@ Edge { "parent": Object { "addEdgeIn": Function addEdgeIn(edge), "addEdgeOut": Function addEdgeOut(edge), + "deleteEdgeIn": Function deleteEdgeIn(edge), "edgesIn": Set {}, "edgesOut": Map { "missing" => Edge { diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index ab08357ece359..9a2e5791b4de1 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -57,6 +57,9 @@ const top = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const a = { @@ -81,6 +84,9 @@ const a = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const b = { @@ -104,6 +110,9 @@ const b = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const bb = { @@ -127,6 +136,9 @@ const bb = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const aa = { @@ -150,6 +162,9 @@ const aa = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const c = { @@ -173,6 +188,9 @@ const c = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } t.matchSnapshot(new Edge({ @@ -364,6 +382,9 @@ const referenceTop = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, overrides: new OverrideSet({ overrides: { referenceGrandchild: '$referenceChild', @@ -403,6 +424,9 @@ const referenceChild = { this.overrides = edge.overrides this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } new Edge({ @@ -442,6 +466,9 @@ const referenceGrandchild = { this.overrides = edge.overrides this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const referenceGrandchildEdge = new Edge({ @@ -490,6 +517,9 @@ const badOverride = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, overrides: new OverrideSet({ overrides: { b: '1.x', @@ -775,6 +805,9 @@ const bundleChild = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const bundleParent = { @@ -797,6 +830,9 @@ const bundleParent = { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const bundledEdge = new Edge({ @@ -858,6 +894,9 @@ t.test('override references find the correct root', (t) => { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const foo = { @@ -885,6 +924,9 @@ t.test('override references find the correct root', (t) => { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } foo.overrides = overrides.getNodeRule(foo) @@ -915,6 +957,9 @@ t.test('override references find the correct root', (t) => { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } bar.overrides = foo.overrides.getNodeRule(bar) @@ -946,6 +991,9 @@ t.test('override references find the correct root', (t) => { addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } virtualBar.overrides = overrides @@ -999,6 +1047,9 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } const foo = { @@ -1029,6 +1080,9 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } foo.overrides = overrides.getNodeRule(foo) @@ -1058,6 +1112,9 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) addEdgeIn (edge) { this.edgesIn.add(edge) }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, } bar.overrides = foo.overrides.getNodeRule(bar) @@ -1072,3 +1129,69 @@ t.test('shrinkwrapped and bundled deps are not overridden and remain valid', (t) t.ok(edge.valid, 'edge is valid') t.end() }) + +t.test('overrideset comparison logic', (t) => { + const overrides1 = new OverrideSet({ + overrides: { + bar: '^2.0.0', + }, + }) + + const overrides2 = new OverrideSet({ + overrides: { + bar: '^2.0.0', + }, + }) + + const overrides3 = new OverrideSet({ + overrides: { + foo: '^2.0.0', + }, + }) + + const overrides4 = new OverrideSet({ + overrides: { + foo: '^1.0.0', + }, + }) + + const overrides5 = new OverrideSet({ + overrides: { + bar: '^2.0.0', + foo: '^2.0.0', + }, + }) + + const overrides6 = new OverrideSet({ + overrides: { + }, + }) + + const overrides7 = new OverrideSet({ + overrides: { + bar: { + '.': '^2.0.0', + baz: '1.2.3', + }, + }, + }) + + t.ok(overrides1.isEqual(overrides1), 'overridesets are equal') + t.ok(overrides1.isEqual(overrides2), 'overridesets are equal') + t.ok(!overrides1.isEqual(overrides3), 'overridesets are different') + t.ok(!overrides1.isEqual(overrides5), 'overridesets are different') + t.ok(!overrides1.isEqual(overrides6), 'overridesets are different') + t.ok(!overrides1.isEqual(overrides7), 'overridesets are different') + t.ok(!overrides3.isEqual(overrides1), 'overridesets are different') + t.ok(!overrides3.isEqual(overrides4), 'overridesets are different') + t.ok(!overrides3.isEqual(overrides5), 'overridesets are different') + t.ok(!overrides4.isEqual(overrides5), 'overridesets are different') + t.ok(!overrides5.isEqual(overrides1), 'overridesets are different') + t.ok(!overrides5.isEqual(overrides3), 'overridesets are different') + t.ok(!overrides5.isEqual(overrides6), 'overridesets are different') + t.ok(!overrides6.isEqual(overrides1), 'overridesets are different') + t.ok(!overrides6.isEqual(overrides3), 'overridesets are different') + t.ok(overrides6.isEqual(overrides6), 'overridesets are equal') + t.ok(!overrides7.isEqual(overrides1), 'overridesets are different') + t.end() +}) diff --git a/workspaces/arborist/test/override-set.js b/workspaces/arborist/test/override-set.js index 705996b443b22..d34535c1dbbaa 100644 --- a/workspaces/arborist/test/override-set.js +++ b/workspaces/arborist/test/override-set.js @@ -271,4 +271,121 @@ t.test('constructor', async (t) => { const outOfRangeRule = bazEdgeRule.getEdgeRule({ name: 'buzz', spec: 'github:baz/buzz#semver:^2.0.0' }) t.equal(outOfRangeRule.name, 'baz', 'no match - returned parent') }) + + t.test('isequal and findspecificoverrideset tests', async (t) => { + const overrides1 = new OverrideSet({ + overrides: { + foo: { + bar: { + '.': '2.0.0', + baz: '3.0.0', + }, + baz: '2.0.0', + }, + bar: '1.0.0', + baz: '1.0.0', + }, + }) + const overrides2 = new OverrideSet({ + overrides: { + foo: { + bar: { + '.': '2.0.0', + baz: '3.0.0', + }, + baz: '2.0.0', + }, + bar: '1.0.0', + baz: '1.0.0', + }, + }) + const overrides3 = new OverrideSet({ + overrides: { + foo: { + bar: { + '.': '2.0.0', + baz: '3.1.0', + }, + baz: '2.0.0', + }, + bar: '1.0.0', + baz: '1.0.0', + }, + }) + const overrides4 = new OverrideSet({ + overrides: { + foo: { + bar: { + '.': '2.0.0', + }, + baz: '2.0.0', + }, + bar: '1.0.0', + baz: '1.0.0', + }, + }) + const overrides5 = new OverrideSet({ + overrides: { + foo: { + bar: { + '.': '2.0.0', + }, + bat: '2.0.0', + }, + bar: '1.0.0', + baz: '1.0.0', + }, + }) + const overrides6 = new OverrideSet({ + overrides: { + bar: { + '.': '2.0.0', + }, + bat: '2.0.0', + }, + }) + const overrides7 = new OverrideSet({ + overrides: { + bat: '2.0.0', + }, + }) + const overrides8 = new OverrideSet({ + overrides: { + bat: '1.2.0', + }, + }) + const overrides9 = new OverrideSet({ + overrides: { + 'bat@3.0.0': '1.2.0', + }, + }) + + t.ok(overrides1.isEqual(overrides1), 'override set is equal to itself') + t.ok(overrides1.isEqual(overrides2), 'two identical override sets are equal') + t.ok(!overrides1.isEqual(overrides3), 'two different override sets are not equal') + t.ok(!overrides2.isEqual(overrides3), 'two different override sets are not equal') + t.ok(!overrides3.isEqual(overrides1), 'two different override sets are not equal') + t.ok(!overrides3.isEqual(overrides2), 'two different override sets are not equal') + t.ok(!overrides4.isEqual(overrides1), 'two different override sets are not equal') + t.ok(!overrides4.isEqual(overrides2), 'two different override sets are not equal') + t.ok(!overrides4.isEqual(overrides3), 'two different override sets are not equal') + t.ok(!overrides4.isEqual(overrides5), 'two override sets that differ only by package name are not equal') + t.ok(!overrides5.isEqual(overrides4), 'two override sets that differ only by package name are not equal') + t.equal(OverrideSet.findSpecificOverrideSet(overrides5, overrides5), overrides5, "find more specific override set when the sets are identical") + t.equal(OverrideSet.findSpecificOverrideSet(overrides5, overrides6), overrides6, "find more specific override set when it's the second") + t.equal(OverrideSet.findSpecificOverrideSet(overrides6, overrides5), overrides6, "find more specific override set when it's the first") + t.ok(!OverrideSet.doOverrideSetsConflict(overrides1, overrides2), "override sets are equal") + t.ok(!OverrideSet.doOverrideSetsConflict(overrides5, overrides5), "override sets are the same object") + t.ok(!OverrideSet.doOverrideSetsConflict(overrides5, overrides6), "one override set is the specific version of the other") + t.ok(!OverrideSet.doOverrideSetsConflict(overrides6, overrides5), "one override set is the specific version of the other") + t.ok(OverrideSet.doOverrideSetsConflict(overrides5, overrides7), "no override set is the specific version of the other") + t.ok(OverrideSet.doOverrideSetsConflict(overrides7, overrides5), "no override set is the specific version of the other") + t.ok(!overrides7.isEqual(overrides8), 'two override sets that differ in the version are not equal') + t.ok(!overrides8.isEqual(overrides9), 'two override sets that differ in the range are not equal') + t.ok(!overrides7.isEqual(overrides9), 'two override sets that differ in both version and range are not equal') + t.ok(OverrideSet.doOverrideSetsConflict(overrides7, overrides8), "override sets are incomparable due to version") + t.ok(OverrideSet.doOverrideSetsConflict(overrides7, overrides9), "override sets are incomparable due to version and range") + t.ok(OverrideSet.doOverrideSetsConflict(overrides8, overrides9), "override sets are incomparable due to range") + + }) })