Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: issue-5850 - Settle override conflicts between edges and propagate changes #7025

Open
wants to merge 33 commits into
base: latest
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
865070c
Settle overrides conflicts between edges, and propagate changes to th…
AlonNavon Nov 23, 2023
cd20196
cleanup
AlonNavon Nov 26, 2023
0cad4e0
Optimization
AlonNavon Nov 26, 2023
6b276f3
Fix override sets comparisons
AlonNavon Nov 28, 2023
fff3072
Add comparator
AlonNavon Nov 28, 2023
e97176e
Check for undefined
AlonNavon Nov 28, 2023
19155ed
Another place
AlonNavon Nov 28, 2023
dedbccf
Fix
AlonNavon Nov 28, 2023
4a83786
Lint fixes
AlonNavon Dec 6, 2023
7ee2f2e
Added tests
AlonNavon Dec 6, 2023
0917421
Overrides aren't inherited from parent, but from to nodes
AlonNavon Oct 27, 2024
6d0ad42
Always use raw spec when analyzing overrides
AlonNavon Oct 28, 2024
67d7d5c
Replaceability
AlonNavon Oct 28, 2024
643cbea
Overridden status
AlonNavon Oct 28, 2024
c5f6e4a
Fix undefined bugs
AlonNavon Oct 28, 2024
6c9bbf9
Parent doesn't matter to overrides
AlonNavon Oct 28, 2024
63760d9
Update override set in reload
AlonNavon Oct 28, 2024
6111630
Erroneous node
AlonNavon Oct 28, 2024
b42d4c8
Add comments
AlonNavon Oct 29, 2024
7d97726
Code review fixes
AlonNavon Nov 4, 2024
35e5790
Fix edge satisfaction logic
AlonNavon Nov 4, 2024
35ce2f7
Fix dedupe logic
AlonNavon Nov 4, 2024
5630954
Code review fixes
AlonNavon Nov 4, 2024
f859e92
Oops
AlonNavon Nov 4, 2024
29f0750
Static function
AlonNavon Nov 5, 2024
17ec457
Typo
AlonNavon Nov 5, 2024
4cde831
Shouldn't throw
AlonNavon Nov 7, 2024
701b125
CR fixes
AlonNavon Nov 7, 2024
b119f67
CR fixes
AlonNavon Nov 7, 2024
3136b64
Move functions
AlonNavon Nov 10, 2024
80126c7
Comments
AlonNavon Nov 11, 2024
c4b5338
Override sets unit tests
AlonNavon Nov 11, 2024
9db1232
Clarify comment
AlonNavon Nov 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion workspaces/arborist/lib/dep-valid.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const depValid = (child, requested, requestor) => {
})
}

default: // unpossible, just being cautious
default: // impossible, just being cautious
break
}

Expand Down
26 changes: 25 additions & 1 deletion workspaces/arborist/lib/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,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.
AlonNavon marked this conversation as resolved.
Show resolved Hide resolved
if (!this.overridden) {
wraithgar marked this conversation as resolved.
Show resolved Hide resolved
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)
AlonNavon marked this conversation as resolved.
Show resolved Hide resolved
}

// return the edge data, and an explanation of how that edge came to be here
Expand Down
2 changes: 1 addition & 1 deletion workspaces/arborist/lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ class Node {
// 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 (this.overrides.isEqual(edge.overrides)) {
if (edge.overrides && edge.overrides.name === this.name && edge.overrides.value === this.version) {
if (!edge.overrides.isEqual(edge.from.overrides)) {
return true
}
Expand Down