denoland / std

The Deno Standard Library
https://jsr.io/@std
MIT License
3.03k stars 606 forks source link

semver: `rangeIntersects` ignores the intersection of prerelease versions #4300

Closed nicolo-ribaudo closed 7 months ago

nicolo-ribaudo commented 7 months ago

I'm trying to use the semver range utilities, and I the default handling of pre-releases in ranges different from how npm would do it. I think I need to use the includePrerelease flag (documented in the "Prerelease Tags" section of https://deno.land/std@0.215.0/semver/mod.ts) to get the same behavior as npm, but I cannot figure out where to pass it given that the testRange function does not accept an option bag, and the Range type does not include that property.

iuioiua commented 7 months ago

Interesting. Perhaps we should add this option. Or, we could include pre-release info by default without opt-out. However, we'd have to investigate any potential pitfalls that might be introduced. Thoughts, @timreichen?

timreichen commented 7 months ago

If I understand correctly, this option changes how versions are compared against a range depending on if the range has a prerelease or not: 1.2.3-alpha.7 satisfies >1.2.3-alpha.3 3.4.5-alpha.9 does not satisfy >1.2.3-alpha.3

This rule is confusing to me because it special treats prereleases and the comparison deviates from the general comparison algorithm. The description in the npm repo even states

...but it would not be satisfied by 3.4.5-alpha.9, even though 3.4.5-alpha.9 is technically "greater than" 1.2.3-alpha.3 according to the SemVer sort rules.

I think we should take some time to check if there is a better way to implement that feature other than an option that changes the function algorithm.

iuioiua commented 7 months ago

I think we should take some time to check if there is a better way to implement that feature other than an option that changes the function algorithm.

Yep. This is what I meant by including pre-release info without opt-out 🙂

kt3k commented 7 months ago

@nicolo-ribaudo We deprecated includePrerelease option in #3385 and removed in #3591. The doc part mentioning includePrerelease is a bug of documentation.

After #3591, std/semver should be behaving like includePrerelease is always true because semver spec defines the only one ordering of versions and prerelease is included in it.

Can you share some examples which are different from npm's range comparison?

nicolo-ribaudo commented 7 months ago

(I delete a comment because I completely misremembered what I was trying to do)

The problem I had was that rangeIntersects(parseRange("<7.0.0-beta.20"), parseRange(">7.0.0-beta.0")) returns false, but I would expect it to return true as it can contain all version from 7.0.0-beta.1 to 7.0.0-beta.19. If I use non-prereleases it works as expected (rangeIntersects(parseRange("<1.1.20"), parseRange(">1.1.0")) returns true).

kt3k commented 7 months ago

Thanks for the example. Looks like a bug of rangeIntersects.

To be more specific, the internal util comparatorMax seems ignoring prerelease version since #3385

nicolo-ribaudo commented 7 months ago

Should I open a separate issue for the docs describing a non-existent option?

iuioiua commented 7 months ago

Should I open a separate issue for the docs describing a non-existent option?

Yes please 🙂

kt3k commented 7 months ago

Should I open a separate issue for the docs describing a non-existent option?

I think that was fixed in https://github.com/denoland/deno_std/pull/4324