From 789a01c97eb745616eea664f7b07914fbaef90a7 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Wed, 17 Mar 2021 00:22:28 -0400 Subject: [PATCH] feat: add exec workspaces Add workspaces support to `npm exec` - Refactored logic to read and filter workspaces into `lib/workspaces/get-workspaces.js` - Add ability to execute a package in the context of each configured workspace Fixes: https://github.com/npm/statusboard/issues/288 --- lib/exec.js | 46 ++++-- lib/run-script.js | 29 +--- lib/workspaces/get-workspaces.js | 33 +++++ test/lib/exec.js | 69 +++++++-- test/lib/workspaces/get-workspaces.js | 199 ++++++++++++++++++++++++++ 5 files changed, 330 insertions(+), 46 deletions(-) create mode 100644 lib/workspaces/get-workspaces.js create mode 100644 test/lib/workspaces/get-workspaces.js diff --git a/lib/exec.js b/lib/exec.js index b2443b17accd2..f2c2fefab46d4 100644 --- a/lib/exec.js +++ b/lib/exec.js @@ -12,6 +12,7 @@ const npa = require('npm-package-arg') const fileExists = require('./utils/file-exists.js') const PATH = require('./utils/path.js') const BaseCommand = require('./base-command.js') +const getWorkspaces = require('./workspaces/get-workspaces.js') // it's like this: // @@ -60,17 +61,25 @@ class Exec extends BaseCommand { } exec (args, cb) { - this._exec(args).then(() => cb()).catch(cb) + const path = this.npm.localPrefix + const runPath = process.cwd() + this._exec(args, { path, runPath }).then(() => cb()).catch(cb) + } + + execWorkspaces (args, filters, cb) { + this._execWorkspaces(args, filters).then(() => cb()).catch(cb) } // When commands go async and we can dump the boilerplate exec methods this // can be named correctly - async _exec (args) { - const { package: packages, call, shell } = this.npm.flatOptions + async _exec (_args, { path, runPath }) { + const { package: p, call, shell } = this.npm.flatOptions + const packages = [...p] - if (call && args.length) + if (call && _args.length) throw this.usage + const args = [..._args] const pathArr = [...PATH] // nothing to maybe install, skip the arborist dance @@ -79,7 +88,9 @@ class Exec extends BaseCommand { args, call, shell, + path, pathArr, + runPath, }) } @@ -102,7 +113,9 @@ class Exec extends BaseCommand { return await this.run({ args, call, + path, pathArr, + runPath, shell, }) } @@ -117,11 +130,11 @@ class Exec extends BaseCommand { // node_modules/${name}/package.json, and only pacote fetch if // that fails. const manis = await Promise.all(packages.map(async p => { - const spec = npa(p, this.npm.localPrefix) + const spec = npa(p, path) if (spec.type === 'tag' && spec.rawSpec === '') { // fall through to the pacote.manifest() approach try { - const pj = resolve(this.npm.localPrefix, 'node_modules', spec.name) + const pj = resolve(path, 'node_modules', spec.name) return await readPackageJson(pj) } catch (er) {} } @@ -140,7 +153,7 @@ class Exec extends BaseCommand { // figure out whether we need to install stuff, or if local is fine const localArb = new Arborist({ ...this.npm.flatOptions, - path: this.npm.localPrefix, + path, }) const tree = await localArb.loadActual() @@ -192,16 +205,16 @@ class Exec extends BaseCommand { pathArr.unshift(resolve(installDir, 'node_modules/.bin')) } - return await this.run({ args, call, pathArr, shell }) + return await this.run({ args, call, path, pathArr, runPath, shell }) } - async run ({ args, call, pathArr, shell }) { + async run ({ args, call, path, pathArr, runPath, shell }) { // turn list of args into command string const script = call || args.shift() || shell // do the fakey runScript dance // still should work if no package.json in cwd - const realPkg = await readPackageJson(`${this.npm.localPrefix}/package.json`) + const realPkg = await readPackageJson(`${path}/package.json`) .catch(() => ({})) const pkg = { ...realPkg, @@ -225,7 +238,7 @@ class Exec extends BaseCommand { pkg, banner: false, // we always run in cwd, not --prefix - path: process.cwd(), + path: runPath, stdioString: true, event: 'npx', args, @@ -285,5 +298,16 @@ class Exec extends BaseCommand { .digest('hex') .slice(0, 16) } + + async workspaces (filters) { + return getWorkspaces(filters, { path: this.npm.localPrefix }) + } + + async _execWorkspaces (args, filters) { + const workspaces = await this.workspaces(filters) + + for (const workspacePath of workspaces.values()) + await this._exec(args, { path: workspacePath, runPath: workspacePath }) + } } module.exports = Exec diff --git a/lib/run-script.js b/lib/run-script.js index e361cf7c6263b..5b7fe1cb7500f 100644 --- a/lib/run-script.js +++ b/lib/run-script.js @@ -1,13 +1,12 @@ const { resolve } = require('path') const chalk = require('chalk') const runScript = require('@npmcli/run-script') -const mapWorkspaces = require('@npmcli/map-workspaces') const { isServerPackage } = runScript const rpj = require('read-package-json-fast') const log = require('npmlog') -const minimatch = require('minimatch') const didYouMean = require('./utils/did-you-mean.js') const isWindowsShell = require('./utils/is-windows-shell.js') +const getWorkspaces = require('./workspaces/get-workspaces.js') const cmdList = [ 'publish', @@ -178,31 +177,7 @@ class RunScript extends BaseCommand { } async workspaces (filters) { - const cwd = this.npm.localPrefix - const pkg = await rpj(resolve(cwd, 'package.json')) - const workspaces = await mapWorkspaces({ cwd, pkg }) - const res = filters.length ? new Map() : workspaces - - for (const filterArg of filters) { - for (const [key, path] of workspaces.entries()) { - if (filterArg === key - || resolve(cwd, filterArg) === path - || minimatch(path, `${resolve(cwd, filterArg)}/*`)) - res.set(key, path) - } - } - - if (!res.size) { - let msg = '!' - if (filters.length) { - msg = `:\n ${filters.reduce( - (res, filterArg) => `${res} --workspace=${filterArg}`, '')}` - } - - throw new Error(`No workspaces found${msg}`) - } - - return res + return getWorkspaces(filters, { path: this.npm.localPrefix }) } async runWorkspaces (args, filters) { diff --git a/lib/workspaces/get-workspaces.js b/lib/workspaces/get-workspaces.js new file mode 100644 index 0000000000000..64812d5403576 --- /dev/null +++ b/lib/workspaces/get-workspaces.js @@ -0,0 +1,33 @@ +const { resolve } = require('path') +const mapWorkspaces = require('@npmcli/map-workspaces') +const minimatch = require('minimatch') +const rpj = require('read-package-json-fast') + +const getWorkspaces = async (filters, { path }) => { + const pkg = await rpj(resolve(path, 'package.json')) + const workspaces = await mapWorkspaces({ cwd: path, pkg }) + const res = filters.length ? new Map() : workspaces + + for (const filterArg of filters) { + for (const [workspaceName, workspacePath] of workspaces.entries()) { + if (filterArg === workspaceName + || resolve(path, filterArg) === workspacePath + || minimatch(workspacePath, `${resolve(path, filterArg)}/*`)) + res.set(workspaceName, workspacePath) + } + } + + if (!res.size) { + let msg = '!' + if (filters.length) { + msg = `:\n ${filters.reduce( + (res, filterArg) => `${res} --workspace=${filterArg}`, '')}` + } + + throw new Error(`No workspaces found${msg}`) + } + + return res +} + +module.exports = getWorkspaces diff --git a/test/lib/exec.js b/test/lib/exec.js index eb9fef6a61da2..0a45f64e4e803 100644 --- a/test/lib/exec.js +++ b/test/lib/exec.js @@ -186,7 +186,7 @@ t.test('npm exec foo, already present locally', t => { if (er) throw er t.strictSame(MKDIRPS, [], 'no need to make any dirs') - t.match(ARB_CTOR, [{ package: ['foo'], path }]) + t.match(ARB_CTOR, [{ path }]) t.strictSame(ARB_REIFY, [], 'no need to reify anything') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') t.match(RUN_SCRIPTS, [{ @@ -300,7 +300,7 @@ t.test('npm exec foo, not present locally or in central loc', t => { if (er) throw er t.strictSame(MKDIRPS, [installDir], 'need to make install dir') - t.match(ARB_CTOR, [{ package: ['foo'], path }]) + t.match(ARB_CTOR, [{ path }]) t.match(ARB_REIFY, [{add: ['foo@'], legacyPeerDeps: false}], 'need to install foo@') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') const PATH = `${resolve(installDir, 'node_modules', '.bin')}${delimiter}${process.env.PATH}` @@ -340,7 +340,7 @@ t.test('npm exec foo, not present locally but in central loc', t => { if (er) throw er t.strictSame(MKDIRPS, [installDir], 'need to make install dir') - t.match(ARB_CTOR, [{ package: ['foo'], path }]) + t.match(ARB_CTOR, [{ path }]) t.match(ARB_REIFY, [], 'no need to install again, already there') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') const PATH = `${resolve(installDir, 'node_modules', '.bin')}${delimiter}${process.env.PATH}` @@ -380,7 +380,7 @@ t.test('npm exec foo, present locally but wrong version', t => { if (er) throw er t.strictSame(MKDIRPS, [installDir], 'need to make install dir') - t.match(ARB_CTOR, [{ package: ['foo'], path }]) + t.match(ARB_CTOR, [{ path }]) t.match(ARB_REIFY, [{ add: ['foo@2.x'], legacyPeerDeps: false }], 'need to add foo@2.x') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') const PATH = `${resolve(installDir, 'node_modules', '.bin')}${delimiter}${process.env.PATH}` @@ -417,7 +417,7 @@ t.test('npm exec --package=foo bar', t => { if (er) throw er t.strictSame(MKDIRPS, [], 'no need to make any dirs') - t.match(ARB_CTOR, [{ package: ['foo'], path }]) + t.match(ARB_CTOR, [{ path }]) t.strictSame(ARB_REIFY, [], 'no need to reify anything') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') t.match(RUN_SCRIPTS, [{ @@ -459,7 +459,7 @@ t.test('npm exec @foo/bar -- --some=arg, locally installed', t => { if (er) throw er t.strictSame(MKDIRPS, [], 'no need to make any dirs') - t.match(ARB_CTOR, [{ package: ['@foo/bar'], path }]) + t.match(ARB_CTOR, [{ path }]) t.strictSame(ARB_REIFY, [], 'no need to reify anything') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') t.match(RUN_SCRIPTS, [{ @@ -502,7 +502,7 @@ t.test('npm exec @foo/bar, with same bin alias and no unscoped named bin, locall if (er) throw er t.strictSame(MKDIRPS, [], 'no need to make any dirs') - t.match(ARB_CTOR, [{ package: ['@foo/bar'], path }]) + t.match(ARB_CTOR, [{ path }]) t.strictSame(ARB_REIFY, [], 'no need to reify anything') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') t.match(RUN_SCRIPTS, [{ @@ -666,7 +666,7 @@ t.test('npm exec -p foo -c "ls -laF"', t => { if (er) throw er t.strictSame(MKDIRPS, [], 'no need to make any dirs') - t.match(ARB_CTOR, [{ package: ['foo'], path }]) + t.match(ARB_CTOR, [{ path }]) t.strictSame(ARB_REIFY, [], 'no need to reify anything') t.equal(PROGRESS_ENABLED, true, 'progress re-enabled') t.match(RUN_SCRIPTS, [{ @@ -1082,3 +1082,56 @@ t.test('forward legacyPeerDeps opt', t => { t.done() }) }) + +t.test('workspaces', t => { + npm.localPrefix = t.testdir({ + node_modules: { + '.bin': { + foo: '', + }, + }, + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + bin: 'cli.js', + }), + 'cli.js': '', + }, + b: { + 'package.json': JSON.stringify({ + name: 'b', + version: '1.0.0', + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['packages/*'], + }), + }) + + PROGRESS_IGNORED = true + npm.localBin = resolve(npm.localPrefix, 'node_modules/.bin') + + exec.execWorkspaces(['foo', 'one arg', 'two arg'], ['a', 'b'], er => { + if (er) + throw er + + t.match(RUN_SCRIPTS, [{ + pkg: { scripts: { npx: 'foo' }}, + args: ['one arg', 'two arg'], + banner: false, + path: process.cwd(), + stdioString: true, + event: 'npx', + env: { + PATH: [npm.localBin, ...PATH].join(delimiter), + }, + stdio: 'inherit', + }]) + t.end() + }) +}) diff --git a/test/lib/workspaces/get-workspaces.js b/test/lib/workspaces/get-workspaces.js new file mode 100644 index 0000000000000..ebed9dd35c519 --- /dev/null +++ b/test/lib/workspaces/get-workspaces.js @@ -0,0 +1,199 @@ +const { resolve } = require('path') +const t = require('tap') +const getWorkspaces = require('../../../lib/workspaces/get-workspaces.js') + +const normalizePath = p => p + .replace(/\\+/g, '/') + .replace(/\r\n/g, '\n') + +const cleanOutput = (str, path) => normalizePath(str) + .replace(normalizePath(path), '{PATH}') + +const clean = (res, path) => { + const cleaned = new Map() + for (const [key, value] of res.entries()) + cleaned.set(key, cleanOutput(value, path)) + return cleaned +} + +t.test('get-workspaces', async t => { + const path = t.testdir({ + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + scripts: { glorp: 'echo a doing the glerp glop' }, + }), + }, + b: { + 'package.json': JSON.stringify({ + name: 'b', + version: '2.0.0', + scripts: { glorp: 'echo b doing the glerp glop' }, + }), + }, + c: { + 'package.json': JSON.stringify({ + name: 'c', + version: '1.0.0', + scripts: { + test: 'exit 0', + posttest: 'echo posttest', + lorem: 'echo c lorem', + }, + }), + }, + d: { + 'package.json': JSON.stringify({ + name: 'd', + version: '1.0.0', + scripts: { + test: 'exit 0', + posttest: 'echo posttest', + }, + }), + }, + e: { + 'package.json': JSON.stringify({ + name: 'e', + scripts: { test: 'exit 0', start: 'echo start something' }, + }), + }, + noscripts: { + 'package.json': JSON.stringify({ + name: 'noscripts', + version: '1.0.0', + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'x', + version: '1.2.3', + workspaces: ['packages/*'], + }), + }) + + let workspaces + + workspaces = await getWorkspaces(['a', 'b'], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + a: '{PATH}/packages/a', + b: '{PATH}/packages/b', + })), + 'should filter by package name' + ) + + workspaces = await getWorkspaces(['./packages/c'], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + c: '{PATH}/packages/c', + })), + 'should filter by package directory' + ) + + workspaces = await getWorkspaces(['packages/c'], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + c: '{PATH}/packages/c', + })), + 'should filter by rel package directory' + ) + + workspaces = await getWorkspaces([resolve(path, 'packages/c')], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + c: '{PATH}/packages/c', + })), + 'should filter by absolute package directory' + ) + + workspaces = await getWorkspaces(['packages'], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + a: '{PATH}/packages/a', + b: '{PATH}/packages/b', + c: '{PATH}/packages/c', + d: '{PATH}/packages/d', + e: '{PATH}/packages/e', + noscripts: '{PATH}/packages/noscripts', + })), + 'should filter by parent directory name' + ) + + workspaces = await getWorkspaces(['./packages/'], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + a: '{PATH}/packages/a', + b: '{PATH}/packages/b', + c: '{PATH}/packages/c', + d: '{PATH}/packages/d', + e: '{PATH}/packages/e', + noscripts: '{PATH}/packages/noscripts', + })), + 'should filter by parent directory path' + ) + + workspaces = await getWorkspaces([resolve(path, './packages')], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + a: '{PATH}/packages/a', + b: '{PATH}/packages/b', + c: '{PATH}/packages/c', + d: '{PATH}/packages/d', + e: '{PATH}/packages/e', + noscripts: '{PATH}/packages/noscripts', + })), + 'should filter by absolute parent directory path' + ) + + workspaces = await getWorkspaces([], { path }) + t.deepEqual( + clean(workspaces, path), + new Map(Object.entries({ + a: '{PATH}/packages/a', + b: '{PATH}/packages/b', + c: '{PATH}/packages/c', + d: '{PATH}/packages/d', + e: '{PATH}/packages/e', + noscripts: '{PATH}/packages/noscripts', + })), + 'should return all workspaces if no filter set' + ) + + try { + await getWorkspaces(['missing'], { path }) + throw new Error('missed throw') + } catch (err) { + t.match( + err, + /No workspaces found/, + 'should throw no workspaces found error' + ) + } + + const unconfiguredWorkspaces = t.testdir({ + 'package.json': JSON.stringify({ + name: 'no-configured-workspaces', + version: '1.0.0', + }), + }) + try { + await getWorkspaces([], { path: unconfiguredWorkspaces }) + throw new Error('missed throw') + } catch (err) { + t.match( + err, + /No workspaces found/, + 'should throw no workspaces found error' + ) + } +})