From ad82c09ff79ee183846b85fd7a31cf35264c1b12 Mon Sep 17 00:00:00 2001 From: Nathan Fritz Date: Sat, 26 Feb 2022 17:35:46 -0800 Subject: [PATCH] fix: ignore implict workspace for some commands closes #4404 --- lib/npm.js | 30 +++++++++++++- test/lib/npm.js | 105 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/lib/npm.js b/lib/npm.js index ce8f6aee8f575..16c19f4cc1b7a 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -24,6 +24,28 @@ const _tmpFolder = Symbol('_tmpFolder') const _title = Symbol('_title') const pkg = require('../package.json') +const ignoreWorkspacesCommands = new Set([ + 'adduser', + 'access', + 'login', + 'bin', + 'birthday', + 'bugs', + 'cache', + 'help-search', + 'help', + 'hook', + 'logout', + 'org', + // 'owner', // TODO, this may need some workspace support + 'ping', + 'prefix', + 'profile', + // 'restart', // TODO add workspace support + 'root', + 'search', +]) + class Npm extends EventEmitter { static get version () { return pkg.version @@ -113,12 +135,18 @@ class Npm extends EventEmitter { } const workspacesEnabled = this.config.get('workspaces') + /* istanbul ignore next */ + const implicitWorkspace = (this.config.get('workspace', 'default') || []).length > 0 const workspacesFilters = this.config.get('workspace') if (workspacesEnabled === false && workspacesFilters.length > 0) { throw new Error('Can not use --no-workspaces and --workspace at the same time') } - const filterByWorkspaces = workspacesEnabled || workspacesFilters.length > 0 + // only call execWorkspaces when we have workspaces explicitly set + // or when it is implicit and not in our ignore list + const filterByWorkspaces = + (workspacesEnabled || workspacesFilters.length > 0) + && (!implicitWorkspace || !ignoreWorkspacesCommands.has(cmd)) // normally this would go in the constructor, but our tests don't // actually use a real npm object so this.npm.config isn't always // populated. this is the compromise until we can make that a reality diff --git a/test/lib/npm.js b/test/lib/npm.js index 2a0c5a89d2d99..76b4aad1c6132 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -1,5 +1,5 @@ const t = require('tap') -const { resolve, dirname } = require('path') +const { resolve, dirname, join } = require('path') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') const mockGlobals = require('../fixtures/mock-globals') @@ -520,3 +520,106 @@ t.test('unknown command', async t => { { code: 'EUNKNOWNCOMMAND' } ) }) + +t.test('explicit workspace rejection', async t => { + mockGlobals(t, { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + '--workspace', './packages/a', + ], + }) + const mock = await loadMockNpm(t, { + testdir: { + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + scripts: { test: 'echo test a' }, + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['./packages/a'], + }), + }, + }) + await t.rejects( + mock.npm.exec('ping', []), + /This command does not support workspaces/ + ) +}) + +t.test('implicit workspace rejection', async t => { + const mock = await loadMockNpm(t, { + testdir: { + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + scripts: { test: 'echo test a' }, + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['./packages/a'], + }), + }, + }) + const cwd = join(mock.npm.config.localPrefix, 'packages', 'a') + mock.npm.config.set('workspace', [cwd], 'default') + console.log(mock.npm.config.get('workspace', 'default')) + mockGlobals(t, { + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + ], + }) + await t.rejects( + mock.npm.exec('owner', []), + /This command does not support workspaces/ + ) +}) + +t.test('implicit workspace accept', async t => { + const mock = await loadMockNpm(t, { + testdir: { + packages: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + scripts: { test: 'echo test a' }, + }), + }, + }, + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + workspaces: ['./packages/a'], + }), + }, + }) + const cwd = join(mock.npm.config.localPrefix, 'packages', 'a') + mock.npm.config.set('workspace', [cwd], 'default') + mockGlobals(t, { + 'process.cwd': () => mock.npm.config.cwd, + 'process.argv': [ + process.execPath, + process.argv[1], + '--color', 'false', + ], + }) + await t.rejects( + mock.npm.exec('org', []), + /.*Usage/ + ) +})