fix: handle subset check across union of comparator sets - #894
Open
deepakganesh78 wants to merge 1 commit into
Open
fix: handle subset check across union of comparator sets#894deepakganesh78 wants to merge 1 commit into
deepakganesh78 wants to merge 1 commit into
Conversation
When checking if a range is a subset of another range with multiple
OR-branches (e.g., subset('>=17.2.0', '^17.2.0 || >17')), the existing
code only checked each dom comparator set independently. If no single
dom set covered the sub range, it returned false — even when the union
of all dom sets would cover it.
Root cause: simpleSubset() checks sub against each dom set one at a time.
For '>=17.2.0' vs '^17.2.0 || >17', neither '^17.2.0' ([17.2.0, 18.0.0-0))
nor '>17' (>=18.0.0) alone covers [17.2.0, +inf), but their union does.
Fix: Add unionCovers() as a fallback when dom has multiple OR-branches
and no single branch covers the sub range. It extracts interval bounds
from each dom set, sorts by lower bound, and sweeps left-to-right to
check contiguous coverage. Handles prerelease adjacency (e.g.,
<18.0.0-0 is adjacent to >=18.0.0 in non-prerelease mode), null-set
branches, inclusive/exclusive bound comparisons, and includePrerelease.
Fixes npm#703
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #703
Problem
subset('>=17.2.0', '^17.2.0 || >17')returnsfalsebut should returntrue.The existing
subset()implementation checks each dom comparator set independently viasimpleSubset(). If no single dom set covers the sub range, it returnsfalse— even when the union of all dom sets fully covers the sub range.For this case:
^17.2.0expands to>=17.2.0 <18.0.0-0— covers[17.2.0, 18.0.0-0)>17expands to>=18.0.0— covers[18.0.0, +∞)[17.2.0, +∞)which fully contains>=17.2.0Root cause
simpleSubset()only checks sub against each dom set one at a time. There was no mechanism to consider the union of multiple dom sets.Fix
Added
unionCovers()as a fallback when dom has multiple OR-branches and no single branch covers the sub range. The algorithm:Key details:
<18.0.0-0and>=18.0.0are adjacent (no release version between them), so the algorithm recognizes this as contiguous coverage>=/<=) vs exclusive (>,<) boundsincludePrereleaseoption: Respects the option by disabling prerelease adjacency shortcuts when enabledCompatibility
This is a non-breaking change — it only makes
subset()returntruein cases where it previously (incorrectly) returnedfalse. No existing behavior is changed.Validation
*ranges, inclusive/exclusive bounds,includePrereleasemode