fix(workspaces): explicitly error in global mode

Also includes a preliminary refactor to consolidate workspace logic now
that every command that supports workspaces has it implemented.

PR-URL: #3417
Credit: @wraithgar
Close: #3417
Reviewed-by: @ruyadorno

lib/audit.js

@@ -58,7 +58,7 @@ class Audit extends ArboristWorkspaceCmd {
       audit: true,
       path: this.npm.prefix,
       reporter,
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     }
 
     const arb = new Arborist(opts)

lib/base-command.js

@@ -1,6 +1,7 @@
 // Base class for npm.commands[cmd]
 const usageUtil = require('./utils/usage.js')
 const ConfigDefinitions = require('./utils/config/definitions.js')
+const getWorkspaces = require('./workspaces/get-workspaces.js')
 
 class BaseCommand {
   constructor (npm) {
@@ -72,5 +73,13 @@ class BaseCommand {
       { code: 'ENOWORKSPACES' }
     )
   }
+
+  async setWorkspaces (filters) {
+    // TODO npm guards workspaces/global mode so we should use this.npm.prefix?
+    const ws = await getWorkspaces(filters, { path: this.npm.localPrefix })
+    this.workspaces = ws
+    this.workspaceNames = [...ws.keys()]
+    this.workspacePaths = [...ws.values()]
+  }
 }
 module.exports = BaseCommand

lib/ci.js

@@ -55,7 +55,7 @@ class CI extends ArboristWorkspaceCmd {
       path: where,
       log: this.npm.log,
       save: false, // npm ci should never modify the lockfile or package.json
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     }
 
     const arb = new Arborist(opts)

lib/dedupe.js

@@ -50,7 +50,7 @@ class Dedupe extends ArboristWorkspaceCmd {
       log: this.npm.log,
       path: where,
       dryRun,
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     }
     const arb = new Arborist(opts)
     await arb.dedupe(opts)

lib/diff.js

@@ -8,7 +8,6 @@ const npmlog = require('npmlog')
 const pacote = require('pacote')
 const pickManifest = require('npm-pick-manifest')
 
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 const readPackageName = require('./utils/read-package-name.js')
 const BaseCommand = require('./base-command.js')
 
