Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support monorepos by coercing version strings #128

Open
adam-coster opened this issue Mar 27, 2023 · 4 comments
Open

Support monorepos by coercing version strings #128

adam-coster opened this issue Mar 27, 2023 · 4 comments

Comments

@adam-coster
Copy link

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!

@ckerr
Copy link
Member

ckerr commented Mar 27, 2023

We added semver.valid checks in d4c005f to fix #61 and in 3374b50 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! 😸

@juliangruber
Copy link
Contributor

+1 to using semver.coerce, as long as it doesn't product false positives, where a release that shouldn't be considered is selected.

@adam-coster
Copy link
Author

adam-coster commented Mar 27, 2023

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
  }
}

@ckerr
Copy link
Member

ckerr commented Mar 28, 2023

Looks like there's consensus on coerce, so @adam-coster feel free to make the PR you suggested 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants