fix: ignore implict workspace for some commands

closes #4404
npm · Feb 28, 2022 · ad82c09 · ad82c09
1 parent 2db3eff
commit ad82c09
Show file tree

Hide file tree

Showing 2 changed files with 133 additions and 2 deletions.
diff --git 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
@@ -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/
+  )
+})