From 849b520c8f05a87856867684e7562bb2fd1e7c21 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 27 Apr 2022 12:48:16 -0700 Subject: [PATCH] feat: make npm owner workspace aware --- docs/content/commands/npm-owner.md | 46 +++++ lib/commands/owner.js | 51 +++-- .../test/lib/load-all-commands.js.test.cjs | 2 + .../test/lib/utils/npm-usage.js.test.cjs | 2 + test/lib/commands/owner.js | 189 ++++++++++++++++++ test/lib/npm.js | 2 +- 6 files changed, 277 insertions(+), 15 deletions(-) diff --git a/docs/content/commands/npm-owner.md b/docs/content/commands/npm-owner.md index 0779984e19a9d..72dfe6a22d01b 100644 --- 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. +#### `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. + + + + +#### `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. + + + + ### See Also diff --git a/lib/commands/owner.js b/lib/commands/owner.js index 285b06be8e5fe..9338b22a5e857 100644 --- 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 index 37349cbe01e7d..eccb06580d9f9 100644 --- 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>/] Options: [--registry ] [--otp ] +[-w|--workspace [-w|--workspace ...]] +[-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 index eb71ced8d73b5..d7ec4953d6ab3 100644 --- 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 ] [--otp ] + [-w|--workspace [-w|--workspace ...]] + [-ws|--workspaces] alias: author diff --git a/test/lib/commands/owner.js b/test/lib/commands/owner.js index d80ce36fece98..800d5b96a5876 100644 --- 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 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 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 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 index 1966ca9600088..566cbca20f6bf 100644 --- 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/ ) })