@@ -90,9 +89,8 @@ class Diff extends BaseCommand {
   }
 
   async diffWorkspaces (args, filters) {
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
-    for (const workspacePath of workspaces.values()) {
+    await this.setWorkspaces(filters)
+    for (const workspacePath of this.workspacePaths) {
       this.top = workspacePath
       this.prefix = workspacePath
       await this.diff(args)
@@ -104,7 +102,6 @@ class Diff extends BaseCommand {
   async packageName (path) {
     let name
     try {
-      // TODO this won't work as expected in global mode
       name = await readPackageName(this.prefix)
     } catch (e) {
       npmlog.verbose('diff', 'could not read project dir package.json')

lib/dist-tag.js

@@ -5,7 +5,6 @@ const semver = require('semver')
 
 const otplease = require('./utils/otplease.js')
 const readPackageName = require('./utils/read-package-name.js')
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 const BaseCommand = require('./base-command.js')
 
 class DistTag extends BaseCommand {
@@ -180,10 +179,9 @@ class DistTag extends BaseCommand {
   }
 
   async listWorkspaces (filters) {
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
+    await this.setWorkspaces(filters)
 
-    for (const [name] of workspaces) {
+    for (const name of this.workspaceNames) {
       try {
         this.npm.output(`${name}:`)
         await this.list(npa(name), this.npm.flatOptions)

lib/docs.js

@@ -2,7 +2,6 @@ const log = require('npmlog')
 const pacote = require('pacote')
 const openUrl = require('./utils/open-url.js')
 const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 
 const BaseCommand = require('./base-command.js')
 class Docs extends BaseCommand {
@@ -42,9 +41,8 @@ class Docs extends BaseCommand {
   }
 
   async docsWorkspaces (args, filters) {
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
-    return this.docs([...workspaces.values()])
+    await this.setWorkspaces(filters)
+    return this.docs(this.workspacePaths)
   }
 
   async getDocs (pkg) {

lib/exec.js

@@ -1,7 +1,6 @@
 const libexec = require('libnpmexec')
 const BaseCommand = require('./base-command.js')
 const getLocationMsg = require('./exec/get-workspace-location-msg.js')
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 
 // it's like this:
 //
@@ -105,16 +104,15 @@ class Exec extends BaseCommand {
   }
 
   async _execWorkspaces (args, filters) {
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
+    await this.setWorkspaces(filters)
     const color = this.npm.config.get('color')
 
-    for (const workspacePath of workspaces.values()) {
-      const locationMsg = await getLocationMsg({ color, path: workspacePath })
+    for (const path of this.workspacePaths) {
+      const locationMsg = await getLocationMsg({ color, path })
       await this._exec(args, {
         locationMsg,
-        path: workspacePath,
-        runPath: workspacePath,
+        path,
+        runPath: path,
       })
     }
   }

lib/explain.js

@@ -46,8 +46,8 @@ class Explain extends ArboristWorkspaceCmd {
     const arb = new Arborist({ path: this.npm.prefix, ...this.npm.flatOptions })
     const tree = await arb.loadActual()
 
-    if (this.workspaces && this.workspaces.length)
-      this.filterSet = arb.workspaceDependencySet(tree, this.workspaces)
+    if (this.workspaceNames && this.workspaceNames.length)
+      this.filterSet = arb.workspaceDependencySet(tree, this.workspaceNames)
 
     const nodes = new Set()
     for (const arg of args) {

lib/fund.js

@@ -95,7 +95,7 @@ class Fund extends ArboristWorkspaceCmd {
     const fundingInfo = getFundingInfo(tree, {
       ...this.flatOptions,
       log: this.npm.log,
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     })
 
     if (this.npm.config.get('json'))

lib/install.js

@@ -144,7 +144,7 @@ class Install extends ArboristWorkspaceCmd {
       auditLevel: null,
       path: where,
       add: args,
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     }
     const arb = new Arborist(opts)
     await arb.reify(opts)

lib/link.js

@@ -146,7 +146,7 @@ class Link extends ArboristWorkspaceCmd {
       log: this.npm.log,
       add: names.map(l => `file:${resolve(globalTop, 'node_modules', l)}`),
       save,
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     })
 
     await reifyFinish(this.npm, localArb)

lib/ls.js

@@ -94,8 +94,8 @@ class LS extends ArboristWorkspaceCmd {
     // We only have to filter the first layer of edges, so we don't
     // explore anything that isn't part of the selected workspace set.
     let wsNodes
-    if (this.workspaces && this.workspaces.length)
-      wsNodes = arb.workspaceNodes(tree, this.workspaces)
+    if (this.workspaceNames && this.workspaceNames.length)
+      wsNodes = arb.workspaceNodes(tree, this.workspaceNames)
     const filterBySelectedWorkspaces = edge => {
       if (!wsNodes || !wsNodes.length)
         return true

lib/npm.js

@@ -108,6 +108,9 @@ const npm = module.exports = new class extends EventEmitter {
       this.output(impl.usage)
       cb()
     } else if (filterByWorkspaces) {
+      if (this.config.get('global'))
+        return cb(new Error('Workspaces not supported for global packages'))
+
       impl.execWorkspaces(args, this.config.get('workspace'), er => {
         process.emit('timeEnd', `command:${cmd}`)
         cb(er)

lib/outdated.js

@@ -59,8 +59,10 @@ class Outdated extends ArboristWorkspaceCmd {
     this.list = []
     this.tree = await arb.loadActual()
 
-    if (this.workspaces && this.workspaces.length)
-      this.filterSet = arb.workspaceDependencySet(this.tree, this.workspaces)
+    if (this.workspaceNames && this.workspaceNames.length) {
+      this.filterSet =
+        arb.workspaceDependencySet(this.tree, this.workspaceNames)
+    }
 
     if (args.length !== 0) {
       // specific deps

lib/pack.js

@@ -3,7 +3,6 @@ const log = require('npmlog')
 const pacote = require('pacote')
 const libpack = require('libnpmpack')
 const npa = require('npm-package-arg')
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 
 const { getContents, logTar } = require('./utils/tar.js')
 
@@ -97,9 +96,8 @@ class Pack extends BaseCommand {
       return this.pack(args)
     }
 
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
-    return this.pack([...workspaces.values(), ...args.filter(a => a !== '.')])
+    await this.setWorkspaces(filters)
+    return this.pack([...this.workspacePaths, ...args.filter(a => a !== '.')])
   }
 }
 module.exports = Pack

lib/prune.js

@@ -34,7 +34,7 @@ class Prune extends ArboristWorkspaceCmd {
       ...this.npm.flatOptions,
       path: where,
       log: this.npm.log,
-      workspaces: this.workspaces,
+      workspaces: this.workspaceNames,
     }
     const arb = new Arborist(opts)
     await arb.prune(opts)

lib/publish.js

@@ -11,7 +11,6 @@ const chalk = require('chalk')
 
 const otplease = require('./utils/otplease.js')
 const { getContents, logTar } = require('./utils/tar.js')
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 
 // for historical reasons, publishConfig in package.json can contain ANY config
 // keys that npm supports in .npmrc files and elsewhere.  We *may* want to
@@ -138,7 +137,7 @@ class Publish extends BaseCommand {
       })
     }
 
-    if (!this.workspaces) {
+    if (!this.suppressOutput) {
       if (!silent && json)
         this.npm.output(JSON.stringify(pkgContents, null, 2))
       else if (!silent)
@@ -150,17 +149,16 @@ class Publish extends BaseCommand {
 
   async publishWorkspaces (args, filters) {
     // Suppresses JSON output in publish() so we can handle it here
-    this.workspaces = true
+    this.suppressOutput = true
 
     const results = {}
     const json = this.npm.config.get('json')
     const silent = log.level === 'silent'
     const noop = a => a
     const color = this.npm.color ? chalk : { green: noop, bold: noop }
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
+    await this.setWorkspaces(filters)
 
-    for (const [name, workspace] of workspaces.entries()) {
+    for (const [name, workspace] of this.workspaces.entries()) {
       let pkgContents
       try {
         pkgContents = await this.publish([workspace])

lib/rebuild.js

@@ -47,7 +47,7 @@ class Rebuild extends ArboristWorkspaceCmd {
       ...this.npm.flatOptions,
       path: where,
       // TODO when extending ReifyCmd
-      // workspaces: this.workspaces,
+      // workspaces: this.workspaceNames,
     })
 
     if (args.length) {

lib/repo.js

@@ -1,6 +1,5 @@
 const log = require('npmlog')
 const pacote = require('pacote')
-const getWorkspaces = require('./workspaces/get-workspaces.js')
 const { URL } = require('url')
 
 const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
@@ -44,9 +43,8 @@ class Repo extends BaseCommand {
   }
 
   async repoWorkspaces (args, filters) {
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
-    return this.repo([...workspaces.values()])
+    await this.setWorkspaces(filters)
+    return this.repo(this.workspacePaths)
   }
 
   async get (pkg) {

lib/run-script.js

@@ -6,7 +6,6 @@ const rpj = require('read-package-json-fast')
 const log = require('npmlog')
 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',
@@ -195,10 +194,9 @@ class RunScript extends BaseCommand {
 
   async runWorkspaces (args, filters) {
     const res = []
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
+    await this.setWorkspaces(filters)
 
-    for (const workspacePath of workspaces.values()) {
+    for (const workspacePath of this.workspacePaths) {
       const pkg = await rpj(`${workspacePath}/package.json`)
       const runResult = await this.run(args, {
         path: workspacePath,
@@ -227,15 +225,14 @@ class RunScript extends BaseCommand {
   }
 
   async listWorkspaces (args, filters) {
-    const workspaces =
-      await getWorkspaces(filters, { path: this.npm.localPrefix })
+    await this.setWorkspaces(filters)
 
     if (log.level === 'silent')
       return
 
     if (this.npm.config.get('json')) {
       const res = {}
-      for (const workspacePath of workspaces.values()) {
+      for (const workspacePath of this.workspacePaths) {
         const { scripts, name } = await rpj(`${workspacePath}/package.json`)
         res[name] = { ...scripts }
       }
@@ -244,15 +241,15 @@ class RunScript extends BaseCommand {
     }
 
     if (this.npm.config.get('parseable')) {
-      for (const workspacePath of workspaces.values()) {
+      for (const workspacePath of this.workspacePaths) {
         const { scripts, name } = await rpj(`${workspacePath}/package.json`)
         for (const [script, cmd] of Object.entries(scripts || {}))
           this.npm.output(`${name}:${script}:${cmd}`)
       }
       return
     }
 
-    for (const workspacePath of workspaces.values())
+    for (const workspacePath of this.workspacePaths)
       await this.list(args, workspacePath)
   }
 }