dhoulb / multi-semantic-release

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

Release note generation disagreeing with package.json #47

Closed Ellosaur closed 3 years ago

Ellosaur commented 3 years ago

We have a monorepo with three main components:

We notice that some releases appear to update the builder's release notes with the correct intended action but in the same commit the changes to the builder's package.json do not reflect this. For example the below claims a dependency bump of runner to 1.8.2 but as you can see no such change was made. Is one change overwriting another perhaps?

image

Our packages use the root level config:

  "release": {
    "plugins": [
      "@semantic-release/commit-analyzer",
      "@semantic-release/release-notes-generator",
      "@semantic-release/changelog",
      "@semantic-release/npm",
      "@semantic-release/github",
      "@semantic-release/git"
    ]
  }

Our builder app is almost the same except it wont publish to npm:

  "release": {
    "plugins": [
      "@semantic-release/commit-analyzer",
      "@semantic-release/release-notes-generator",
      "@semantic-release/changelog",
      [
        "@semantic-release/npm",
        {
          "npmPublish": false
        }
      ],
      "@semantic-release/github",
      "@semantic-release/git"
    ]
  },
antongolub commented 3 years ago

Hey, @Ellosaur,

Interesting... The only place we change manifests is right before any plugin (like npm) triggers its own prepare step.

const prepare = async (pluginOptions, context) => {
            // Wait until the current pkg is ready to be tagged
            getLucky("_readyForTagging", pkg);
            await waitFor("_readyForTagging", pkg);

            updateManifestDeps(pkg);
            pkg._depsUpdated = true;

            const res = await plugins.prepare(context);
            pkg._prepared = true;

            debug("prepared: %s", pkg.name);

            return res;
        };

So technically smth may revert msr's changes. To clarify this point we could add some debug notes to verify that manifest was properly modified.

Ellosaur commented 3 years ago

FYI we're now using semantic-release-monorepo which doesn't attempt to update package references so it avoids this problem. We leave references as ~1.x so that the latest package is always referenced

reuzel commented 3 years ago

Also encountering this issue now... Using the latest version. Seems that it starts occurring when I enabled the cache in Gitlab CICD. My suspicion is that this may occur if the filesystem (e.g. .pnpm store) holds multiple versions of the same package... Is the next version somehow redetermined after the changelog is written?

joostreuzel commented 3 years ago

Diving into the code.... My observation is that the release notes are correct, but the package.json dependencies are not.

The issue lies somewhere with the bumpDependency function in getDependentRelease (line 175 updateDeps.js). There, the next version of the dependent package is being calculated and written to the dependency sections of the manifest. Interestingly this calculation is separate from the calculation that writes the _nextRelease field to the localDeps packages that is being used by the notes generator (createInlinePluginCreator line 186).

The bumpDependency is a kind of side-effect of the resolveReleaseType function that decides what the type of upgrade needs to be for the package. I was wondering if the bug can be fixed by removing the bumpDependency function from getDependentRelease. The manifest dependencies update (bumps) could come in the prepare section of the inline plugin before it is written to file. There it could simply iterate over the localDeps checking for _nextReleases and update the manifest accordingly - not so different as how the generateNotes iterates over the localDeps. This way the manifest update is better located and no longer a side effect.

--- few hours later ---

Studying a bit further: there is a lot of code in the determination of the next release version that I feel is non-necessary. Assuming that the resolveReleaseType simply needs to check if any of the local packages dependencies is to be released, it can simply recursively check if the package dependencies have a releaseType of their own. No need to do a version check. Let semantic-release then define the versions. That could cleanup some of the code as well...

reuzel commented 3 years ago

Actually, there is more going on in the package that does not seem right...

I'll try to create a pull request. The outline of the changes:

antongolub commented 3 years ago

:tada: This issue has been resolved in version 2.8.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: