feat(workspaces): add --include-workspace-root and explicit --no-workspacesfritzy/workspace-root

Adds a new config item that includes the workspace root. This also changes
--workspaces to a trinary, so that setting it to false will explicitly exclude
workspaces altogether.

PR-URL: https://github.com/npm/cli/pull/3890
Credit: @fritzy
Close: #3890
Reviewed-by: @wraithgar

10 files changed, 270 insertions, 58 deletions

diff --git a/test/lib/explain.js b/test/lib/explain.js
index f690aeb2c..ebec13619 100644
--- a/test/lib/explain.js
+++ b/test/lib/explain.js
@@ -2,7 +2,7 @@ const t = require('tap')
 const npm = {
   prefix: null,
   color: true,
-  flatOptions: {},
+  flatOptions: { workspacesEnabled: true },
   output: (...args) => {
     OUTPUT.push(args)
   },
@@ -301,3 +301,80 @@ t.test('workspaces', async t => {
     })
   })
 })
+
+t.test('workspaces disabled', async t => {
+  npm.localPrefix = npm.prefix = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'workspaces-project',
+      version: '1.0.0',
+      workspaces: ['packages/*'],
+      dependencies: {
+        abbrev: '^1.0.0',
+      },
+    }),
+    node_modules: {
+      a: t.fixture('symlink', '../packages/a'),
+      b: t.fixture('symlink', '../packages/b'),
+      c: t.fixture('symlink', '../packages/c'),
+      once: {
+        'package.json': JSON.stringify({
+          name: 'once',
+          version: '1.0.0',
+          dependencies: {
+            wrappy: '2.0.0',
+          },
+        }),
+      },
+      abbrev: {
+        'package.json': JSON.stringify({
+          name: 'abbrev',
+          version: '1.0.0',
+        }),
+      },
+      wrappy: {
+        'package.json': JSON.stringify({
+          name: 'wrappy',
+          version: '2.0.0',
+        }),
+      },
+    },
+    packages: {
+      a: {
+        'package.json': JSON.stringify({
+          name: 'a',
+          version: '1.0.0',
+          dependencies: {
+            once: '1.0.0',
+          },
+        }),
+      },
+      b: {
+        'package.json': JSON.stringify({
+          name: 'b',
+          version: '1.0.0',
+          dependencies: {
+            abbrev: '^1.0.0',
+          },
+        }),
+      },
+      c: {
+        'package.json': JSON.stringify({
+          name: 'c',
+          version: '1.0.0',
+        }),
+      },
+    },
+  })
+
+  await new Promise((res, rej) => {
+    explain.npm.flatOptions.workspacesEnabled = false
+    explain.exec(['once'], err => {
+      t.equal(
+        err,
+        'No dependencies found matching once',
+        'should throw usage if dep not found when excluding ws'
+      )
+      res()
+    })
+  })
+})
diff --git a/test/lib/init.js b/test/lib/init.js
index 1cefb1fc9..f11ce356f 100644
--- a/test/lib/init.js
+++ b/test/lib/init.js
@@ -513,3 +513,26 @@ t.test('workspaces', t => {
 
   t.end()
 })
+t.test('npm init workspces with root', t => {
+  t.teardown(() => {
+    npm._mockOutputs.length = 0
+  })
+  npm.localPrefix = t.testdir({})
+  npm.flatOptions.includeWorkspaceRoot = true
+
+  // init-package-json prints directly to console.log
+  // this avoids poluting test output with those logs
+  console.log = noop
+
+  process.chdir(npm.localPrefix)
+  init.execWorkspaces([], ['packages/a'], err => {
+    if (err)
+      throw err
+
+    const pkg = require(resolve(npm.localPrefix, 'package.json'))
+    t.equal(pkg.version, '1.0.0')
+    t.equal(pkg.license, 'ISC')
+    t.matchSnapshot(npm._mockOutputs, 'does not print helper info')
+    t.end()
+  })
+})
diff --git a/test/lib/ls.js b/test/lib/ls.js
index 5f196501e..46dfd7fba 100644
--- a/test/lib/ls.js
+++ b/test/lib/ls.js
@@ -110,6 +110,7 @@ const config = {
   'package-lock-only': false,
 }
 const flatOptions = {
+  workspacesEnabled: true,
 }
 const npm = mockNpm({
   config,
@@ -1530,6 +1531,25 @@ t.test('ls', (t) => {
       })
     })
 
