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

Exclude prereleases from range maximums #320

Closed
wants to merge 1 commit into from

Conversation

eemeli
Copy link
Contributor

@eemeli eemeli commented Apr 7, 2020

In semver v4, the ranges were simplified in 2331a9e "to remove -0 everywhere". This had the unintended side effect of including the endpoint version's prerelease versions in the resulting range, as semver.lt('2.0.0-0', '2.0.0') === true.

This bug is apparent in intersection tests:

semver.intersects('1', '2.0.0-0') // false
semver.intersects('1', '^2.0.0-0') // was true, here fixed to false

To fix this, the Range class is updated to always include -0 in the < Comparator when generating one from expressions like 1.x, 1.0 - 1.5, ~1.2.3, or ^1.2.

Tests are updated, moving one test from range-include to range-exclude, comparing 2.0.0-rc1 against ^1.0.0 when includePrerelease is true

Fixes #223
Fixes #254

In semver v4, the ranges were simplified in 2331a9e "to remove -0
everywhere". This had the unintended side effect of including the
endpoint version's prerelease versions in the resulting range, as
lt('1.0.0-0', '1.0.0') === true.

Tests are updated, adding -0 to the maximal endpoints. One test is
moved from range-include to range-exclude, comparing 2.0.0-rc1 against
^1.0.0

Fixes npm#223
Fixes npm#254
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a257a17 on eemeli:fix-ranges into dfe658f on npm:master.

@elashpear
Copy link

@isaacs why was this PR and all related issues closed without merging and with no rationale listed? Is this intended behavior? It seems surprising and unintuitive.

@ljharb
Copy link
Contributor

ljharb commented Jul 13, 2021

@elashpear this PR was merged via landing a squashed commit - see the hash in the closing message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants