[BUG] inc premajor, preminor, and prepatch do the wrong thing if the version is already a pre-release.

Is there an existing issue for this?

I have searched the existing issues

Current Behavior

From the docs:

premajor in one call will bump the version up to the next major version and down to a prerelease of that major version. preminor, and prepatch work the same way.

If we do these steps in isolation (using the internal, "pre" that does the down to a prerelease) we get:

> semver.inc('2.0.0-pre', 'major')
'2.0.0'
> semver.inc('2.0.0', 'pre')
'2.0.0-0'

But if we do premajor

> semver.inc('2.0.0-pre', 'premajor')
'3.0.0-0'

Expected Behavior

premajor et al should "do what it says on the tin", and not jump an extra major/minor/patch.

Steps To Reproduce

No response

Environment

semver@7.6.3

real world use case: npm/template-oss#495

presumably 2.0.0 + premajor would still jump to 3.0.0-0?

Yes, they all currently do the right thing if the version being incremented is not already a prerelease.

> semver.inc('2.0.0', 'major')
'3.0.0'
> semver.inc('3.0.0', 'pre')
'3.0.0-0'
> semver.inc('2.0.0', 'premajor')
'3.0.0-0'

> semver.inc('3.2.4-pre.1', 'premajor')
'4.0.0-0'

It seems we also drop the existing preid, unlike

> semver.inc('3.2.4-pre.1', 'prerelease')
'3.2.4-pre.2'

I also want to note that semver inc currently isn't always strict and does "just drop prerelease"

Incremented "premajor version" will always drop prerelease.

> semver.inc('7.0.0-pre.0', 'major')
'7.0.0'
> semver.inc('7.0.0-pre.0', 'minor')
'7.0.0'
> semver.inc('7.0.0-pre.0', 'patch')
'7.0.0'

Incremented "preminor version" will drop prerelease if minor change.

> semver.inc('6.1.0-pre.0', 'major')
'7.0.0'
> semver.inc('6.1.0-pre.0', 'minor')
'6.1.0'
> semver.inc('6.1.0-pre.0', 'patch')
'6.1.0'

Incremented "prepatch version" will drop prerelease if patch change.

> semver.inc('6.0.5-pre.0', 'major')
'7.0.0'
> semver.inc('6.0.5-pre.0', 'minor')
'6.1.0'
> semver.inc('6.0.5-pre.0', 'patch')
'6.0.5'

So there is prerelease context aware functionality already incorporated into the existing inc.

Draft PR at #752

With the draft, premajor, preminor, prepatch do two things to the pre-release:

add a new pre-release 0, if it does not exist
increases the pre-release id, if it exists

That's confusing.

I see these use cases:
a) Starting a new release with pre-release 0 (premajor, preminor, prepatch)
b) Increasing pre-releases, usually several times (prerelease)
c) Removing the pre-release (release, new)

Using premajor, preminor, prepatch with an existing pre-release should be an error. They work correctly otherwise.

@mbtools

@mbtools I don't think it's feasible to throw, you need the conditional logic, you missed the case where your version is a preminor and you want to go to premajor, it shouldn't throw, it should bump if it's premior or prepatch and if it's premajor it will increment the pre.0 number.

That's confusing.

I would submit that this is a product of design, and unavoidable. Prerelease is not very intuitive and a lot of folks stumble here.

The use case here is one of automation. Consider a situation where you know you have a major/minor/patch change to be made, and you also are either in pre-release mode or you aren't. You should be able to tell semver to bump your version with just those two pieces of information. Otherwise it's a LOT more tooling and work to infer what you should do.

If I have a piece of software that's on 10.1.0-pre.0, and I have two new changes:

patch and breaking, I should expect to use premajor to now bump to 11.0.0-pre.0.
feat and patch, I should expect to use preminor to bump to 10.1.0-pre.1 since I'm already in preminor mode, but I need a new version.

Consider the original purpose "bump to next release type and drop into pre-release mode". If you're already in the prerelease mode for that type that's effectively just a new pre-release for that same version.

ETA:

10.0.0-pre.0 bumped to the next major is 10.0.0 and then dropped to pre-release would be 10.0.0-pre.0 again, but that's not what you actually want. You are still asking for the "next" premajor". You really do want 10.0.0-pre.1

