feat: make npm owner workspace aware (#4835)

wraithgar · web-flow · commit 62af3a1dc003 · 2022-05-03T13:42:22.000-07:00
diff --git a/docs/content/commands/npm-owner.md b/docs/content/commands/npm-owner.md
@@ -73,6 +73,52 @@ password, npm will prompt on the command line for one.
 <!-- automatically generated, do not edit manually -->
 <!-- see lib/utils/config/definitions.js -->
 
+#### `workspace`
+
+* Default:
+* Type: String (can be set multiple times)
+
+Enable running a command in the context of the configured workspaces of the
+current project while filtering by running only the workspaces defined by
+this configuration option.
+
+Valid values for the `workspace` config are either:
+
+* Workspace names
+* Path to a workspace directory
+* Path to a parent workspace directory (will result in selecting all
+  workspaces within that folder)
+
+When set for the `npm init` command, this may be set to the folder of a
+workspace which does not yet exist, to create the folder and set it up as a
+brand new workspace within the project.
+
+This value is not exported to the environment for child processes.
+
+<!-- automatically generated, do not edit manually -->
+<!-- see lib/utils/config/definitions.js -->
+
+#### `workspaces`
+
+* Default: null
+* Type: null or Boolean
+
+Set to true to run the command in the context of **all** configured
+workspaces.
+
+Explicitly setting this to false will cause commands like `install` to
+ignore workspaces altogether. When not set explicitly:
+
+- Commands that operate on the `node_modules` tree (install, update, etc.)
+will link workspaces into the `node_modules` folder. - Commands that do
+other things (test, exec, publish, etc.) will operate on the root project,
+_unless_ one or more workspaces are specified in the `workspace` config.
+
+This value is not exported to the environment for child processes.
+
+<!-- automatically generated, do not edit manually -->
+<!-- see lib/utils/config/definitions.js -->
+
 <!-- AUTOGENERATED CONFIG DESCRIPTIONS END -->
 
 ### See Also
diff --git a/lib/commands/owner.js b/lib/commands/owner.js
@@ -22,6 +22,8 @@ class Owner extends BaseCommand {
   static params = [
     'registry',
     'otp',
+    'workspace',
+    'workspaces',
   ]
 
   static usage = [
@@ -69,22 +71,43 @@ class Owner extends BaseCommand {
   }
 
   async exec ([action, ...args]) {
-    switch (action) {
-      case 'ls':
-      case 'list':
-        return this.ls(args[0])
-      case 'add':
-        return this.changeOwners(args[0], args[1], 'add')
-      case 'rm':
-      case 'remove':
-        return this.changeOwners(args[0], args[1], 'rm')
-      default:
+    if (action === 'ls' || action === 'list') {
+      await this.ls(args[0])
+    } else if (action === 'add') {
+      await this.changeOwners(args[0], args[1], 'add')
+    } else if (action === 'rm' || action === 'remove') {
+      await this.changeOwners(args[0], args[1], 'rm')
+    } else {
+      throw this.usageError()
+    }
+  }
+
+  async execWorkspaces ([action, ...args], filters) {
+    await this.setWorkspaces(filters)
+    // ls pkg or owner add/rm package
+    if ((action === 'ls' && args.length > 0) || args.length > 1) {
+      const implicitWorkspaces = this.npm.config.get('workspace', 'default')
+      if (implicitWorkspaces.length === 0) {
+        log.warn(`Ignoring specified workspace(s)`)
+      }
+      return this.exec([action, ...args])
+    }
+
+    for (const [name] of this.workspaces) {
+      if (action === 'ls' || action === 'list') {
+        await this.ls(name)
+      } else if (action === 'add') {
+        await this.changeOwners(args[0], name, 'add')
+      } else if (action === 'rm' || action === 'remove') {
+        await this.changeOwners(args[0], name, 'rm')
+      } else {
         throw this.usageError()
+      }
     }
   }
 
   async ls (pkg) {
-    pkg = await this.getPkg(pkg)
+    pkg = await this.getPkg(this.npm.prefix, pkg)
     const spec = npa(pkg)
 
     try {
@@ -101,12 +124,12 @@ class Owner extends BaseCommand {
     }
   }
 
-  async getPkg (pkg) {
+  async getPkg (prefix, pkg) {
     if (!pkg) {
       if (this.npm.config.get('global')) {
         throw this.usageError()
       }
-      const { name } = await readJson(resolve(this.npm.prefix, 'package.json'))
+      const { name } = await readJson(resolve(prefix, 'package.json'))
       if (!name) {
         throw this.usageError()
       }
@@ -121,7 +144,7 @@ class Owner extends BaseCommand {
       throw this.usageError()
     }
 
-    pkg = await this.getPkg(pkg)
+    pkg = await this.getPkg(this.npm.prefix, pkg)
     log.verbose(`owner ${addOrRm}`, '%s to %s', user, pkg)
 
     const spec = npa(pkg)
diff --git a/tap-snapshots/test/lib/load-all-commands.js.test.cjs b/tap-snapshots/test/lib/load-all-commands.js.test.cjs
@@ -596,6 +596,8 @@ npm owner ls [<@scope>/]<pkg>
 
 Options:
 [--registry <registry>] [--otp <otp>]
+[-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
+[-ws|--workspaces]
 
 alias: author
 
diff --git a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs
@@ -664,6 +664,8 @@ All commands:
                     
                     Options:
                     [--registry <registry>] [--otp <otp>]
+                    [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
+                    [-ws|--workspaces]
                     
                     alias: author
                     
diff --git a/test/lib/commands/owner.js b/test/lib/commands/owner.js
@@ -2,6 +2,7 @@ const t = require('tap')
 const { load: loadMockNpm } = require('../../fixtures/mock-npm.js')
 const MockRegistry = require('../../fixtures/mock-registry.js')
 
+const path = require('path')
 const npa = require('npm-package-arg')
 const packageName = '@npmcli/test-package'
 const spec = npa(packageName)
@@ -12,6 +13,42 @@ const maintainers = [
   { email: 'test-user-b@npmjs.org', name: 'test-user-b' },
 ]
 
+const workspaceFixture = {
+  'package.json': JSON.stringify({
+    name: packageName,
+    version: '1.2.3-test',
+    workspaces: ['workspace-a', 'workspace-b', 'workspace-c'],
+  }),
+  'workspace-a': {
+    'package.json': JSON.stringify({
+      name: 'workspace-a',
+      version: '1.2.3-a',
+    }),
+  },
+  'workspace-b': {
+    'package.json': JSON.stringify({
+      name: 'workspace-b',
+      version: '1.2.3-n',
+    }),
+  },
+  'workspace-c': JSON.stringify({
+    'package.json': {
+      name: 'workspace-n',
+      version: '1.2.3-n',
+    },
+  }),
+}
+
+function registryPackage (t, registry, name) {
+  const mockRegistry = new MockRegistry({ tap: t, registry })
+
+  const manifest = mockRegistry.manifest({
+    name,
+    packuments: [{ maintainers, version: '1.0.0' }],
+  })
+  mockRegistry.package({ manifest })
+}
+
 t.test('owner no args', async t => {
   const { npm } = await loadMockNpm(t)
   await t.rejects(
@@ -429,6 +466,158 @@ t.test('owner rm <user> no cwd package', async t => {
   )
 })
 
+t.test('workspaces', async t => {
+  t.test('owner no args --workspace', async t => {
+    const { npm } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+    })
+    npm.config.set('workspace', ['workspace-a'])
+    await t.rejects(
+      npm.exec('owner', []),
+      { code: 'EUSAGE' },
+      'rejects with usage'
+    )
+  })
+
+  t.test('owner ls implicit workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+      globals: ({ prefix }) => ({
+        'process.cwd': () => path.join(prefix, 'workspace-a'),
+      }),
+    })
+    registryPackage(t, npm.config.get('registry'), 'workspace-a')
+    await npm.exec('owner', ['ls'])
+    t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
+  })
+
+  t.test('owner ls explicit workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+      globals: ({ prefix }) => ({
+        'process.cwd': () => prefix,
+      }),
+    })
+    npm.config.set('workspace', ['workspace-a'])
+    registryPackage(t, npm.config.get('registry'), 'workspace-a')
+    await npm.exec('owner', ['ls'])
+    t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
+  })
+
+  t.test('owner ls <pkg> implicit workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+      globals: ({ prefix }) => ({
+        'process.cwd': () => path.join(prefix, 'workspace-a'),
+      }),
+    })
+    registryPackage(t, npm.config.get('registry'), packageName)
+    await npm.exec('owner', ['ls', packageName])
+    t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
+  })
+
+  t.test('owner ls <pkg> explicit workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+      globals: ({ prefix }) => ({
+        'process.cwd': () => prefix,
+      }),
+    })
+    npm.config.set('workspace', ['workspace-a'])
+    registryPackage(t, npm.config.get('registry'), packageName)
+    await npm.exec('owner', ['ls', packageName])
+    t.match(joinedOutput(), maintainers.map(m => `${m.name} <${m.email}>`).join('\n'))
+  })
+
+  t.test('owner add implicit workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+      globals: ({ prefix }) => ({
+        'process.cwd': () => path.join(prefix, 'workspace-a'),
+      }),
+    })
+    const username = 'foo'
+    const registry = new MockRegistry({ tap: t, registry: npm.config.get('registry') })
+
+    const manifest = registry.manifest({
+      name: 'workspace-a',
+      packuments: [{ maintainers, version: '1.0.0' }],
+    })
+    registry.package({ manifest })
+    registry.couchuser({ username })
+    registry.nock.put(`/workspace-a/-rev/${manifest._rev}`, body => {
+      t.match(body, {
+        _id: manifest._id,
+        _rev: manifest._rev,
+        maintainers: [
+          ...manifest.maintainers,
+          { name: username, email: '' },
+        ],
+      })
+      return true
+    }).reply(200, {})
+    await npm.exec('owner', ['add', username])
+    t.equal(joinedOutput(), `+ ${username} (workspace-a)`)
+  })
+
+  t.test('owner add --workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+    })
+    npm.config.set('workspace', ['workspace-a'])
+    const username = 'foo'
+    const registry = new MockRegistry({ tap: t, registry: npm.config.get('registry') })
+
+    const manifest = registry.manifest({
+      name: 'workspace-a',
+      packuments: [{ maintainers, version: '1.0.0' }],
+    })
+    registry.package({ manifest })
+    registry.couchuser({ username })
+    registry.nock.put(`/workspace-a/-rev/${manifest._rev}`, body => {
+      t.match(body, {
+        _id: manifest._id,
+        _rev: manifest._rev,
+        maintainers: [
+          ...manifest.maintainers,
+          { name: username, email: '' },
+        ],
+      })
+      return true
+    }).reply(200, {})
+    await npm.exec('owner', ['add', username])
+    t.equal(joinedOutput(), `+ ${username} (workspace-a)`)
+  })
+
+  t.test('owner rm --workspace', async t => {
+    const { npm, joinedOutput } = await loadMockNpm(t, {
+      prefixDir: workspaceFixture,
+      globals: ({ prefix }) => ({
+        'process.cwd': () => path.join(prefix, 'workspace-a'),
+      }),
+    })
+    const registry = new MockRegistry({ tap: t, registry: npm.config.get('registry') })
+
+    const username = maintainers[0].name
+    const manifest = registry.manifest({
+      name: 'workspace-a',
+      packuments: [{ maintainers, version: '1.0.0' }],
+    })
+    registry.package({ manifest })
+    registry.couchuser({ username })
+    registry.nock.put(`/workspace-a/-rev/${manifest._rev}`, body => {
+      t.match(body, {
+        _id: manifest._id,
+        _rev: manifest._rev,
+        maintainers: maintainers.slice(1),
+      })
+      return true
+    }).reply(200, {})
+    await npm.exec('owner', ['rm', username])
+    t.equal(joinedOutput(), `- ${username} (workspace-a)`)
+  })
+})
+
 t.test('completion', async t => {
   t.test('basic commands', async t => {
     const { npm } = await loadMockNpm(t)
diff --git a/test/lib/npm.js b/test/lib/npm.js
@@ -620,7 +620,7 @@ t.test('implicit workspace rejection', async t => {
     }),
   })
   await t.rejects(
-    mock.npm.exec('owner', []),
+    mock.npm.exec('team', []),
     /This command does not support workspaces/
   )
 })

Original file line number	Diff line number	Diff line change
`@@ -620,7 +620,7 @@ t.test('implicit workspace rejection', async t => {`
`620`	`620`	`}),`
`621`	`621`	`})`
`622`	`622`	`await t.rejects(`
`623`		`- mock.npm.exec('owner', []),`
	`623`	`+ mock.npm.exec('team', []),`
`624`	`624`	`/This command does not support workspaces/`
`625`	`625`	`)`
`626`	`626`	`})`