Open adam-coster opened 1 year ago
We added semver.valid
checks in d4c005f to fix https://github.com/electron/update.electronjs.org/issues/61 and in 3374b502a2b7ef8d49dbce1d1f7d6224b66c4b52 to fix #45.
I'd like another opinion from maintainers more active in this repo than me; but from a quick read of d4c005f, it looks to me like semver.coerce
would still work for those use cases.
Also CC @juliangruber for opinion as he wrote both of those patches! :smile_cat:
+1 to using semver.coerce
, as long as it doesn't product false positives, where a release that shouldn't be considered is selected.
semver.coerce
is pretty aggressive about trying to get a semver string out of something. Probably too aggressive.
An alternative would be a custom semver-cleaning function that grabs the first actually-valid semver substring out of a larger string:
/**
* @param {string} version_string
* @returns {string|null}
*/
function find_semver_substring(version_string) {
if (!version_string || typeof version_string !== 'string') {
return false;
}
let version = semver.valid(version_string);
if (version) {
return version;
}
// Else grab the first substring that looks like a semver
// Adapted from https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
version =
/(?<major>0|[1-9]\d*)\.(?<minor>0|[1-9]\d*)\.(?<patch>0|[1-9]\d*)(?:-(?<prerelease>(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?<buildmetadata>[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?/.exec(
version_string,
)?.[0];
return version ?? null;
}
That regex is from the official semver docs, with the leading ^
and trailing $
removed (so it'll match substrings) and switch from Perl capture-group naming to JavaScript.
If it doesn't make sense to capture build strings that part of the regex could be removed, or run the final result through semver.valid
for consistency.
Some examples, where each field is the string passed into semver
's valid
, clean
, or coerced
methods, or the above "custom" function:
{
"v10.0.3": {
"valid": "10.0.3",
"clean": "10.0.3",
"coerced": "10.0.3",
"custom": "10.0.3"
},
"0.0.11": {
"valid": "0.0.11",
"clean": "0.0.11",
"coerced": "0.0.11",
"custom": "0.0.11"
},
"0.0.11-rc": {
"valid": "0.0.11-rc",
"clean": "0.0.11-rc",
"coerced": "0.0.11",
"custom": "0.0.11-rc"
},
"monorepo@v2.3.4-rc.0.10": {
"valid": null,
"clean": null,
"coerced": "2.3.4",
"custom": "2.3.4-rc.0.10"
},
"monorepo2.3": {
"valid": null,
"clean": null,
"coerced": "2.3.0",
"custom": null
},
"1.2.3-rc": {
"valid": "1.2.3-rc",
"clean": "1.2.3-rc",
"coerced": "1.2.3",
"custom": "1.2.3-rc"
},
"hello world 1": {
"valid": null,
"clean": null,
"coerced": "1.0.0",
"custom": null
}
}
Looks like there's consensus on coerce, so @adam-coster feel free to make the PR you suggested :+1:
The updater currenty relies on
semver.valid
to determine whether incoming version strings are valid. It does this for the parsed URL as well as the tags from GitHub releases.Monorepos have to uses more complex tag prefixes, since they can contain many projects that each have a different version. The typical format is
package-name@0.1.2
.If the updater were to clean putative version strings with
semver.coerce
, that would solve this problem.It looks like that would be an easy update -- if the feature is wanted I can make a PR!