dhoulb / multi-semantic-release

Proof of concept that wraps semantic-release to work with monorepos.
BSD Zero Clause License
202 stars 37 forks source link

feat: add new bump option "prefix" #101

Closed moroine closed 2 years ago

moroine commented 2 years ago

Use Case:

Let's say we have:

Here are the package.json definition of a & b in the repository (we use 0.0.0 to force using local versions):

{
  "name": "a",
  "version": "0.0.0",
}
{
  "name": "b",
  "version": "0.0.0",
  "peerDependencies": {
     "a": "^0.0.0"
   }
}

When publishing the new version 1.0.1 the published package.json will be:

{
  "name": "b",
  "version": "1.0.1",
  "peerDependencies": {
     "a": "^1.0.1"
   }
}
antongolub commented 2 years ago

@moroine,

Could plz clarify why --deps.bump='inherit' does not produce the same effect?

moroine commented 2 years ago

Because looks like deps.bump will just check if current version satisfy requirement. Hence it will check if 1.0.1 satisfy^0.0.0. But because it doesn't satisfy, it will bump the version package to 1.0.1 without keeping the ^.

I've added a test to be able to compare differences with inherit strategy. I was originally thinking of using inherit but this would be a breaking change

moroine commented 2 years ago

@antongolub what do you think about this?

antongolub commented 2 years ago

Perhaps, we have an inaccuracy in describing of 'bump.inherit' behavior. Let's look on the test instead:

describe("resolveNextVersion()", () => {
    // prettier-ignore
    const cases = [
        ["1.0.0", "1.0.1", undefined, "1.0.1"],
        ["1.0.0", "1.0.1", "override", "1.0.1"],

        ["*", "1.3.0", "satisfy", "*"],
        ["^1.0.0", "1.0.1", "satisfy", "^1.0.0"],
        ["^1.2.0", "1.3.0", "satisfy", "^1.2.0"],
        ["1.2.x", "1.2.2", "satisfy", "1.2.x"],

        ["~1.0.0", "1.1.0", "inherit", "~1.1.0"],
        ["1.2.x", "1.2.1", "inherit", "1.2.x"],
        ["1.2.x", "1.3.0", "inherit", "1.3.x"],
        ["^1.0.0", "2.0.0", "inherit", "^2.0.0"],
        ["*", "2.0.0", "inherit", "*"],
        ["~1.0", "2.0.0", "inherit", "~2.0"],
        ["~2.0", "2.1.0", "inherit", "~2.1"],
    ]

If ["~1.0", "2.0.0", "inherit", "~2.0"] works why ["^0.0.0", "1.0.0", "inherit", "^1.0.0"] does not — that's what i'm trying to figure out.

moroine commented 2 years ago

@antongolub did you got time to have a look at it?

dbouwman commented 2 years ago

@antongolub we are also running into a similar issue where the ^ is being removed.

I my test project, I ran changes that should have hit this test case (in dependencies and peerDependencies)

["^1.0.0", "2.0.0", "inherit", "^2.0.0"],

except that it actually inserted "2.0.0" when running in a real release.

antongolub commented 2 years ago

@moroine, @dbouwman,

Maybe it's better to introduce override-prefix? So we could also pass ~.

moroine commented 2 years ago

@antongolub this is very problematic. Do you prefer to add this PR or still want to find out if this is a bug.

About override-prefix I think that's a good idea, but what would be the value of deps.bump ?

antongolub commented 2 years ago

@moroine, there's no bug as I can see now. And your feature is not covered by satisfy strtegy, I was wrong. So I suggest tweaking a bit your PR:

  1. Introduce --deps.prefix = '' | '^' | '~' that will be attached to the next v if --deps.bump === 'override'.
  2. Refactor resolveReleaseType, updateManifestDeps

I've landed your proposal on our experimental fork: https://github.com/qiwi/multi-semantic-release/pull/70/files

moroine commented 2 years ago

@antongolub your changes are excellent, I've also used your descriptions 😄

What are the differences with qiwi/multi-semantic-release? Does it respect the dependencies order, but maximize parallelization?

antongolub commented 2 years ago

image

The main diff: @qiwi fork replaces concurrent event-driven flow with toposort-based queues. It's definitely slower, but it's easier to debug our proprietary plugins.

antongolub commented 2 years ago

@moroine, LGTM, I'm ready to merge. But could you check what happens if:

if (scope[dependency.name]) {
    scope[dependency.name] = resolveNextVersion(dependency._lastRelease?.version || '0.0.0', dependency._nextRelease?.version, bumpStrategy, prefix);
} 

I think it would be better if only resolveNextVersion implted prefix handling logic.

antongolub commented 2 years ago

@moroine, this works:

const { isObject, isEqual, transform, get } = require("lodash");
//...
if (scope[dependency.name]) {
    scope[dependency.name] = resolveNextVersion(
    get(dependency, "_lastRelease.version", "0.0.0"),
    release.version,
    bumpStrategy,
    prefix
    );
}
antongolub commented 2 years ago

@moroine, finally merged.

Thanks a lot for your improvements and for your patience! The new feature will be available in v2.13.0.

antongolub commented 2 years ago

:tada: This PR is included in version 2.13.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: