Confusing process.release.compareVersion API

[BridgeAR]

It might only be me but the current implementation is not intuitive for me because I expect the output to be exactly the opposite of what it is. Even though it is not right or wrong either way.

Let's give an example.

The current master version is 10.0.0-pre.

That way I expect 9.0.0 to return -1 because the input value is lower than the current version. Instead, it is 1, because the current version is bigger than the input value. So it depends on what viewpoint the user is in.

For me, it is more intuitive to have the input value being the main important part and not the current version (I hope it is clear what I want to express...).

So I would like to hear from others @nodejs/collaborators how they feel about this (it did not yet land on any release, so there would theoretically still be time to change this without being semver-major).

[tniessen]

@devsnek originally landed the API with the signum as you described in #19438, which was reverted after I expressed my concern that the signum might be counterintuitive (see #19574 (comment)):

The signum of the result of compareVersion seems counter-intuitive to me. In most (if not all) frameworks, a comparator returns a negative result if the first argument (or this) is considered to be "before" the second argument, and a positive result if the first argument (or this) is considered to be "after" the second argument (see e.g. java.lang.Comparable for a single-argument comparison). compareVersion contradicts this well-established norm. I would expect compareVersion(10, 0, 0) >= 0 to return whether this release is v10 or newer, which follows the norm and is intuitive, but #19438 implements the opposite.

The following discussion was a bit difficult to follow as @devsnek edited his response about ten times, but in one of the first revisions he agreed with me.

[devsnek]

I suggest you change it, as the way I see it isn't apparently very popular. I'll approve whatever pr is opened 👍

[jdalton]

\cc @isaacs as author of the semver package for insight into API design.

[rvagg]

Why is this even in core? This is the definition of feature creep, users should just be using the semver package to do this themselves rather than duplicating all that logic and prerelease rules.

The whole of #19587 is a smell, we're duplicating existing functionality (the numbers themselves! what's wrong with a split() to get the major??) and functionality available via npm that includes some non-trivial rules that we need to make ensure is in sync (having dabbled in semver parsing and those rules I can say this is not straightforward).

I'd +1 removing it entirely as a "fix", would I be alone in that?

[rvagg]

proposing reversion @ #20062 along with some reasons and some notes on how the API is currently broken

[BridgeAR]

The whole API got reverted. Closing therefore