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

[ENHANCEMENT] add ability to clone a semver instance #378

Open
Tracked by #501
madhead opened this issue Mar 27, 2021 · 11 comments
Open
Tracked by #501

[ENHANCEMENT] add ability to clone a semver instance #378

madhead opened this issue Mar 27, 2021 · 11 comments
Labels
semver:major backwards-incompatible breaking changes
Milestone

Comments

@madhead
Copy link

madhead commented Mar 27, 2021

What / Why

I'm trying to compute a bunch of various increments for a specific version (TypeScript):

console.log(version.inc('major'))
console.log(version.inc('minor'))
console.log(version.inc('patch'))

The problem is the SemVer class itself is mutable, so each increment is computed against previous increment result. This is not what I need.

So, I need a way to clone this object, like.

console.log(new SemVer(version).inc('major'))
console.log(new SemVer(version).inc('minor'))
console.log(new SemVer(version).inc('patch'))

But the constructor of the SemVer (new SemVer(version)) doesn't create a new instance when passed another instance with the same options. So, again, the code doesn't work.

Is there a way to clone a SemVer?

@pigulla
Copy link

pigulla commented May 20, 2021

I'd also be interested in this. Did you find a solution?

@madhead
Copy link
Author

madhead commented May 20, 2021

Unfortunately, no, I had to store the components separately and use them to construct new instances.

@rauno56
Copy link

rauno56 commented Jan 15, 2022

When the version is a string, it works as you expect it to @madhead, @pigulla.

{
  // works as you expect:
  const version = '1.2.3';
  console.log(new SemVer(version).inc('major'))
  console.log(new SemVer(version).inc('minor'))
  console.log(new SemVer(version).inc('patch'))
}

{
  // When you already have a SemVer object and want to copy it, pass "version"
  const semVersion = new SemVer('1.2.3');
  console.log(new SemVer(semVersion.version).inc('major'))
  console.log(new SemVer(semVersion.version).inc('minor'))
  console.log(new SemVer(semVersion.version).inc('patch'))
}

@madhead
Copy link
Author

madhead commented Apr 5, 2022

No fix, just closing?

@lukekarrys
Copy link
Member

The previous comment is the best answer to the question at this time, since there is no way to fully clone a semver instance (with options). I would recommend making a wrapper function that takes a version and options and always returns a new instance. We don't have any plans at this time to add a new method to clone an instance.

@ljharb
Copy link

ljharb commented Apr 5, 2022

@lukekarrys it kind of seems like this then is a feature request to add that? something like new Semver(instance) or Semver.clone(instance)

@lukekarrys
Copy link
Member

I think new SemVer(instance) cloning the existing instance would be a breaking change, that I would rather not make. A static method on the class would make more sense, but I'm still hesitant to add api surface area. A PR or new issue with this feature proposal could get more input as to whether it should be added or not.

fwiw here's what I was thinking previously for a wrapper function:

const clone = (v) => new SemVer(v.version ?? v, v.options)

@ljharb
Copy link

ljharb commented Apr 5, 2022

i'd expect v => v instanceof Semver ? new Semver(v.version, v.options) : new Semver(v).

i'm not sure why it'd be a breaking change; do you think anyone's relying on new Semver(instance) if it doesn't work right now?

@lukekarrys
Copy link
Member

i'm not sure why it'd be a breaking change

I'm erring on the side of caution, and not knowing the history of the decision. It appears to be an explicit design decision to reuse an instance (and I traced this back to at least v2.0.0)

if (version instanceof SemVer) {
if (version.loose === !!options.loose &&
version.includePrerelease === !!options.includePrerelease) {
return version
} else {
version = version.version
}

if it doesn't work right now?

It does work to do new Semver(instance) it just returns instance (if the options match).

I can imagine situations for a highly used library like this, in which this would be a breaking change. Even the fact that this would break strict equality checks, makes me lean towards breaking change.

const s = new SemVer('1.0.0')
s === new SemVer(s) // true

@ljharb
Copy link

ljharb commented Apr 6, 2022

Fair point.

@lukekarrys lukekarrys reopened this Oct 27, 2022
@lukekarrys lukekarrys added the semver:major backwards-incompatible breaking changes label Oct 27, 2022
@lukekarrys lukekarrys changed the title [QUESTION] How do I clone a SemVer? [ENHANCEMENT] add ability to clone a semver instance Oct 27, 2022
@lukekarrys
Copy link
Member

Reopening this and labeling as semver:major so we don't forget to figure out a proper API for this in the future. It might not end up being a breaking change, but grouping it with the rest of the semver:major issues will help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:major backwards-incompatible breaking changes
Projects
None yet
Development

No branches or pull requests

6 participants