deprecation(semver): deprecate SemVerRange, introduce Range
#4161
Conversation
There was a problem hiding this comment.
I mostly agree with this PR. The only part I'm slightly uncertain about is having Comparator extend SemVer. Either way. I left a few suggestions.
Also, I notice we still use some deprecated functions within other functions. We should look into switching to the updated functions in those cases. Probably best done after this PR, separately.
| @@ -12,5 +12,5 @@ import { format } from "./format.ts"; | |||
| */ | |||
| export function comparatorFormat(comparator: Comparator): string { | |||
| const { semver, operator } = comparator; | |||
| return `${operator}${format(semver)}`; | |||
| return `${operator}${format(semver ?? comparator)}`; | |||
There was a problem hiding this comment.
Nice. Can we please have tests for added functionality? We want to ensure that both SemVer and Comparator inputs work fine on all these functions, including those in this PR.
There was a problem hiding this comment.
How do you mean exactly?
There was a problem hiding this comment.
I added some test cases. PTAL.
| @@ -47,5 +47,5 @@ export function parseComparator(comparator: string): Comparator { | |||
| } | |||
| : ANY; | |||
|
|
|||
| return { operator, semver }; | |||
| return { operator, ...semver, semver }; | |||
There was a problem hiding this comment.
I forsee the output of this object being confusing to users during the deprecation period. But I don't see another viable way of doing this. Food for thought...
i don't see a better way to remove the
I'll have a look at that after this PR. I think we might be best of putting all comparator code into one |
Co-authored-by: Asher Gomez <ashersaupingomez@gmail.com>
Done |
|
This looks like an immediate breaking change to me. [
[
{
operator: ">=",
major: 1,
minor: 2,
patch: 3,
prerelease: [],
build: [],
semver: { major: 1, minor: 2, patch: 3, prerelease: [], build: [] }
},
{
operator: "<",
major: 2,
minor: 0,
patch: 0,
prerelease: [],
build: [],
semver: { major: 2, minor: 0, patch: 0, prerelease: [], build: [] }
}
]
]instead of: {
ranges: [
[
{
operator: ">=",
semver: { major: 1, minor: 2, patch: 3, prerelease: [], build: [] }
},
{
operator: "<",
semver: { major: 2, minor: 0, patch: 0, prerelease: [], build: [] }
}
]
]
}It's typed like both are supported, but actually only |
|
I feel this kind of thing can't be done in a non-breaking way. |
|
|
Ah, ok. Sorry I missed |
Co-authored-by: Yoshiya Hinosawa <stibium121@gmail.com>
Co-authored-by: Yoshiya Hinosawa <stibium121@gmail.com>
Co-authored-by: Yoshiya Hinosawa <stibium121@gmail.com>
kt3k
changed the title
SemVerRange to RangeSemVerRange, introduce Range
This PR does a bunch of things. While some things could theoretically be split up into separate PRs (ref: #4059), some stand-alone PRs might look redundant.
SemVerRangetype in favour ofRange.isSemVerRange()in favour ofisRange()Comparator.semverproperty optional in favour ofComparatorextendingSemVerthat allows a simpler and less nestedRangetype.Note:
Comparatormight become be deprecated (ref: deprecation(semver): deprecateComparator.semverproperty #4059 (comment)), but becauseSemVerRangeis dependent on its structure, it needs to be changed in order to avoid breaking changes.