I would use the new release to exit pre-release mode, and then use the existing premajor/minor/patch to get into a new pre-release. That's a clear separation of functionality.

premajor in one call will bump the version up to the next major version and down to a prerelease of that major version. preminor, and prepatch work the same way.

We are asking the question "what does it mean to do this operation on a version that is already in a prerelease for that release type already"?

inc prepatch means going to a new patch
inc preminor means going to a new minor release
inc premajor means going to a new major release
...and initializing the pre-release identifier

My interpretation of the doc is that this will always lead to a higher x.y.z version ("bump the version") and is independent of any existing pre-release identifier. My preference is to error if a pre-release id exists, but it works correctly now.

I'm just one semver user and suggest to get some other opinions :-)

It would be good to separate feat: add "release" inc type from #752. It's needed by the community 🙏

@mbtools

@mbtools maybe it would be helpful to you to take a look at the literal "automated process" function we'd use with release-please https://github.com/npm/template-oss/blob/main/lib/release/semver-versioning-strategy.js#L56-L68

  bump(currentVersion, commits) {
    const prerelease = this.options.prerelease
    const tag = this.options.prereleaseType
    const releaseType = parseCommits(commits)
    const addPreIfNeeded = prerelease ? `pre${releaseType}` : releaseType
    const version = inc(currentVersion.toString(), addPreIfNeeded, tag)
    /* istanbul ignore next */
    if (!version) {
      throw new Error('Could not bump version')
    }
    return Version.parse(version)
  }

If you want to independently add the release type with tests go for it. I was just adding it in a discrete commit while I was in here.

@wraithgar

Looking at the template code and related test cases was helpful. I agree with the mapping very much 👍