+    await new Promise((res, rej) => {
+      config.all = false
+      config.depth = 0
+      npm.color = true
+      npm.flatOptions.workspacesEnabled = false
+      ls.exec([], (err) => {
+        if (err)
+          rej(err)
+
+        t.matchSnapshot(redactCwd(result),
+          'should not list workspaces with --no-workspaces')
+        config.all = true
+        config.depth = Infinity
+        npm.color = false
+        npm.flatOptions.workspacesEnabled = true
+        res()
+      })
+    })
+
     // --all
     await new Promise((res, rej) => {
       ls.exec([], (err) => {
diff --git a/test/lib/npm.js b/test/lib/npm.js
index 7d6176247..1451cd879 100644
--- a/test/lib/npm.js
+++ b/test/lib/npm.js
@@ -292,6 +292,44 @@ t.test('npm.load', t => {
     await new Promise((res) => setTimeout(res))
   })
 
+  t.test('--no-workspaces with --workspace', async t => {
+    const dir = 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/*'],
+      }),
+    })
+    process.argv = [
+      process.execPath,
+      process.argv[1],
+      '--userconfig', resolve(dir, '.npmrc'),
+      '--color', 'false',
+      '--workspaces', 'false',
+      '--workspace', 'a',
+    ]
+    const { npm } = mockNpm(t)
+    await npm.load()
+    npm.localPrefix = dir
+    await new Promise((res, rej) => {
+      npm.commands.run([], er => {
+        if (!er)
+          return rej(new Error('Expected an error'))
+        t.match(er.message, 'Can not use --no-workspaces and --workspace at the same time')
+        res()
+      })
+    })
+  })
+
   t.test('workspace-aware configs and commands', async t => {
     const dir = t.testdir({
       packages: {
diff --git a/test/lib/outdated.js b/test/lib/outdated.js
index 518436d0a..acd5c25e8 100644
--- a/test/lib/outdated.js
+++ b/test/lib/outdated.js
@@ -83,6 +83,10 @@ const globalDir = t.testdir({
   },
 })
 
+const flatOptions = {
+  workspacesEnabled: true,
+}
+
 const outdated = (dir, opts) => {
   logs = ''
   const Outdated = t.mock('../../lib/outdated.js', {
@@ -94,6 +98,7 @@ const outdated = (dir, opts) => {
     ...opts,
     localPrefix: dir,
     prefix: dir,
+    flatOptions,
     globalDir: `${globalDir}/node_modules`,
     output,
   })
@@ -562,6 +567,21 @@ t.test('workspaces', async t => {
   })
 
   await new Promise((res, rej) => {
+    flatOptions.workspacesEnabled = false
+    outdated(testDir, {}).exec([], err => {
+      if (err)
+        rej(err)
+
+      // TODO: This should display dog, but doesn't because arborist filters
+      // workspace deps even if they're also root deps
+      // This will be fixed in a future arborist version
+      t.matchSnapshot(logs, 'should display only root outdated when ws disabled')
+      flatOptions.workspacesEnabled = true
+      res()
+    })
+  })
+
+  await new Promise((res, rej) => {
     outdated(testDir, {
       config: {
         json: true,
diff --git a/test/lib/publish.js b/test/lib/publish.js
index df73b6863..6b0021db6 100644
--- a/test/lib/publish.js
+++ b/test/lib/publish.js
@@ -57,6 +57,7 @@ t.test('should publish with libnpmpublish, passing through flatOptions and respe
   const npm = mockNpm({
     flatOptions: {
       customValue: true,
+      workspacesEnabled: true,
     },
   })
   npm.config.getCredentialsByURI = (uri) => {
diff --git a/test/lib/repo.js b/test/lib/repo.js
index e1ac90b1e..41bff2744 100644
--- a/test/lib/repo.js
+++ b/test/lib/repo.js
@@ -1,5 +1,5 @@
 const t = require('tap')
-const { fake: mockNpm } = require('../fixtures/mock-npm.js')
+const { real: mockNpm } = require('../fixtures/mock-npm.js')
 const { join, sep } = require('path')
 
 const pkgDirs = t.testdir({
@@ -154,6 +154,7 @@ const pkgDirs = t.testdir({
       name: 'workspaces-test',
       version: '1.2.3-test',
       workspaces: ['workspace-a', 'workspace-b', 'workspace-c'],
+      repository: 'https://github.com/npm/workspaces-test',
     }),
     'workspace-a': {
       'package.json': JSON.stringify({
@@ -185,19 +186,17 @@ const openUrl = async (npm, url, errMsg) => {
   opened[url]++
 }
 
-const Repo = t.mock('../../lib/repo.js', {
+const { command, npm } = mockNpm(t, {
   '../../lib/utils/open-url.js': openUrl,
 })
-const flatOptions = {}
-const npm = mockNpm({ flatOptions })
-const repo = new Repo(npm)
+t.before(async () => {
+  await npm.load()
+})
 
 t.afterEach(() => opened = {})
 
 t.test('open repo urls', t => {
-  // XXX It is very odd that `where` is how pacote knows to look anywhere other
-  // than the cwd. I would think npm.localPrefix would factor in somehow
-  flatOptions.where = pkgDirs
+  npm.localPrefix = pkgDirs
   const expect = {
     hostedgit: 'https://github.com/foo/hostedgit',
     hostedgitat: 'https://github.com/foo/hostedgitat',
@@ -227,22 +226,19 @@ t.test('open repo urls', t => {
   const keys = Object.keys(expect)
   t.plan(keys.length)
   keys.forEach(pkg => {
-    t.test(pkg, t => {
-      repo.exec([['.', pkg].join(sep)], (err) => {
-        if (err)
-          throw err
-        const url = expect[pkg]
-        t.match({
-          [url]: 1,
-        }, opened, `opened ${url}`, {opened})
-        t.end()
-      })
+    t.test(pkg, async t => {
+      await command('repo', [['.', pkg].join(sep)])
+      const url = expect[pkg]
+      t.match({
+        [url]: 1,
+      }, opened, `opened ${url}`, {opened})
+      t.end()
     })
   })
 })
 
 t.test('fail if cannot figure out repo url', t => {
-  flatOptions.where = pkgDirs
+  npm.localPrefix = pkgDirs
   const cases = [
     'norepo',
     'repoobbj-nourl',
@@ -253,57 +249,65 @@ t.test('fail if cannot figure out repo url', t => {
   t.plan(cases.length)
 
   cases.forEach(pkg => {
-    t.test(pkg, t => {
-      repo.exec([['.', pkg].join(sep)], (err) => {
-        t.match(err, { pkgid: pkg })
-        t.end()
-      })
+    t.test(pkg, async t => {
+      t.rejects(
+        command('repo', [['.', pkg].join(sep)]),
+        { pkgid: pkg }
+      )
     })
   })
 })
 
-t.test('open default package if none specified', t => {
-  flatOptions.where = pkgDirs
-  repo.exec([], (er) => {
-    if (er)
-      throw er
-    t.equal(opened['https://example.com/thispkg'], 1, 'opened expected url', {opened})
-    t.end()
-  })
+t.test('open default package if none specified', async t => {
+  npm.localPrefix = pkgDirs
+  await command('repo', [])
+  t.equal(opened['https://example.com/thispkg'], 1, 'opened expected url', {opened})
 })
 
 t.test('workspaces', t => {
-  flatOptions.where = undefined
   npm.localPrefix = join(pkgDirs, 'workspaces')
 
-  t.test('all workspaces', (t) => {
-    repo.execWorkspaces([], [], (err) => {
-      t.notOk(err)
-      t.match({
-        'https://repo.workspace-a/': 1, // Gets translated to https!
-        'https://github.com/npm/workspace-b': 1,
-      }, opened, 'opened two valid repo urls')
-      t.end()
-    })
+  t.afterEach(() => {
+    npm.config.set('workspaces', null)
+    npm.config.set('workspace', [])
+    npm.config.set('include-workspace-root', false)
   })
 
-  t.test('one workspace', (t) => {
-    repo.execWorkspaces([], ['workspace-a'], (err) => {
-      t.notOk(err)
-      t.match({
-        'https://repo.workspace-a/': 1,
-      }, opened, 'opened one requested repo urls')
-      t.end()
-    })
+  t.test('include workspace root', async (t) => {
+    npm.config.set('workspaces', true)
+    npm.config.set('include-workspace-root', true)
+    await command('repo', [])
+    t.match({
+      'https://github.com/npm/workspaces-test': 1,
+      'https://repo.workspace-a/': 1, // Gets translated to https!
+      'https://github.com/npm/workspace-b': 1,
+    }, opened, 'opened two valid repo urls')
   })
 
-  t.test('invalid workspace', (t) => {
-    repo.execWorkspaces([], ['workspace-x'], (err) => {
-      t.match(err, /No workspaces found/)
-      t.match(err, /workspace-x/)
-      t.match({}, opened, 'opened no repo urls')
-      t.end()
-    })
+  t.test('all workspaces', async (t) => {
+    npm.config.set('workspaces', true)
+    await command('repo', [])
+    t.match({
+      'https://repo.workspace-a/': 1, // Gets translated to https!
+      'https://github.com/npm/workspace-b': 1,
+    }, opened, 'opened two valid repo urls')
+  })
+
+  t.test('one workspace', async (t) => {
+    npm.config.set('workspace', ['workspace-a'])
+    await command('repo', [])
+    t.match({
+      'https://repo.workspace-a/': 1,
+    }, opened, 'opened one requested repo urls')
+  })
+
+  t.test('invalid workspace', async (t) => {
+    npm.config.set('workspace', ['workspace-x'])
+    await t.rejects(
+      command('repo', []),
+      /workspace-x/
+    )
+    t.match({}, opened, 'opened no repo urls')
   })
   t.end()
 })
diff --git a/test/lib/utils/completion/installed-deep.js b/test/lib/utils/completion/installed-deep.js
index aa0d85ec1..f0e36faee 100644
--- a/test/lib/utils/completion/installed-deep.js
+++ b/test/lib/utils/completion/installed-deep.js
@@ -6,6 +6,7 @@ let globalDir = 'MISSING_GLOBAL_DIR'
 const _flatOptions = {
   depth: Infinity,
   global: false,
+  workspacesEnabled: true,
   get prefix () {
     return prefix
   },
diff --git a/test/lib/utils/config/definitions.js b/test/lib/utils/config/definitions.js
index 88993303b..622e603bc 100644
--- a/test/lib/utils/config/definitions.js
+++ b/test/lib/utils/config/definitions.js
@@ -875,3 +875,20 @@ t.test('workspace', t => {
   t.match(flat.userAgent, /workspaces\/true/)
   t.end()
 })
+
+t.test('workspaces derived', t => {
+  const obj = {
+    workspaces: ['a'],
+    'user-agent': definitions['user-agent'].default,
+  }
+  const flat = {}
+  definitions.workspaces.flatten('workspaces', obj, flat)
+  t.equal(flat.workspacesEnabled, true)
+  obj.workspaces = null
+  definitions.workspaces.flatten('workspaces', obj, flat)
+  t.equal(flat.workspacesEnabled, true)
+  obj.workspaces = false
+  definitions.workspaces.flatten('workspaces', obj, flat)
+  t.equal(flat.workspacesEnabled, false)
+  t.end()
+})
diff --git a/test/lib/workspaces/get-workspaces.js b/test/lib/workspaces/get-workspaces.js
index 4ea055e02..0f51d95fc 100644
--- a/test/lib/workspaces/get-workspaces.js
+++ b/test/lib/workspaces/get-workspaces.js
@@ -86,6 +86,17 @@ t.test('get-workspaces', async t => {
     'should filter by package name'
   )
 
+  workspaces = await getWorkspaces(['a', 'b'], { path, includeWorkspaceRoot: true })
+  t.same(
+    clean(workspaces, path),
+    new Map(Object.entries({
+      x: '{PATH}',
+      a: '{PATH}/packages/a',
+      b: '{PATH}/packages/b',
+    })),
+    'include rootspace root'
+  )
+
   workspaces = await getWorkspaces(['./packages/c'], { path })
   t.same(
     clean(workspaces, path),