From 8b2e7382cd28c7d25dfaa320b3878443187fee88 Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 2 May 2024 21:40:05 -0700 Subject: [PATCH] fix: remove custom node-workspace plugin release-please now has all the fixes we need and we can use the upstream plugin --- lib/release/node-workspace-format.js | 37 ++++++---- lib/release/node-workspace.js | 103 --------------------------- lib/release/release-please.js | 3 - 3 files changed, 25 insertions(+), 118 deletions(-) delete mode 100644 lib/release/node-workspace.js diff --git a/lib/release/node-workspace-format.js b/lib/release/node-workspace-format.js index 4eca1ea5..57135d1a 100644 --- a/lib/release/node-workspace-format.js +++ b/lib/release/node-workspace-format.js @@ -3,7 +3,7 @@ const { ManifestPlugin } = require('release-please/build/src/plugin.js') const { addPath } = require('release-please/build/src/plugins/workspace.js') const { TagName } = require('release-please/build/src/util/tag-name.js') const { ROOT_PROJECT_PATH } = require('release-please/build/src/manifest.js') -const { DEPS, link, wrapSpecs } = require('./util.js') +const { DEPS, link, wrapSpecs, list } = require('./util.js') /* istanbul ignore next: TODO fix flaky tests and enable coverage */ module.exports = class extends ManifestPlugin { @@ -11,7 +11,6 @@ module.exports = class extends ManifestPlugin { #releasesByPackage = new Map() #pathsByComponent = new Map() - #WORKSPACE_SCOPE = /(?workspace): `?(?\S+?)[@\s](?\S+?)`?$/gm async preconfigure (strategiesByPath) { // First build a list of all releases that will happen based on @@ -32,6 +31,16 @@ module.exports = class extends ManifestPlugin { return candidates } + #replaceWorkspace ({ name, versionRange }) { + const version = versionRange.replace(/^[\^~]/, '') + const { path, component } = this.#releasesByPackage.get(name) + const { tagSeparator, includeVInTag } = this.repositoryConfig[path] + const { repository: { owner, repo } } = this.github + const tag = new TagName(version, component, tagSeparator, includeVInTag).toString() + const url = `https://github.com/${owner}/${repo}/releases/tag/${tag}` + return list(`${link('workspace', url)}: ${wrapSpecs(`${name}@${version}`)}`) + } + // I don't like how release-please formats workspace changelog entries // so this rewrites them to look like the rest of our changelog. This can't // be part of the changelog plugin since they are written after that by the @@ -42,16 +51,20 @@ module.exports = class extends ManifestPlugin { for (const release of candidate.pullRequest.body.releaseData) { // Update notes with a link to each workspaces release notes // now that we have all of the releases in a single pull request - release.notes = - release.notes.replace(this.#WORKSPACE_SCOPE, (...args) => { - const { scope, name, version } = args.pop() - const { path, component } = this.#releasesByPackage.get(name) - const { tagSeparator, includeVInTag } = this.repositoryConfig[path] - const { repository: { owner, repo } } = this.github - const tag = new TagName(version, component, tagSeparator, includeVInTag).toString() - const url = `https://github.com/${owner}/${repo}/releases/tag/${tag}` - return `${link(scope, url)}: ${wrapSpecs(`${name}@${version}`)}` - }) + release.notes = release.notes + .replace(/^\* The following workspace dependencies were updated\n/gm, '') + .replace(/^\s{2}\* dependencies\n/gm, '') + .replace(/^\s{2}\* devDependencies\n/gm, '') + .replace(/^\s{2}\* peerDependencies\n/gm, '') + .replace(/^\s{2}\* optionalDependencies\n/gm, '') + .replace( + /^\s{4}\* (?[^\s]+) bumped to (?[^\s]+)/gm, + (...args) => this.#replaceWorkspace(args.at(-1)) + ) + .replace( + /^\s{4}\* (?[^\s]+) bumped from (?:[^\s]+) to (?[^\s]+)/gm, + (...args) => this.#replaceWorkspace(args.at(-1)) + ) // Find the associated changelog and update that too const path = this.#pathsByComponent.get(release.component) diff --git a/lib/release/node-workspace.js b/lib/release/node-workspace.js deleted file mode 100644 index 93ef00f6..00000000 --- a/lib/release/node-workspace.js +++ /dev/null @@ -1,103 +0,0 @@ -const { NodeWorkspace } = require('release-please/build/src/plugins/node-workspace') -const { parseConventionalCommits } = require('release-please/build/src/commit') -const { DEPS } = require('./util') -const { WORKSPACE_MESSAGE } = require('./node-workspace-format') - -// This adds a preconfigure method to the release-please node-workspace plugin -// which fixes https://github.com/googleapis/release-please/issues/2089 for our -// use case. We should attempt to upstream this to release-please but it -// fundamentally changes the way the node-workspace plugin behaves so it might -// not be easy to land. For now we extend the base plugin and add one method -// which is much better than previously when we needed to fork and maintain -// release-please ourselves. -/* istanbul ignore next: TODO fix flaky tests and enable coverage */ -class NpmNodeWorkspace extends NodeWorkspace { - updateCandidate (pr) { - // no-op so release-please node-workspace plugin does not add - // its broken changelog to the workspace releases. our changelog - // formatting and the preconfigure method below fix this. - return pr - } - - async preconfigure (strategiesByPath, commitsByPath, releasesByPath) { - // First build a list of all releases that will happen based on the - // conventional commits - const candidates = [] - for (const path in strategiesByPath) { - const pullRequest = await strategiesByPath[path].buildReleasePullRequest( - parseConventionalCommits(commitsByPath[path]), - releasesByPath[path] - ) - // Release please types say this sometimes will return undefined but I could not - // get any scenario where that was the case. If it was undefined we would want to - // just ignore it anyway. - /* istanbul ignore else */ - if (pullRequest) { - candidates.push({ - path, - pullRequest, - config: this.repositoryConfig[path], - }) - } - } - - // Then build the graph of all those releases + any other connected workspaces - const { allPackages, candidatesByPackage } = await this.buildAllPackages(candidates) - const graph = await this.buildGraph(allPackages) - const packageNamesToUpdate = this.packageNamesToUpdate(graph, candidatesByPackage) - const graphPackages = this.buildGraphOrder(graph, packageNamesToUpdate) - - // Then build a list of all the updated versions - const updatedVersions = {} - for (const pkg of graphPackages) { - const path = this.pathFromPackage(pkg) - const packageName = this.packageNameFromPackage(pkg) - const existingCandidate = candidatesByPackage[packageName] - - if (existingCandidate) { - // If there is an existing pull request use that version - updatedVersions[packageName] = existingCandidate.pullRequest?.version - } else { - // Otherwise build another pull request (that will be discarded) just - // to see what the version would be if it only contained a deps commit. - // This is to make sure we use any custom versioning or release strategy. - const releasePullRequest = await strategiesByPath[path].buildReleasePullRequest( - parseConventionalCommits([{ message: `${DEPS}: ${Math.random()}` }]), - releasesByPath[path] - ) - updatedVersions[packageName] = releasePullRequest?.version - } - } - - // Then go through all the packages again and add deps commits for each - // updated workspace - for (const pkg of graphPackages) { - const path = this.pathFromPackage(pkg) - const packageName = this.packageNameFromPackage(pkg) - const graphPackage = graph.get(packageName) - - // Update dependency versions add a deps commit to each path so it gets - // processed later. This else never happens in our cases because our - // packages always have deps, but keeping this around to make it easier to - // upstream in the future. - /* istanbul ignore else */ - if (graphPackage.deps) { - for (const depName of graphPackage.deps) { - const depVersion = updatedVersions[depName] - // Same as the above check, we always have a version here but technically - // we could not in which it would be safe to ignore it. - /* istanbul ignore else */ - if (depVersion) { - commitsByPath[path].push({ - message: WORKSPACE_MESSAGE(depName, depVersion.toString()), - }) - } - } - } - } - - return strategiesByPath - } -} - -module.exports = NpmNodeWorkspace diff --git a/lib/release/release-please.js b/lib/release/release-please.js index 42cd3bb9..96811c2b 100644 --- a/lib/release/release-please.js +++ b/lib/release/release-please.js @@ -11,7 +11,6 @@ const assert = require('assert') const core = require('@actions/core') const omit = require('just-omit') const ChangelogNotes = require('./changelog.js') -const NodeWorkspace = require('./node-workspace.js') const NodeWorkspaceFormat = require('./node-workspace-format.js') const { getPublishTag, noop } = require('./util.js') @@ -69,8 +68,6 @@ class ReleasePlease { new ChangelogNotes(github, o)) RP.registerVersioningStrategy('default', (o) => o.prerelease ? new PrereleaseVersioningStrategy(o) : new DefaultVersioningStrategy(o)) - RP.registerPlugin('node-workspace', ({ github, targetBranch, repositoryConfig, ...o }) => - new NodeWorkspace(github, targetBranch, repositoryConfig, o)) RP.registerPlugin('node-workspace-format', ({ github, targetBranch, repositoryConfig, ...o }) => new NodeWorkspaceFormat(github, targetBranch, repositoryConfig, o))