I consolidated it into one statement (see npm/template-oss#497).

  const nextReleaseType =
    !prereleaseMode ? releaseType :                          // normal release
      !isPrerelease ? 'pre' + releaseType :                  // first prerelease
        !isMajor && releaseType === 'major' ? 'premajor' :   // minor or patch prerelease and major coming
          !isMinor && releaseType === 'minor' ? 'preminor' : // patch prerelease and minor coming
            'prerelease'                                     // next prerelease

If I understand @wraithgar correctly, you would like to reduce this to

  const nextReleaseType =
    !prereleaseMode ? releaseType : 'pre' + releaseType

and move the other logic into semver.inc.

This is tempting but I don't think it matches the documentation for inc. It means, for example, that semver.inc('7.0.0-pre.1', 'premajor' ) = '7.0.0-pre.2'. This is not "bumping the version" and "down to a prerelease." It's the opposite: keeping the version and upping the prerelease.

I'm fine with such change but it's breaking and should be documented accordingly.

@mbtools

@mbtools i'm glad that helped and thanks for the rewrite (a lot of ternaries though), the idea is that we'd remove that function and just embed that logic into semver, it's hard to advocate / explain to third party libraries such as release-please to implement "semver" but then we have this custom function thrown in there.

If I understand @wraithgar correctly, you would like to reduce this to

  const nextReleaseType =
    !prereleaseMode ? releaseType : 'pre' + releaseType

yes!

This is tempting but I don't think it matches the documentation for inc. It means, for example, that semver.inc('7.0.0-pre.1', 'premajor' ) = '7.0.0-pre.2'. This is not "bumping the version" and "down to a prerelease." It's the opposite: keeping the version and upping the prerelease.

this is why I commented above with #751 (comment)

inc already has conditional logic that doesn't do the literal "bump the version", I see this as precedent that alternative paths happen when you are in prerelease mode:

For instance this doesn't do what it says: (and would be made more literal by a release releaseType for sure)

> semver.inc('7.0.0-pre.0', 'major')
'7.0.0'

> semver.inc('7.0.0-pre.0', 'major')
'7.0.0'

That is a major increase. prerelease is not intuitive.

Alright, we agree on what semver should do for npm 😃

The remaining question: Will it break other projects?

I think the existing premajor/minor/patch behavior is correct and I doubt I'm alone.

If a project is on 6.x.y and we get a breaking change, we do premajor to get to 7.0.0-pre-0. We work on it i.e. prerelease to 7.0.0-pre.4. If we get another breaking change, before releasing the major, that's premajor again.

Today, this results in 8.0.0-pre-0. So this second breaking change goes to a different release.

With the suggested change, the second breaking change would be folded into the existing 7.0.0... line. That is quite different. Projects working on multiple release lines in parallel might have an issue with that.

I think the existing premajor/minor/patch behavior is correct and I doubt I'm alone.

If a project is on 6.x.y and we get a breaking change, we do premajor to get to 7.0.0-pre-0. We work on it i.e. prerelease to 7.0.0-pre.4. If we get another breaking change, before releasing the major, that's premajor again.

Today, this results in 8.0.0-pre-0. So this second breaking change goes to a different release.

I also wonder if this will break for anyone. In your example I personally don't see the point never releasing a version 7 because you made you wanted to make two successive premajors releases ie 7.0.0-pre.0 then 8.0.0-pre.0 would result in having something like 6.9.2 to 8.0.0.

I think the difference in thinking is "are we breaking 6.9.2" vs "are we breaking 7.0.0-pre.0" and I philosophically don't think you can make a breaking change to a prerelease. Any change should reference the latest non-prerelease. Also whats stopping jumping n number of majors during the prerelease cycle? 6.9.2 to 18.3.24 is not useful.

I'd say you can't avoid making breaking changes in a prerelease :-p iow that every prerelease is potentially breaking version to version.

but i agree that if this is only changing semantics for people who are bumping from one prerelease to another, it's going to affect a very small number of people.

Is there a scenario where someone may be branching out different future versions and keeping multiple "futureports" (opposite of a "backport") at the same time and would like to maintain 7.0.0-0 and 8.0.0-0 non-linearly, maybe?

Here's the argument against the proposal, the "pain path"

Person branches 6.9.2
Person branches from 6.9.2 and to a v7 branch runs npm version premajor gets 7.0.0-0
Person branches from 6.9.2 and to a v8 branch runs npm version premajor x2 and gets 7.0.0-1 and they want 8.0.0-0

This person should just change the version manually, they're non-linearly trying to "increment".

Within this scenario the v7 branch still should never jump to v8, though...

We use changesets in the Verdaccio project. I found this:

For a breaking change, they use semver.inc(... 'major') and prerelease++. The effect of their "increment" implementation is 7.0.0-pre.1 + breaking change = 8.0.0-pre.2. So they do expect a major increase while prerelease mode 👀

The good news is that they don't use pre... so a change here won't break them.

--

release-please interpretation is similar to what is suggested i.e. keep the version, increase the counter. However, their interpretation of the prerelease number is different. 1.0.0-beta01 + breaking change = 1.0.0-beta02. 🤷

7.0.0-pre.1 + breaking change = 8.0.0-pre.2

If they never released 7.0.0 this seems like an incorrect version change. That just means 7.0.0 has more than one breaking change.

Navigation Menu

Repository navigation

[BUG] inc premajor, preminor, and prepatch do the wrong thing if the version is already a pre-release. #751

Is there an existing issue for this?

Current Behavior

Expected Behavior

Steps To Reproduce

Environment

13 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[BUG] inc premajor, preminor, and prepatch do the wrong thing if the version is already a pre-release. #751

Description

Is there an existing issue for this?

Current Behavior

Expected Behavior

Steps To Reproduce

Environment

Activity

reggi commented on Dec 11, 2024

ljharb commented on Dec 11, 2024

wraithgar commented on Dec 11, 2024

wraithgar commented on Dec 11, 2024

reggi commented on Dec 11, 2024

wraithgar commented on Dec 11, 2024

mbtools commented on Dec 12, 2024

reggi commented on Dec 12, 2024

wraithgar commented on Dec 12, 2024

mbtools commented on Dec 12, 2024

13 remaining items

wraithgar commented on Dec 12, 2024

mbtools commented on Dec 12, 2024

mbtools commented on Dec 12, 2024

reggi commented on Dec 12, 2024

wraithgar commented on Dec 12, 2024

mbtools commented on Dec 13, 2024

reggi commented on Dec 13, 2024

wraithgar commented on Dec 13, 2024

mbtools commented on Dec 13, 2024

reggi commented on Dec 13, 2024

ljharb commented on Dec 13, 2024

reggi commented on Dec 13, 2024

mbtools commented on Dec 14, 2024

wraithgar commented on Dec 16, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions