fix: npm-version does not git-commit nor git-tag when package in subdirectory

.github/workflows/ci.yml

@@ -85,6 +85,10 @@ jobs:
         shell: ${{ matrix.platform.shell }}
     steps:
     - uses: actions/checkout@v3
+    - name: Setup git user
+      run: |
+        git config --global user.email "ops+npm-cli@npmjs.com"
+        git config --global user.name "npm cli ops bot"
     - name: Use Node.js ${{ matrix.node-version }}
       uses: actions/setup-node@v3
       with:

docs/content/commands/npm-version.md

@@ -186,11 +186,12 @@ of `patch`, `minor`, `major`, `prepatch`, `preminor`, `premajor`,
 be incremented by 1 in the specified field.  `from-git` will try to read
 the latest git tag, and use that as the new npm version.
 
-If run in a git repo, it will also create a version commit and tag.  This
-behavior is controlled by `git-tag-version` (see below), and can be
-disabled on the command line by running `npm --no-git-tag-version version`.
-It will fail if the working directory is not clean, unless the `-f` or
-`--force` flag is set.
+If run in a git repo, it will also create a version commit and tag, unless
+the working directory is within a workspace (in which case no commit is
+made).  This behavior is controlled by `git-tag-version` (see below), and
+can be disabled on the command line by running `npm --no-git-tag-version
+version`.  It will fail if the working directory is not clean, unless the
+`-f` or `--force` flag is set.
 
 If supplied with `-m` or `--message` config option, npm will use it as a
 commit message when creating a version commit.  If the `message` config

node_modules/@npmcli/map-workspaces/lib/index.js

@@ -12,7 +12,7 @@ function appendNegatedPatterns (patterns) {
   for (let pattern of patterns) {
     const excl = pattern.match(/^!+/)
     if (excl) {
-      pattern = pattern.substr(excl[0].length)
+      pattern = pattern.slice(excl[0].length)
     }
 
     // strip off any / from the start of the pattern.  /foo => foo

node_modules/@npmcli/map-workspaces/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@npmcli/map-workspaces",
-  "version": "2.0.3",
+  "version": "2.0.4",
   "main": "lib/index.js",
   "files": [
     "bin/",
@@ -42,7 +42,7 @@
   },
   "devDependencies": {
     "@npmcli/eslint-config": "^3.0.1",
-    "@npmcli/template-oss": "3.4.1",
+    "@npmcli/template-oss": "3.5.0",
     "tap": "^16.0.1"
   },
   "dependencies": {
@@ -53,6 +53,6 @@
   },
   "templateOSS": {
     "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.",
-    "version": "3.4.1"
+    "version": "3.5.0"
   }
 }

node_modules/libnpmversion

@@ -0,0 +1 @@
+../workspaces/libnpmversion

package-lock.json

@@ -962,9 +962,9 @@
       }
     },
     "node_modules/@npmcli/map-workspaces": {
-      "version": "2.0.3",
-      "resolved": "https://registry.npmjs.org/@npmcli/map-workspaces/-/map-workspaces-2.0.3.tgz",
-      "integrity": "sha512-X6suAun5QyupNM8iHkNPh0AHdRC2rb1W+MTdMvvA/2ixgmqZwlq5cGUBgmKHUHT2LgrkKJMAXbfAoTxOigpK8Q==",
+      "version": "2.0.4",
+      "resolved": "https://registry.npmjs.org/@npmcli/map-workspaces/-/map-workspaces-2.0.4.tgz",
+      "integrity": "sha512-bMo0aAfwhVwqoVM5UzX1DJnlvVvzDCHae821jv48L1EsrYwfOZChlqWYXEtto/+BkBXetPbEWgau++/brh4oVg==",
       "inBundle": true,
       "dependencies": {
         "@npmcli/name-from-folder": "^1.0.1",
@@ -10233,6 +10233,7 @@
       "license": "ISC",
       "dependencies": {
         "@npmcli/git": "^3.0.0",
+        "@npmcli/map-workspaces": "^2.0.4",
         "@npmcli/run-script": "^3.0.0",
         "json-parse-even-better-errors": "^2.3.1",
         "proc-log": "^2.0.0",
@@ -10817,9 +10818,9 @@
       }
     },
     "@npmcli/map-workspaces": {
-      "version": "2.0.3",
-      "resolved": "https://registry.npmjs.org/@npmcli/map-workspaces/-/map-workspaces-2.0.3.tgz",
-      "integrity": "sha512-X6suAun5QyupNM8iHkNPh0AHdRC2rb1W+MTdMvvA/2ixgmqZwlq5cGUBgmKHUHT2LgrkKJMAXbfAoTxOigpK8Q==",
+      "version": "2.0.4",
+      "resolved": "https://registry.npmjs.org/@npmcli/map-workspaces/-/map-workspaces-2.0.4.tgz",
+      "integrity": "sha512-bMo0aAfwhVwqoVM5UzX1DJnlvVvzDCHae821jv48L1EsrYwfOZChlqWYXEtto/+BkBXetPbEWgau++/brh4oVg==",
       "requires": {
         "@npmcli/name-from-folder": "^1.0.1",
         "glob": "^8.0.1",
@@ -13152,6 +13153,7 @@
       "requires": {
         "@npmcli/eslint-config": "^3.0.1",
         "@npmcli/git": "^3.0.0",
+        "@npmcli/map-workspaces": "^2.0.4",
         "@npmcli/run-script": "^3.0.0",
         "@npmcli/template-oss": "3.5.0",
         "json-parse-even-better-errors": "^2.3.1",

workspaces/libnpmversion/lib/is-workspace-safe.js

@@ -0,0 +1,33 @@
+const { dirname } = require('path')
+const mapWorkspaces = require('@npmcli/map-workspaces')
+const readJson = require('./read-json.js')
+
+// returns false if cwd is within a workspace
+module.exports = async (gitDir, path, cwd = process.cwd()) => {
+  // doesn't matter
+  if (gitDir == null) {
+    return true
+  }
+  // always safe to commit from git root
+  if (cwd === gitDir) {
+    return true
+  }
+  // if your path's package.json contains workspaces, you are in a top level package, not a workspace
+  var rpj = await readJson(`${path}/package.json`)
+  if (rpj.workspaces) {
+    return true
+  }
+  // loop up, looking for a package.json with workspaces
+  while (cwd !== gitDir && cwd !== dirname(cwd)) {
+    cwd = dirname(cwd)
+    try {
+      rpj = await readJson(`${cwd}/package.json`)
+    } catch(er) { continue }
+    const workspaceMap = await mapWorkspaces({cwd, pkg: rpj})
+    const mapValues = [...workspaceMap.values()]
+    if (mapValues.includes(path)) {
+        return false
+    }
+  }
+  return true
+}

workspaces/libnpmversion/lib/version.js

@@ -9,6 +9,7 @@ const git = require('@npmcli/git')
 const commit = require('./commit.js')
 const tag = require('./tag.js')
 const log = require('proc-log')
+const isWorkspaceSafe = require('./is-workspace-safe.js')
 
 const runScript = require('@npmcli/run-script')
 
@@ -51,13 +52,13 @@ module.exports = async (newversion, opts) => {
     })
   }
 
-  const isGitDir = newversion === 'from-git' || await git.is(opts)
+  const gitDir = await git.find(opts)
 
   // ok!  now we know the new version, and the old version is in pkg
 
   // - check if git dir is clean
   // returns false if we should not keep doing git stuff
-  const doGit = gitTagVersion && isGitDir && await enforceClean(opts)
+  const doGit = gitTagVersion && Boolean(gitDir) && await enforceClean(opts) && await isWorkspaceSafe(gitDir, path)
 
   if (!ignoreScripts) {
     await runScript({

workspaces/libnpmversion/package.json

@@ -37,6 +37,7 @@
   },
   "dependencies": {
     "@npmcli/git": "^3.0.0",
+    "@npmcli/map-workspaces": "^2.0.4",
     "@npmcli/run-script": "^3.0.0",
     "json-parse-even-better-errors": "^2.3.1",
     "proc-log": "^2.0.0",

workspaces/libnpmversion/test/is-workspace-safe.js

@@ -0,0 +1,54 @@
+const t = require('tap')
+const isWorkspaceSafe = require('../lib/is-workspace-safe.js')
+const path = require('path')
+
+t.cleanSnapshot = s => s.replace(/\\+/g, '/')
+
+t.test('all the potential states', async t => {
+  const pkg = {
+    name: 'foo'
+  }
+  const dir = t.testdir({
+    'git': {
+      'package.json': JSON.stringify({
+        ...pkg,
+        workspaces: [
+          "packages/a"
+        ]
+      }, null, 2),
+    },
+    'git/packages/a': {
+      'package.json': JSON.stringify({
+        ...pkg
+      }, null, 2),
+    },
+    'git/other': {},
+    'not-git': {},
+    'git-too': {
+      'package.json': JSON.stringify({
+        ...pkg
+      }, null, 2),
+    },
+    'git-too/subdir': {}
+  })
+
+  await t.test('no git', async t => {
+    t.ok(await isWorkspaceSafe(null, `${dir}/not-git`), 'should be safe because there is no git')
+  })
+
+  await t.test('is git root', async t => {
+    t.ok(await isWorkspaceSafe(`${dir}/git`, `${dir}/git`, `${dir}/git`), 'should be safe because cwd is git root')
+  })
+
+  await t.test('top level package has workspaces', async t => {
+    t.ok(await isWorkspaceSafe(`${dir}/git`, `${dir}/git`, `${dir}/git/other`), 'should be safe because we see the workspaces')
+  })
+
+  await t.test('in workspace', async t => {
+    t.notOk(await isWorkspaceSafe(`${dir}/git`, `${dir}${path.sep}git${path.sep}packages${path.sep}a`, `${dir}${path.sep}git${path.sep}packages${path.sep}a`), 'should return false for being in a workspace')
+  })
+
+  await t.test('no workspaces', async t=> {
+    t.ok(await isWorkspaceSafe(`${dir}/git-too`, `${dir}/git-too`, `${dir}/git-too/subdir`), 'should be safe because of no workspaces')
+  })
+})

workspaces/libnpmversion/test/version.js

@@ -4,6 +4,7 @@ const requireInject = require('require-inject')
 const actionLog = []
 
 const gitMock = {
+  find: async opts => !opts.path.includes('not-git') ? 'git' : null,
   is: async opts => !/\bnot-git$/.test(opts.path),
   spawn: async (args, opts) => actionLog.push(['spawn', args, opts]),
 }
@@ -14,7 +15,7 @@ const version = requireInject('../lib/version.js', {
   '../lib/commit.js': async (v, opts) => actionLog.push(['commit', v, opts]),
   '../lib/tag.js': async (v, opts) => actionLog.push(['tag', v, opts]),
   '../lib/retrieve-tag.js': async (opts) => {
-    if (/\bnot-git$/.test(opts.path)) {
+    if (opts.path.includes('not-git')) {
       throw new Error('not a git dir')
     }
     actionLog.push(['retrieve-tag', opts])
@@ -40,6 +41,21 @@ t.test('test out bumping the version in all the ways', async t => {
 
   const dir = t.testdir({
     git: {
+      'package.json': JSON.stringify({
+        ...pkg,
+        workspaces: [
+          "packages/a"
+        ],
+      }, null, 2),
+      'package-lock.json': JSON.stringify(lock, null, 2),
+    },
+    'git/packages/a': {
+      'package.json': JSON.stringify({
+        ...pkg,
+      }, null, 2),
+      'package-lock.json': JSON.stringify(lock, null, 2),
+    },
+    'git/packages/a': {
       'package-lock.json': JSON.stringify(lock, null, 2),
     },
     'not-git': {
@@ -49,14 +65,36 @@ t.test('test out bumping the version in all the ways', async t => {
           '': { ...pkg },
         },
       }, null, 2),
+      'package.json': JSON.stringify({
+        ...pkg,
+        workspaces: [
+          "packages/b"
+        ]
+      }, null, 2),
+    },
+    'not-git/packages/b': {
+      'npm-shrinkwrap.json': JSON.stringify({
+        ...lock,
+        packages: {
+          '': { ...pkg },
+        },
+      }, null, 2),
+    },
+    'not-git/packages/b': {
+      'npm-shrinkwrap.json': JSON.stringify({
+        ...lock,
+        packages: {
+          '': { ...pkg },
+        },
+      }, null, 2),
     },
   })
 
   await t.test('git dir', async t => {
     t.afterEach(async () => {
       actionLog.length = 0
     })
-    const path = `${dir}/git`
+    var path = `${dir}/git`
     await t.test('major', async t => {
       // for this one, let's pretend that the package-lock.json is .gitignored
       const { spawn } = gitMock
@@ -153,6 +191,8 @@ t.test('test out bumping the version in all the ways', async t => {
       ])
       t.equal(pkg.version, '2.2.0')
     })
+    // for these, let's test subdirectories
+    path = `${dir}/git/packages/a`
     await t.test('explicit version', async t => {
       t.equal(await version('=v3.2.1', { path, pkg, gitTagVersion: true }), '3.2.1')
       t.match(actionLog, [
@@ -238,7 +278,7 @@ t.test('test out bumping the version in all the ways', async t => {
     t.afterEach(async () => {
       actionLog.length = 0
     })
-    const path = `${dir}/not-git`
+    var path = `${dir}/not-git`
     await t.test('major', async t => {
       t.equal(await version('major', { path, pkg }), '2.0.0')
       t.match(actionLog, [
@@ -312,6 +352,8 @@ t.test('test out bumping the version in all the ways', async t => {
       ])
       t.equal(pkg.version, '3.2.1')
     })
+    // for these, let's test subdirectories
+    path = `${dir}/not-git/packages/b`
     await t.test('invalid version', async t => {
       await t.rejects(version('invalid version', { path, pkg }), {
         message: 'Invalid version: invalid version',