deprecation(semver): rename isSemVerComparator()

[timreichen]

renames
SemVerComparator => Comparator
isSemVerComparator() => isComparator()
isSemVerRange() => isRange()
SemVerRange => Range
All old functions and interfaces are deprecated.

[timreichen]

@iuioiua Before merging: There is also an opportunity to simplify Range and lose the single ranges property:

before

export interface Range {
  // The outer array is OR while each inner array is AND
  ranges: SemVerRangeOr;
}

after

// The outer array is OR while each inner array is AND
export type Range = Comparator[][];

WDYT?

[iuioiua]

Sorry, I missed a couple of nits.

[iuioiua]

Sorry, I missed a couple of nits.

[timreichen]

No problem.

@iuioiua Before merging: There is also an opportunity to simplify Range and lose the single ranges property:

before

export interface Range {
  // The outer array is OR while each inner array is AND
  ranges: SemVerRangeOr;
}

after

// The outer array is OR while each inner array is AND
export type Range = Comparator[][];

What is your opinion on this?

[timreichen]

Actually, looking into Range a bit more, it seems that the current implementation is kinda strange.
It is defined like multiple segments on a line. Imo Range implies to be a single segment.

[iuioiua]

Yeah, it is strange. I see repeated use of range.ranges. It should probably be just ranges (without the ranges property).

[timreichen]

How about we redefine Range to export type Range = Comparator[] and remove one OR layer. The OR layer is based on parseRange implementation which can parse range || range, but it should actually return Range[] instead of Range and be called parseRanges instead of parseRange.

[iuioiua]

The first solution sounds better. But maybe worth doing in another PR.

[timreichen]

Yeah, it is strange. I see repeated use of range.ranges. It should probably be just ranges (without the ranges property).

The more I investigate about the Range implementation, the more am I convinced that the two renames

• isSemVerRange() => isRange()
• SemVerRange => Range

are misleading.

Their actual definition of SemVerRange is not a single Range but an array of Ranges.
Instead of renaming Ranges, we should implement Range and also adjust the API functions to accept Range instead of Ranges. Basically all range* functions (rangeFormat, rangeMin, rangeMax, rangeIntersects, minSatisfying, maxSatisfying, testRange, etc.) just loop over the Range array anyway. So instead I would suggest changing these to accept a single Range and get rid of all these loops.

Some of the function names for Range are now used for Ranges. As a solution, we could rename these to *Ranges and deprecate the *Range, then reuse the *Range name after the deprecation deadline is over.
For example:

1. testRange(r:Ranges) =>testRanges(r:Ranges), deprecate testRange(r:Ranges)
2. after deprecation deadline: redefine testRange(r:Range)

WDYT?

Please do not merge until this fundamental problem is solved.

[iuioiua]

Are you able to open a new issue focusing on this weird nested range design? There may be a reason (or there may not be) for this design decision.

Either way, let's proceed with this PR as it focuses on removing the SemVer portion of these symbol names.

[timreichen]

@iuioiua I have opened an issue on this. This misnomer is also present in the npm semver. Even the bnf file in node-semver calls it range-set.

Would you be ok if I reduced this PR to

• isSemVerComparator() => isComparator()
• SemVerComparator => Comparator

so it can be merged?
Else we potentially would need to re-deprecate the Range later again and wait a cycle until we can fix the problem.

[iuioiua]

Yep, let's reduce the PR 👍🏾

[timreichen]

Yep, let's reduce the PR 👍🏾

done

[iuioiua]

LGTM! Thanks! Let's see what Yoshiya thinks.

[kt3k]

LGTM