fix: update a few release nits after trying it on the cli (#179)

- Correctly link workspace dep changelog entry to release even when release component is different than the package name - Update changelog entry also when rewriting workspaces deps - Clone repo during release process with `fetch-depth: 0` so we can have full commit history to build `AUTHORS` files - Removes `preversion`, `postversion`, and `prepublishOnly` scripts now that all tagging happens via `release-please` - Correctly version root when going from prerelease -> release Closes #178
npm · Sep 1, 2022 · 70782b3 · 70782b3
1 parent e851661
commit 70782b3
Show file tree

Hide file tree

Showing 12 changed files with 149 additions and 104 deletions.
diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml
@@ -49,6 +49,7 @@ jobs:
         run: echo "::set-output name=branch::${{ fromJSON(needs.release-please.outputs.pr).headBranchName }}"
       - uses: actions/checkout@v3
         with:
+          fetch-depth: 0
           ref: ${{ steps.ref.outputs.branch }}
       - name: Setup git user
         run: |

diff --git a/lib/content/pkg.json b/lib/content/pkg.json
@@ -6,13 +6,9 @@
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "{{npmBin}} run lint -- --fix",
-    "preversion": "{{npmBin}} test",
-    {{#if pkgPrivate}}
-    "postversion": "git push origin --follow-tags",
-    {{else}}
-    "postversion": "{{npmBin}} publish",
-    "prepublishOnly": "git push origin --follow-tags",
-    {{/if}}
+    "preversion": {{{del}}},
+    "postversion": {{{del}}},
+    "prepublishOnly": {{{del}}},
     "snap": "tap",
     "test": "tap",
     "posttest": "{{npmBin}} run lint",

diff --git a/lib/content/release-please.yml b/lib/content/release-please.yml
@@ -37,7 +37,7 @@ jobs:
       - name: Output ref
         id: ref
         run: echo "::set-output name=branch::$\{{ fromJSON(needs.release-please.outputs.pr).headBranchName }}"
-      {{> setupGit with=(obj ref="${{ steps.ref.outputs.branch }}")}}
+      {{> setupGit with=(obj ref="${{ steps.ref.outputs.branch }}" fetch-depth=0)}}
       {{> setupNode}}
       {{> setupDeps}}
       - name: Post pull request actions

diff --git a/lib/release-please/index.js b/lib/release-please/index.js
@@ -3,11 +3,15 @@ const logger = require('./logger.js')
 const ChangelogNotes = require('./changelog.js')
 const Version = require('./version.js')
 const WorkspaceDeps = require('./workspace-deps.js')
+const NodeWorkspace = require('./node-workspace.js')
 
 RP.setLogger(logger)
 RP.registerChangelogNotes('default', (options) => new ChangelogNotes(options))
 RP.registerVersioningStrategy('default', (options) => new Version(options))
-RP.registerPlugin('workspace-deps', (options) => new WorkspaceDeps(options))
+RP.registerPlugin('workspace-deps', (o) =>
+  new WorkspaceDeps(o.github, o.targetBranch, o.repositoryConfig))
+RP.registerPlugin('node-workspace', (o) =>
+  new NodeWorkspace(o.github, o.targetBranch, o.repositoryConfig))
 
 const main = async ({ repo: fullRepo, token, dryRun, branch }) => {
   if (!token) {

diff --git a/lib/release-please/node-workspace.js b/lib/release-please/node-workspace.js
@@ -0,0 +1,11 @@
+const Version = require('./version.js')
+const RP = require('release-please/build/src/plugins/node-workspace')
+
+module.exports = class NodeWorkspace extends RP.NodeWorkspace {
+  bumpVersion (pkg) {
+    // The default release please node-workspace plugin forces a patch
+    // bump for the root if it only includes workspace dep updates.
+    // This does the same thing except it respects the prerelease config.
+    return new Version(pkg).bump(pkg.version, [{ type: 'fix' }])
+  }
+}
diff --git a/lib/release-please/workspace-deps.js b/lib/release-please/workspace-deps.js
@@ -1,4 +1,6 @@
 const { ManifestPlugin } = require('release-please/build/src/plugin')
+const { Changelog } = require('release-please/build/src/updaters/changelog.js')
+const { PackageJson } = require('release-please/build/src/updaters/node/package-json.js')
 
 const matchLine = (line, re) => {
   const trimmed = line.trim().replace(/^[*\s]+/, '')
@@ -10,61 +12,86 @@ const matchLine = (line, re) => {
 
 module.exports = class WorkspaceDeps extends ManifestPlugin {
   run (pullRequests) {
-    for (const { pullRequest } of pullRequests) {
-      const depLinks = pullRequest.body.releaseData.reduce((acc, release) => {
-        if (release.component) {
-          const url = matchLine(release.notes.split('\n')[0], /\[[^\]]+\]\((.*?)\)/)
-          if (url) {
-            acc[release.component] = url[1]
+    try {
+      for (const { pullRequest } of pullRequests) {
+        const getChangelog = (release) => pullRequest.updates.find((u) => {
+          const isChangelog = u.updater instanceof Changelog
+          const isComponent = release.component && u.path.startsWith(release.component)
+          const isRoot = !release.component && !u.path.includes('/')
+          return isChangelog && (isComponent || isRoot)
+        })
+
+        const getComponent = (pkgName) => pullRequest.updates.find((u) => {
+          const isPkg = u.updater instanceof PackageJson
+          return isPkg && JSON.parse(u.updater.rawContent).name === pkgName
+        }).path.replace(/\/package\.json$/, '')
+
+        const depLinksByComponent = pullRequest.body.releaseData.reduce((acc, release) => {
+          if (release.component) {
+            const path = [
+              this.github.repository.owner,
+              this.github.repository.repo,
+              'releases',
+              'tag',
+              release.tag.toString(),
+            ]
+            acc[release.component] = `https://github.com/${path.join('/')}`
           }
-        }
-        return acc
-      }, {})
+          return acc
+        }, {})
 
-      for (const release of pullRequest.body.releaseData) {
-        const lines = release.notes.split('\n')
-        const newLines = []
+        for (const release of pullRequest.body.releaseData) {
+          const lines = release.notes.split('\n')
+          const newLines = []
 
-        let inWorkspaceDeps = false
-        let collectWorkspaceDeps = false
+          let inWorkspaceDeps = false
+          let collectWorkspaceDeps = false
 
-        for (const line of lines) {
-          if (matchLine(line, 'The following workspace dependencies were updated')) {
+          for (const line of lines) {
+            if (matchLine(line, 'The following workspace dependencies were updated')) {
             // We are in the section with our workspace deps
             // Set the flag and discard this line since we dont want it in the final output
-            inWorkspaceDeps = true
-          } else if (inWorkspaceDeps) {
-            if (collectWorkspaceDeps) {
-              const depMatch = matchLine(line, /^(\S+) bumped from \S+ to (\S+)$/)
-              if (depMatch) {
+              inWorkspaceDeps = true
+            } else if (inWorkspaceDeps) {
+              if (collectWorkspaceDeps) {
+                const depMatch = matchLine(line, /^(\S+) bumped from \S+ to (\S+)$/)
+                if (depMatch) {
                 // If we have a line that is a workspace dep update, then reformat
                 // it and save it to the new lines
-                const [, depName, newVersion] = depMatch
-                const depSpec = `\`${depName}@${newVersion}\``
-                const url = depLinks[depName]
-                newLines.push(`  * ${url ? `[${depSpec}](${url})` : depSpec}`)
-              } else {
+                  const [, depName, newVersion] = depMatch
+                  const depSpec = `\`${depName}@${newVersion}\``
+                  const url = depLinksByComponent[getComponent(depName)]
+                  newLines.push(`  * deps: [${depSpec}](${url})`)
+                } else {
                 // Anything else means we are done with dependencies so ignore
                 // this line and dont look for any more
-                collectWorkspaceDeps = false
-              }
-            } else if (matchLine(line, 'dependencies')) {
+                  collectWorkspaceDeps = false
+                }
+              } else if (matchLine(line, 'dependencies')) {
               // Only collect dependencies discard dev deps and everything else
-              collectWorkspaceDeps = true
-            } else if (matchLine(line, '') || matchLine(line, /^#/)) {
-              inWorkspaceDeps = false
+                collectWorkspaceDeps = true
+              } else if (matchLine(line, '') || matchLine(line, /^#/)) {
+                inWorkspaceDeps = false
+                newLines.push(line)
+              }
+            } else {
               newLines.push(line)
             }
-          } else {
-            newLines.push(line)
           }
-        }
 
-        const newNotes = newLines.join('\n').trim()
-        const emptyDeps = newNotes.match(/### Dependencies[\n]+(### .*)/m)
+          let newNotes = newLines.join('\n').trim()
+          const emptyDeps = newNotes.match(/### Dependencies[\n]+(### .*)/m)
+          if (emptyDeps) {
+            newNotes = newNotes.replace(emptyDeps[0], emptyDeps[1])
+          }
 
-        release.notes = emptyDeps ? newNotes.replace(emptyDeps[0], emptyDeps[1]) : newNotes
+          release.notes = newNotes
+          getChangelog(release).updater.changelogEntry = newNotes
+        }
       }
+    } catch {
+      // Always return pull requests even if we failed so
+      // we dont fail the release
     }
 
     return pullRequests

diff --git a/package.json b/package.json
@@ -12,9 +12,6 @@
     "lint": "eslint \"**/*.js\"",
     "lintfix": "npm run lint -- --fix",
     "posttest": "npm run lint",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
-    "preversion": "npm test",
     "snap": "tap",
     "test": "tap",
     "template-oss-apply": "template-oss-apply --force",

diff --git a/tap-snapshots/test/apply/full-content.js.test.cjs b/tap-snapshots/test/apply/full-content.js.test.cjs
@@ -470,6 +470,7 @@ jobs:
         run: echo "::set-output name=branch::\${{ fromJSON(needs.release-please.outputs.pr).headBranchName }}"
       - uses: actions/checkout@v3
         with:
+          fetch-depth: 0
           ref: \${{ steps.ref.outputs.branch }}
       - name: Setup git user
         run: |
@@ -678,9 +679,6 @@ package.json
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -1398,6 +1396,7 @@ jobs:
         run: echo "::set-output name=branch::\${{ fromJSON(needs.release-please.outputs.pr).headBranchName }}"
       - uses: actions/checkout@v3
         with:
+          fetch-depth: 0
           ref: \${{ steps.ref.outputs.branch }}
       - name: Setup git user
         run: |
@@ -1613,9 +1612,6 @@ package.json
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -1737,9 +1733,6 @@ workspaces/a/package.json
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -1816,9 +1809,6 @@ workspaces/b/package.json
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -2117,6 +2107,7 @@ jobs:
         run: echo "::set-output name=branch::\${{ fromJSON(needs.release-please.outputs.pr).headBranchName }}"
       - uses: actions/checkout@v3
         with:
+          fetch-depth: 0
           ref: \${{ steps.ref.outputs.branch }}
       - name: Setup git user
         run: |
@@ -2375,9 +2366,6 @@ workspaces/a/package.json
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -2450,9 +2438,6 @@ workspaces/b/package.json
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"

diff --git a/tap-snapshots/test/check/diffs.js.test.cjs b/tap-snapshots/test/check/diffs.js.test.cjs
@@ -434,7 +434,7 @@ The module file package.json needs to be updated:
   "author" is "heynow", expected "GitHub Inc."
   "files[1]" is "x", expected "lib/"
   "scripts.lint:fix" is "x", expected to be removed
-  "scripts.preversion" is "x", expected "npm test"
+  "scripts.preversion" is "x", expected to be removed
   "standard" is {
     "config": "x "
   }, expected to be removed

diff --git a/tap-snapshots/test/check/index.js.test.cjs b/tap-snapshots/test/check/index.js.test.cjs
@@ -61,9 +61,6 @@ The module file package.json needs to be updated:
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -144,9 +141,6 @@ The module file package.json needs to be updated:
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -209,9 +203,6 @@ The module file package.json needs to be updated:
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"
@@ -274,9 +265,6 @@ The module file package.json needs to be updated:
     "postlint": "template-oss-check",
     "template-oss-apply": "template-oss-apply --force",
     "lintfix": "npm run lint -- --fix",
-    "preversion": "npm test",
-    "postversion": "npm publish",
-    "prepublishOnly": "git push origin --follow-tags",
     "snap": "tap",
     "test": "tap",
     "posttest": "npm run lint"

diff --git a/test/apply/index.js b/test/apply/index.js
@@ -113,13 +113,10 @@ t.test('private workspace', async (t) => {
   const rpManifest = await s.readJson('.release-please-manifest.json')
   const rpConfig = await s.readJson('release-please-config.json')
 
-  t.ok(pkg.scripts.prepublishOnly)
-  t.ok(pkg.scripts.postversion)
-
+  t.notOk(pkg.scripts.prepublishOnly)
+  t.notOk(pkg.scripts.postversion)
   t.notOk(privatePkg.scripts.prepublishOnly)
-  t.ok(privatePkg.scripts.postversion)
-
-  t.equal(pkg.scripts.prepublishOnly, privatePkg.scripts.postversion)
+  t.notOk(privatePkg.scripts.postversion)
 
   t.equal(rpManifest['.'], '1.0.0')
   t.equal(rpManifest['workspaces/b'], '1.0.0')