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

Release note generation disagreeing with package.json #47

Closed
Ellosaur opened this issue Dec 23, 2020 · 6 comments · Fixed by #72
Closed

Release note generation disagreeing with package.json #47

Ellosaur opened this issue Dec 23, 2020 · 6 comments · Fixed by #72
Labels

Comments

@Ellosaur
Copy link

We have a monorepo with three main components:

  • builder our app
  • runner a package used by our app
  • common a package use by both runner and builder directly

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
Copy link
Collaborator

antongolub commented Dec 23, 2020

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
Copy link
Author

Ellosaur commented Feb 2, 2021

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
Copy link
Contributor

reuzel commented Aug 9, 2021

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
Copy link

joostreuzel commented Aug 10, 2021

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
Copy link
Contributor

reuzel commented Aug 11, 2021

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

  • Not all dependencies need to be upgraded. For example, a dependency with '*' should not lead to a update. The notes however don't show this. The notes simply output that any changed dependency will trigger an update of the package. As such, the notes are not right as well...
  • There may a kind of race conditions. All packages are analyzing their dependencies. These dependencies are a directed graph, not a tree, and as such the same package may be analyzed several times. This may lead to strange results.

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

  • analyze commits will analyze changes only.

    • First call the semantic-release if any package changed, then
    • for each package create a list (recursively) which dependencies require update, this will be added as new field to package data structure
    • determine based on the change list if the package requires update
    • set a flag in package to indicate if dependency analysis is done to prevent repeated analysis
  • generateNotes will iterate over the dependencyChange list to generate the notes (thus skipping dependencies that are upgraded, but don't need an upgrade of the package itself based on the semver dependency rules)

  • prepare will iterate of the the dependencyChange list to update the package content. Here it has access to the version semantic-version creates, ensuring that package versions are exactly as semanticrelease indicates

@antongolub
Copy link
Collaborator

🎉 This issue has been resolved in version 2.8.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

4 participants