fix: improve algorithm to detect dependency changes (#74)

* fix: support merge workflows in generateNotes
* fix: misalignment between manifest and notes
* test: add failing missing dependency test case
* fix: cyclic dependency errors
* refactor: add helper to find highest release type

Author: reuzel

lib/createInlinePluginCreator.js

@@ -107,8 +107,27 @@ function createInlinePluginCreator(packages, multiContext, synchronizer, flags)
 			pkg._analyzed = true;
 			await waitForAll("_analyzed");
 
-			// Make sure type is "patch" if the package has any deps that have changed.
-			pkg._nextType = resolveReleaseType(pkg, flags.deps.bump, flags.deps.release);
+			//go in rounds to check for dependency changes that impact the release type until no changes
+			//are found in any of the packages. Doing this in rounds will have the changes "bubble-up" in
+			//the dependency graph until all have been processed.
+
+			let round = 0;
+			let stable = false;
+
+			while (!stable) {
+				const signal = "_depCheck" + round++;
+
+				//estimate the type of update for the package
+				const nextType = resolveReleaseType(pkg, flags.deps.bump, flags.deps.release);
+
+				//indicate if it changed
+				pkg[signal] = pkg._nextType === nextType ? "stable" : "changed";
+				pkg._nextType = nextType;
+
+				await waitForAll(signal);
+
+				stable = packages.every((p) => p[signal] === "stable");
+			}
 
 			debug("commits analyzed: %s", pkg.name);
 			debug("release type: %s", pkg._nextType);

lib/updateDeps.js

@@ -124,58 +124,67 @@ const _nextPreHighestVersion = (latestTag, lastVersion, pkgPreRelease) => {
 	return bumpFromTags ? getHighestVersion(bumpFromLast, bumpFromTags) : bumpFromLast;
 };
 
+/**
+ * Returns the 'highest' type of release update, major > minor > patch > undefined.
+ * @param  {...string} releaseTypes types (patch | minor | major | undefined) of which the highest to return.
+ * @returns {string} release type considered highest
+ */
+const getHighestReleaseType = (...releaseTypes) =>
+	["major", "minor", "patch"].find((type) => releaseTypes.includes(type));
+
 /**
  * Resolve package release type taking into account the cascading dependency update.
  *
  * @param {Package} pkg Package object.
  * @param {string|undefined} bumpStrategy Dependency resolution strategy: override, satisfy, inherit.
  * @param {string|undefined} releaseStrategy Release type triggered by deps updating: patch, minor, major, inherit.
- * @param {Package[]} ignore=[] Packages to ignore (to prevent infinite loops when traversing graphs of dependencies).
  * @returns {string|undefined} Resolved release type.
  * @internal
  */
-const resolveReleaseType = (pkg, bumpStrategy = "override", releaseStrategy = "patch", ignore = []) => {
-	//make sure any dependency changes are resolved before returning the release type
-	if (!pkg._depsResolved) {
-		//create a list of dependencies that require change
-		pkg._depsChanged = pkg.localDeps
-			.filter((d) => !ignore.includes(d))
-			.filter((d) => needsDependencyUpdate(pkg, d, bumpStrategy, releaseStrategy, [pkg, ...ignore]));
-
-		//get the (preliminary) release type of the package based on release strategy (and analyzed changed dependencies)
-		pkg._nextType = getDependentRelease(pkg, releaseStrategy);
-
-		//indicate that all deps are resolved (fixates the next type and depsChanged)
-		pkg._depsResolved = ignore.length === 0;
+const resolveReleaseType = (pkg, bumpStrategy = "override", releaseStrategy = "patch") => {
+	// Define release type for dependent package if any of its deps changes.
+	// `patch`, `minor`, `major` — strictly declare the release type that occurs when any dependency is updated.
+	// `inherit` — applies the "highest" release of updated deps to the package.
+	// For example, if any dep has a breaking change, `major` release will be applied to the all dependants up the chain.
+
+	//create a list of dependencies that require change to the manifest
+	pkg._depsChanged = pkg.localDeps.filter((d) => needsDependencyUpdate(pkg, d, bumpStrategy));
+
+	//check if any dependencies have changed. If not return the current type of release
+	if (
+		!pkg._lastRelease || //not released yet
+		pkg._depsChanged.length === 0 || //no deps available
+		pkg._depsChanged.every((dep) => dep._lastRelease && !dep._nextType) //no new deps or deps upgraded
+	)
+		return pkg._nextType;
+
+	//find highest release type if strategy is inherit, starting of type set by commit analyzer
+	if (releaseStrategy === "inherit") {
+		return getHighestReleaseType(pkg._nextType, ...pkg._depsChanged.map((d) => d._nextType));
 	}
 
-	return pkg._nextType;
+	//set to highest of commit analyzer found change and releaseStrategy
+	//releaseStrategy of major could override local update of minor
+	return getHighestReleaseType(pkg._nextType, releaseStrategy);
 };
 
 /**
  * Indicates if the manifest file requires a change for the given dependency
  * @param {Package} pkg Package object.
+ * @param {Package} dependency dependency to check
  * @param {string|undefined} bumpStrategy Dependency resolution strategy: override, satisfy, inherit.
- * @param {string|undefined} releaseStrategy Release type triggered by deps updating: patch, minor, major, inherit.
- * @param {Package[]} ignore Packages to ignore (to prevent infinite loops).
+ * @returns {boolean } true if dependency needs to change
  */
-
-const needsDependencyUpdate = (pkg, dependency, bumpStrategy, releaseStrategy, ignore) => {
+const needsDependencyUpdate = (pkg, dependency, bumpStrategy) => {
 	//get last release of dependency
 	const depLastVersion = dependency._lastRelease && dependency._lastRelease.version;
 
-	// 3. check if dependency was released before (if not, this is assumed to be a new package + dependency)
+	//Check if dependency was released before (if not, this is assumed to be a new package + dependency)
 	const wasReleased = depLastVersion !== undefined;
 	if (!wasReleased) return true; //new packages always require a package re-release
 
-	//get nextType of the dependency (recursion occurs here!)
-	// Has changed if...
-	// 1. Any local dep package itself triggered changed
-	// 2. Any local dep package has local deps that triggered change.
-	const depNextType = resolveReleaseType(dependency, bumpStrategy, releaseStrategy, ignore);
-
 	//get estimated next version of dependency (which is lastVersion if no change expected)
-	const depNextVersion = depNextType
+	const depNextVersion = dependency._nextType
 		? dependency._preRelease
 			? getNextPreVersion(dependency)
 			: getNextVersion(dependency)
@@ -185,7 +194,7 @@ const needsDependencyUpdate = (pkg, dependency, bumpStrategy, releaseStrategy, i
 	const { dependencies = {}, devDependencies = {}, peerDependencies = {}, optionalDependencies = {} } = pkg.manifest;
 	const scopes = [dependencies, devDependencies, peerDependencies, optionalDependencies];
 
-	// 4. Check if the manifest dependency rules warrants an update (in any of the dependency scopes)
+	//Check if the manifest dependency rules warrants an update (in any of the dependency scopes)
 	const requireUpdate = scopes.some((scope) =>
 		manifestUpdateNecessary(scope, dependency.name, depNextVersion, bumpStrategy)
 	);
@@ -227,6 +236,9 @@ const manifestUpdateNecessary = (scope, name, nextVersion, bumpStrategy) => {
  * @internal
  */
 const resolveNextVersion = (currentVersion, nextVersion, bumpStrategy = "override") => {
+	//no change...
+	if (currentVersion === nextVersion) return currentVersion;
+
 	// Check the next pkg version against its current references.
 	// If it matches (`*` matches to any, `1.1.0` matches `1.1.x`, `1.5.0` matches to `^1.0.0` and so on)
 	// release will not be triggered, if not `override` strategy will be applied instead.
@@ -256,56 +268,16 @@ const resolveNextVersion = (currentVersion, nextVersion, bumpStrategy = "overrid
 	return nextVersion;
 };
 
-/**
- * Get dependent release type by analyzing the current nextType and changed dependencies
- * @param {Package} pkg The package to determine next type of release of
- * @param {string} releaseStrategy Release type triggered by deps updating: patch, minor, major, inherit.
- * @returns {string|undefined} Returns the highest release type if found, undefined otherwise
- * @internal
- */
-const getDependentRelease = (pkg, releaseStrategy) => {
-	const severityOrder = ["patch", "minor", "major"];
-
-	// Define release type for dependent package if any of its deps changes.
-	// `patch`, `minor`, `major` — strictly declare the release type that occurs when any dependency is updated.
-	// `inherit` — applies the "highest" release of updated deps to the package.
-	// For example, if any dep has a breaking change, `major` release will be applied to the all dependants up the chain.
-
-	//return type set by commit analyzer if no deps changed
-	if (
-		!pkg._lastRelease || //new package
-		!pkg._depsChanged || //no deps analyzed
-		pkg._depsChanged.length === 0 || //no deps available
-		pkg._depsChanged.every((dep) => !dep._nextType && dep._lastRelease) //no new deps or deps upgraded
-	)
-		return pkg._nextType;
-
-	if (releaseStrategy === "inherit") {
-		//find highest release type if strategy is inherit, starting of type set by commit analyzer
-		return pkg._depsChanged.reduce((maxReleaseType, dependency) => {
-			return severityOrder.indexOf(dependency._nextType) > severityOrder.indexOf(maxReleaseType)
-				? dependency._nextType
-				: maxReleaseType;
-		}, pkg._nextType);
-	}
-
-	//return highest of commit analyzer found change and releaseStrategy
-	//releaseStrategy of major could override local update of minor
-	return severityOrder.indexOf(pkg._nextType) > severityOrder.indexOf(releaseStrategy)
-		? pkg._nextType
-		: releaseStrategy;
-};
-
 /**
  * Update pkg deps.
  *
  * @param {Package} pkg The package this function is being called on.
+ * @param {boolean} writeOut Commit the package to the file store (set to false to suppres)
  * @returns {undefined}
  * @internal
  */
-const updateManifestDeps = (pkg) => {
+const updateManifestDeps = (pkg, writeOut = true) => {
 	const { manifest, path } = pkg;
-	const { indent, trailingWhitespace } = recognizeFormat(manifest.__contents__);
 
 	// Loop through changed deps to verify release consistency.
 	pkg._depsChanged.forEach((dependency) => {
@@ -329,11 +301,12 @@ const updateManifestDeps = (pkg) => {
 		});
 	});
 
-	if (!auditManifestChanges(manifest, path)) {
+	if (!writeOut || !auditManifestChanges(manifest, path)) {
 		return;
 	}
 
 	// Write package.json back out.
+	const { indent, trailingWhitespace } = recognizeFormat(manifest.__contents__);
 	writeFileSync(path, JSON.stringify(manifest, null, indent) + trailingWhitespace);
 };
 

test/fixtures/yarnWorkspacesMutualDependency/package.json

@@ -0,0 +1,20 @@
+{
+	"name": "msr-test-yarn",
+	"author": "Joost Reuzel",
+	"version": "0.0.0-semantically-released",
+	"private": true,
+	"license": "0BSD",
+	"engines": {
+		"node": ">=8.3"
+	},
+	"workspaces": [
+		"packages/*"
+	],
+	"release": {
+		"plugins": [
+			"@semantic-release/commit-analyzer",
+			"@semantic-release/release-notes-generator"
+		],
+		"noCi": true
+	}
+}

test/fixtures/yarnWorkspacesMutualDependency/packages/a/package.json

@@ -0,0 +1,7 @@
+{
+	"name": "msr-test-a",
+	"version": "1.0.0",
+	"dependencies": {
+		"msr-test-b": "1.0.0"
+	}
+}

test/fixtures/yarnWorkspacesMutualDependency/packages/b/package.json

@@ -0,0 +1,7 @@
+{
+	"name": "msr-test-b",
+	"version": "1.0.0",
+	"dependencies": {
+		"msr-test-a": "1.0.0"
+	}
+}

test/lib/multiSemanticRelease.test.js

@@ -1060,4 +1060,99 @@ describe("multiSemanticRelease()", () => {
 			message: expect.stringMatching("Package peerDependencies must be object"),
 		});
 	});
+
+	test("Changes in packages with mutual dependency", async () => {
+		// Create Git repo.
+		const cwd = gitInit();
+		// Initial commit.
+		copyDirectory(`test/fixtures/yarnWorkspacesMutualDependency/`, cwd);
+		const sha1 = gitCommitAll(cwd, "feat: Initial release");
+		gitTag(cwd, "msr-test-a@1.0.0");
+		gitTag(cwd, "msr-test-b@1.0.0");
+		// Second commit.
+		writeFileSync(`${cwd}/packages/a/aaa.txt`, "AAA");
+		const sha2 = gitCommitAll(cwd, "feat(aaa): Add missing text file");
+		const url = gitInitOrigin(cwd);
+		gitPush(cwd);
+
+		// Capture output.
+		const stdout = new WritableStreamBuffer();
+		const stderr = new WritableStreamBuffer();
+
+		// Call multiSemanticRelease()
+		// Doesn't include plugins that actually publish.
+		const multiSemanticRelease = require("../../");
+		const result = await multiSemanticRelease(
+			[`packages/a/package.json`, `packages/b/package.json`],
+			{},
+			{ cwd, stdout, stderr },
+			{ deps: { bump: "satisfy" }, dryRun: false }
+		);
+
+		// Get stdout and stderr output.
+		const err = stderr.getContentsAsString("utf8");
+		expect(err).toBe(false);
+		const out = stdout.getContentsAsString("utf8");
+		expect(out).toMatch("Started multirelease! Loading 2 packages...");
+		expect(out).toMatch("Loaded package msr-test-a");
+		expect(out).toMatch("Loaded package msr-test-b");
+		expect(out).toMatch("Queued 2 packages! Starting release...");
+		expect(out).toMatch("Created tag msr-test-a@1.1.0");
+		expect(out).toMatch("Created tag msr-test-b@1.0.1");
+		expect(out).toMatch("Released 2 of 2 packages, semantically!");
+
+		const a = 0;
+		const b = 1;
+		// A.
+		expect(result[a].name).toBe("msr-test-a");
+		expect(result[a].result.lastRelease).toMatchObject({
+			gitHead: sha1,
+			gitTag: "msr-test-a@1.0.0",
+			version: "1.0.0",
+		});
+		expect(result[a].result.nextRelease).toMatchObject({
+			gitHead: sha2,
+			gitTag: "msr-test-a@1.1.0",
+			type: "minor",
+			version: "1.1.0",
+		});
+		expect(result[a].result.nextRelease.notes).toMatch("# msr-test-a [1.1.0]");
+		expect(result[a].result.nextRelease.notes).toMatch("### Features\n\n* **aaa:** Add missing text file");
+		expect(result[a].result.nextRelease.notes).toMatch("### Dependencies\n\n* **msr-test-b:** upgraded to 1.0.1");
+
+		// B.
+		expect(result[b].name).toBe("msr-test-b");
+		expect(result[b].result.lastRelease).toEqual({
+			channels: [null],
+			gitHead: sha1,
+			gitTag: "msr-test-b@1.0.0",
+			name: "msr-test-b@1.0.0",
+			version: "1.0.0",
+		});
+		expect(result[b].result.nextRelease).toMatchObject({
+			gitHead: sha2,
+			gitTag: "msr-test-b@1.0.1",
+			type: "patch",
+			version: "1.0.1",
+		});
+		expect(result[b].result.nextRelease.notes).toMatch("# msr-test-b [1.0.1]");
+		expect(result[b].result.nextRelease.notes).not.toMatch("### Features");
+		expect(result[b].result.nextRelease.notes).not.toMatch("### Bug Fixes");
+		expect(result[b].result.nextRelease.notes).toMatch("### Dependencies\n\n* **msr-test-a:** upgraded to 1.1.0");
+
+		// ONLY 3 times.
+		expect(result[2]).toBe(undefined);
+
+		// Check manifests.
+		expect(require(`${cwd}/packages/a/package.json`)).toMatchObject({
+			dependencies: {
+				"msr-test-b": "1.0.1",
+			},
+		});
+		expect(require(`${cwd}/packages/b/package.json`)).toMatchObject({
+			dependencies: {
+				"msr-test-a": "1.1.0",
+			},
+		});
+	});
 });