From d5c32899b6ffc6254c96f62a06a854bb2c2b95c5 Mon Sep 17 00:00:00 2001
From: Luke Karrys <luke@lukekarrys.com>
Date: Wed, 15 May 2024 12:44:14 -0700
Subject: [PATCH] fix(refactor): use output buffer and error for more commands
 (#7513)

This changes a bunch of commands to use the new `output.buffer`
capabilities from `proc-log` as well as the `outputError` helper from
`utils/output-error.js`.

This also adds a few comments about future display related breaking
changes that npm should make.

There is some new behavior around `run-script` and how it outputs
errors. It now displays the error for each workspace similar to how that
error would get displayed when the process exits.
---
 lib/commands/ls.js                            |   2 +-
 lib/commands/pack.js                          |  18 +-
 lib/commands/pkg.js                           |  74 +++----
 lib/commands/publish.js                       |  89 +++-----
 lib/commands/run-script.js                    | 200 ++++++++----------
 lib/commands/view.js                          |   5 +
 lib/npm.js                                    |   3 +
 lib/utils/display.js                          |  71 ++++---
 lib/utils/tar.js                              |   9 +-
 .../test/lib/commands/run-script.js.test.cjs  | 107 +++++-----
 test/lib/cli/exit-handler.js                  |   2 +-
 test/lib/commands/pkg.js                      | 137 +++++++-----
 12 files changed, 342 insertions(+), 375 deletions(-)

diff --git a/lib/commands/ls.js b/lib/commands/ls.js
index 51e99f429816a..7dbe14fea20bc 100644
--- a/lib/commands/ls.js
+++ b/lib/commands/ls.js
@@ -554,7 +554,7 @@ const jsonOutput = ({ path, problems, result, rootError, seenItems }) => {
     }
   }
 
-  return JSON.stringify(result, null, 2)
+  return result
 }
 
 const parseableOutput = ({ global, long, seenNodes }) => {
diff --git a/lib/commands/pack.js b/lib/commands/pack.js
index f64a21dcc0d9d..79e7f49f819ec 100644
--- a/lib/commands/pack.js
+++ b/lib/commands/pack.js
@@ -53,18 +53,16 @@ class Pack extends BaseCommand {
         prefix: this.npm.localPrefix,
         workspaces: this.workspacePaths,
       })
-      const pkgContents = await getContents(manifest, tarballData)
-      tarballs.push(pkgContents)
+      tarballs.push(await getContents(manifest, tarballData))
     }
 
-    if (json) {
-      output.standard(JSON.stringify(tarballs, null, 2))
-      return
-    }
-
-    for (const tar of tarballs) {
-      logTar(tar, { unicode })
-      output.standard(tar.filename.replace(/^@/, '').replace(/\//, '-'))
+    for (const [index, tar] of Object.entries(tarballs)) {
+      // XXX(BREAKING_CHANGE): publish outputs a json object with package
+      // names as keys. Pack should do the same here instead of an array
+      logTar(tar, { unicode, json, key: index })
+      if (!json) {
+        output.standard(tar.filename.replace(/^@/, '').replace(/\//, '-'))
+      }
     }
   }
 
diff --git a/lib/commands/pkg.js b/lib/commands/pkg.js
index 62553b15103e3..a108d79c51517 100644
--- a/lib/commands/pkg.js
+++ b/lib/commands/pkg.js
@@ -25,13 +25,7 @@ class Pkg extends BaseCommand {
   static workspaces = true
   static ignoreImplicitWorkspace = false
 
-  async exec (args, { prefix } = {}) {
-    if (!prefix) {
-      this.prefix = this.npm.localPrefix
-    } else {
-      this.prefix = prefix
-    }
-
+  async exec (args, { path = this.npm.localPrefix, workspace } = {}) {
     if (this.npm.global) {
       throw Object.assign(
         new Error(`There's no package.json file to manage on global mode`),
@@ -42,13 +36,13 @@ class Pkg extends BaseCommand {
     const [cmd, ..._args] = args
     switch (cmd) {
       case 'get':
-        return this.get(_args)
+        return this.get(_args, { path, workspace })
       case 'set':
-        return this.set(_args)
+        return this.set(_args, { path, workspace }).then(p => p.save())
       case 'delete':
-        return this.delete(_args)
+        return this.delete(_args, { path, workspace }).then(p => p.save())
       case 'fix':
-        return this.fix(_args)
+        return PackageJson.fix(path).then(p => p.save())
       default:
         throw this.usageError()
     }
@@ -56,43 +50,40 @@ class Pkg extends BaseCommand {
 
   async execWorkspaces (args) {
     await this.setWorkspaces()
-    const result = {}
-    for (const [workspaceName, workspacePath] of this.workspaces.entries()) {
-      this.prefix = workspacePath
-      result[workspaceName] = await this.exec(args, { prefix: workspacePath })
+    for (const [workspace, path] of this.workspaces.entries()) {
+      await this.exec(args, { path, workspace })
     }
-    // when running in workspaces names, make sure to key by workspace
-    // name the results of each value retrieved in each ws
-    output.standard(JSON.stringify(result, null, 2))
   }
 
-  async get (args) {
-    const pkgJson = await PackageJson.load(this.prefix)
-
-    const { content } = pkgJson
-    let result = !args.length && content
+  async get (args, { path, workspace }) {
+    this.npm.config.set('json', true)
+    const pkgJson = await PackageJson.load(path)
 
-    if (!result) {
-      const q = new Queryable(content)
-      result = q.query(args)
+    let unwrap = false
+    let result = pkgJson.content
 
+    if (args.length) {
+      result = new Queryable(result).query(args)
       // in case there's only a single result from the query
       // just prints that one element to stdout
       if (Object.keys(result).length === 1) {
+        unwrap = true
         result = result[args]
       }
     }
 
-    // only outputs if not running with workspaces config
-    // execWorkspaces will handle the output otherwise
-    if (!this.workspaces) {
-      output.standard(JSON.stringify(result, null, 2))
+    if (workspace) {
+      // workspaces are always json
+      output.buffer({ [workspace]: result })
+    } else {
+      // if the result was unwrapped, stringify as json which will add quotes around strings
+      // TODO: https://github.com/npm/cli/issues/5508 a raw mode has been requested similar
+      // to jq -r. If that was added then it would conditionally not call JSON.stringify here
+      output.buffer(unwrap ? JSON.stringify(result) : result)
     }
-
-    return result
   }
 
-  async set (args) {
+  async set (args, { path }) {
     const setError = () =>
       this.usageError('npm pkg set expects a key=value pair of args.')
 
@@ -102,7 +93,7 @@ class Pkg extends BaseCommand {
 
     const force = this.npm.config.get('force')
     const json = this.npm.config.get('json')
-    const pkgJson = await PackageJson.load(this.prefix)
+    const pkgJson = await PackageJson.load(path)
     const q = new Queryable(pkgJson.content)
     for (const arg of args) {
       const [key, ...rest] = arg.split('=')
@@ -114,11 +105,10 @@ class Pkg extends BaseCommand {
       q.set(key, json ? JSON.parse(value) : value, { force })
     }
 
-    pkgJson.update(q.toJSON())
-    await pkgJson.save()
+    return pkgJson.update(q.toJSON())
   }
 
-  async delete (args) {
+  async delete (args, { path }) {
     const setError = () =>
       this.usageError('npm pkg delete expects key args.')
 
@@ -126,7 +116,7 @@ class Pkg extends BaseCommand {
       throw setError()
     }
 
-    const pkgJson = await PackageJson.load(this.prefix)
+    const pkgJson = await PackageJson.load(path)
     const q = new Queryable(pkgJson.content)
     for (const key of args) {
       if (!key) {
@@ -136,13 +126,7 @@ class Pkg extends BaseCommand {
       q.delete(key)
     }
 
-    pkgJson.update(q.toJSON())
-    await pkgJson.save()
-  }
-
-  async fix () {
-    const pkgJson = await PackageJson.fix(this.prefix)
-    await pkgJson.save()
+    return pkgJson.update(q.toJSON())
   }
 }
 
diff --git a/lib/commands/publish.js b/lib/commands/publish.js
index 3862f659bb508..6f893b929284b 100644
--- a/lib/commands/publish.js
+++ b/lib/commands/publish.js
@@ -43,6 +43,26 @@ class Publish extends BaseCommand {
       throw this.usageError()
     }
 
+    await this.#publish(args)
+  }
+
+  async execWorkspaces () {
+    await this.setWorkspaces()
+
+    for (const [name, workspace] of this.workspaces.entries()) {
+      try {
+        await this.#publish([workspace], { workspace: name })
+      } catch (err) {
+        if (err.code !== 'EPRIVATE') {
+          throw err
+        }
+        // eslint-disable-next-line max-len
+        log.warn('publish', `Skipping workspace ${this.npm.chalk.cyan(name)}, marked as ${this.npm.chalk.bold('private')}`)
+      }
+    }
+  }
+
+  async #publish (args, { workspace } = {}) {
     log.verbose('publish', replaceInfo(args))
 
     const unicode = this.npm.config.get('unicode')
@@ -61,7 +81,7 @@ class Publish extends BaseCommand {
     // you can publish name@version, ./foo.tgz, etc.
     // even though the default is the 'file:.' cwd.
     const spec = npa(args[0])
-    let manifest = await this.getManifest(spec, opts)
+    let manifest = await this.#getManifest(spec, opts)
 
     // only run scripts for directory type publishes
     if (spec.type === 'directory' && !ignoreScripts) {
@@ -84,15 +104,17 @@ class Publish extends BaseCommand {
       workspaces: this.workspacePaths,
     })
     const pkgContents = await getContents(manifest, tarballData)
+    const logPkg = () => logTar(pkgContents, { unicode, json, key: workspace })
 
     // The purpose of re-reading the manifest is in case it changed,
     // so that we send the latest and greatest thing to the registry
     // note that publishConfig might have changed as well!
-    manifest = await this.getManifest(spec, opts, true)
+    manifest = await this.#getManifest(spec, opts, true)
 
-    // JSON already has the package contents
+    // If we are not in JSON mode then we show the user the contents of the tarball
+    // before it is published so they can see it while their otp is pending
     if (!json) {
-      logTar(pkgContents, { unicode })
+      logPkg()
     }
 
     const resolved = npa.resolve(manifest.name, manifest.version)
@@ -126,6 +148,12 @@ class Publish extends BaseCommand {
       await otplease(this.npm, opts, o => libpub(manifest, tarballData, o))
     }
 
+    // In json mode we dont log until the publish has completed as this will
+    // add it to the output only if completes successfully
+    if (json) {
+      logPkg()
+    }
+
     if (spec.type === 'directory' && !ignoreScripts) {
       await runScript({
         event: 'publish',
@@ -142,62 +170,15 @@ class Publish extends BaseCommand {
       })
     }
 
-    if (!this.suppressOutput) {
-      if (!silent && json) {
-        output.standard(JSON.stringify(pkgContents, null, 2))
-      } else if (!silent) {
-        output.standard(`+ ${pkgContents.id}`)
-      }
-    }
-
-    return pkgContents
-  }
-
-  async execWorkspaces () {
-    // Suppresses JSON output in publish() so we can handle it here
-    this.suppressOutput = true
-
-    const results = {}
-    const json = this.npm.config.get('json')
-    const { silent } = this.npm
-    await this.setWorkspaces()
-
-    for (const [name, workspace] of this.workspaces.entries()) {
-      let pkgContents
-      try {
-        pkgContents = await this.exec([workspace])
-      } catch (err) {
-        if (err.code === 'EPRIVATE') {
-          log.warn(
-            'publish',
-            `Skipping workspace ${
-              this.npm.chalk.cyan(name)
-            }, marked as ${
-              this.npm.chalk.bold('private')
-            }`
-          )
-          continue
-        }
-        throw err
-      }
-      // This needs to be in-line w/ the rest of the output that non-JSON
-      // publish generates
-      if (!silent && !json) {
-        output.standard(`+ ${pkgContents.id}`)
-      } else {
-        results[name] = pkgContents
-      }
-    }
-
-    if (!silent && json) {
-      output.standard(JSON.stringify(results, null, 2))
+    if (!json && !silent) {
+      output.standard(`+ ${pkgContents.id}`)
     }
   }
 
   // if it's a directory, read it from the file system
   // otherwise, get the full metadata from whatever it is
   // XXX can't pacote read the manifest from a directory?
-  async getManifest (spec, opts, logWarnings = false) {
+  async #getManifest (spec, opts, logWarnings = false) {
     let manifest
     if (spec.type === 'directory') {
       const changes = []
diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js
index 3030ba0f7a2c1..180dfa1cdeac5 100644
--- a/lib/commands/run-script.js
+++ b/lib/commands/run-script.js
@@ -1,6 +1,8 @@
-const { log, output } = require('proc-log')
+const { output } = require('proc-log')
 const pkgJson = require('@npmcli/package-json')
 const BaseCommand = require('../base-cmd.js')
+const { getError } = require('../utils/error-message.js')
+const { outputError } = require('../utils/output-error.js')
 
 class RunScript extends BaseCommand {
   static description = 'Run arbitrary package scripts'
@@ -34,27 +36,54 @@ class RunScript extends BaseCommand {
 
   async exec (args) {
     if (args.length) {
-      return this.run(args)
+      await this.#run(args, { path: this.npm.localPrefix })
     } else {
-      return this.list(args)
+      await this.#list(this.npm.localPrefix)
     }
   }
 
   async execWorkspaces (args) {
-    if (args.length) {
-      return this.runWorkspaces(args)
-    } else {
-      return this.listWorkspaces(args)
+    await this.setWorkspaces()
+
+    const ws = [...this.workspaces.entries()]
+    for (const [workspace, path] of ws) {
+      const last = path === ws.at(-1)[1]
+
+      if (!args.length) {
+        const newline = await this.#list(path, { workspace })
+        if (newline && !last) {
+          output.standard('')
+        }
+        continue
+      }
+
+      const pkg = await pkgJson.normalize(path).then(p => p.content)
+      try {
+        await this.#run(args, { path, pkg, workspace })
+      } catch (e) {
+        const err = getError(e, { npm: this.npm, command: null })
+        outputError({
+          ...err,
+          error: [
+            ['', `Lifecycle script \`${args[0]}\` failed with error:`],
+            ...err.error,
+            ['workspace', pkg._id || pkg.name],
+            ['location', path],
+          ],
+        })
+        process.exitCode = err.exitCode
+        if (!last) {
+          output.error('')
+        }
+      }
     }
   }
 
-  async run ([event, ...args], { path = this.npm.localPrefix, pkg } = {}) {
+  async #run ([event, ...args], { path, pkg, workspace }) {
     const runScript = require('@npmcli/run-script')
 
-    if (!pkg) {
-      const { content } = await pkgJson.normalize(path)
-      pkg = content
-    }
+    pkg ??= await pkgJson.normalize(path).then(p => p.content)
+
     const { scripts = {} } = pkg
 
     if (event === 'restart' && !scripts.restart) {
@@ -75,11 +104,13 @@ class RunScript extends BaseCommand {
       }
 
       const suggestions = require('../utils/did-you-mean.js')(pkg, event)
+      const wsArg = workspace && path !== this.npm.localPrefix
+        ? ` --workspace=${pkg._id || pkg.name}`
+        : ''
       throw new Error([
-        `Missing script: "${event}"`,
-        suggestions,
+        `Missing script: "${event}"${suggestions}\n`,
         'To see a list of scripts, run:',
-        '  npm run',
+        `  npm run${wsArg}`,
       ].join('\n'))
     }
 
@@ -109,31 +140,29 @@ class RunScript extends BaseCommand {
     }
   }
 
-  async list (args, path) {
-    /* eslint-disable-next-line max-len */
-    const { content: { scripts, name, _id } } = await pkgJson.normalize(path || this.npm.localPrefix)
-    const pkgid = _id || name
-
-    if (!scripts) {
-      return []
-    }
+  async #list (path, { workspace } = {}) {
+    const { scripts = {}, name, _id } = await pkgJson.normalize(path).then(p => p.content)
+    const scriptEntries = Object.entries(scripts)
 
-    const allScripts = Object.keys(scripts)
     if (this.npm.silent) {
-      return allScripts
+      return
     }
 
     if (this.npm.config.get('json')) {
-      output.standard(JSON.stringify(scripts, null, 2))
-      return allScripts
+      output.buffer(workspace ? { [workspace]: scripts } : scripts)
+      return
     }
 
-    if (this.npm.config.get('parseable')) {
-      for (const [script, cmd] of Object.entries(scripts)) {
-        output.standard(`${script}:${cmd}`)
-      }
+    if (!scriptEntries.length) {
+      return
+    }
 
-      return allScripts
+    if (this.npm.config.get('parseable')) {
+      output.standard(scriptEntries
+        .map((s) => (workspace ? [workspace, ...s] : s).join(':'))
+        .join('\n')
+        .trim())
+      return
     }
 
     // TODO this is missing things like prepare, prepublishOnly, and dependencies
@@ -147,96 +176,39 @@ class RunScript extends BaseCommand {
       'preuninstall', 'uninstall', 'postuninstall',
       'preversion', 'version', 'postversion',
     ]
-    const indent = '\n    '
-    const prefix = '  '
-    const cmds = []
-    const runScripts = []
-    for (const script of allScripts) {
-      const list = cmdList.includes(script) ? cmds : runScripts
-      list.push(script)
-    }
-    const colorize = this.npm.chalk
-
-    if (cmds.length) {
-      output.standard(
-        `${colorize.reset(colorize.bold('Lifecycle scripts'))} included in ${colorize.green(
-          pkgid
-        )}:`
-      )
-    }
-
-    for (const script of cmds) {
-      output.standard(prefix + script + indent + colorize.dim(scripts[script]))
-    }
-
-    if (!cmds.length && runScripts.length) {
-      output.standard(
-        `${colorize.bold('Scripts')} available in ${colorize.green(pkgid)} via \`${colorize.blue(
-          'npm run-script'
-        )}\`:`
-      )
-    } else if (runScripts.length) {
-      output.standard(`\navailable via \`${colorize.blue('npm run-script')}\`:`)
-    }
-
-    for (const script of runScripts) {
-      output.standard(prefix + script + indent + colorize.dim(scripts[script]))
-    }
-
-    output.standard('')
-    return allScripts
-  }
+    const [cmds, runScripts] = scriptEntries.reduce((acc, s) => {
+      acc[cmdList.includes(s[0]) ? 0 : 1].push(s)
+      return acc
+    }, [[], []])
 
-  async runWorkspaces (args) {
-    const res = []
-    await this.setWorkspaces()
+    const { reset, bold, cyan, dim, blue } = this.npm.chalk
+    const pkgId = `in ${cyan(_id || name)}`
+    const title = (t) => reset(bold(t))
 
-    for (const workspacePath of this.workspacePaths) {
-      const { content: pkg } = await pkgJson.normalize(workspacePath)
-      const runResult = await this.run(args, {
-        path: workspacePath,
-        pkg,
-      }).catch(err => {
-        log.error(`Lifecycle script \`${args[0]}\` failed with error:`)
-        log.error(err)
-        log.error(`  in workspace: ${pkg._id || pkg.name}`)
-        log.error(`  at location: ${workspacePath}`)
-        process.exitCode = 1
-      })
-      res.push(runResult)
-    }
-  }
-
-  async listWorkspaces (args) {
-    await this.setWorkspaces()
-
-    if (this.npm.silent) {
-      return
-    }
-
-    if (this.npm.config.get('json')) {
-      const res = {}
-      for (const workspacePath of this.workspacePaths) {
-        const { content: { scripts, name } } = await pkgJson.normalize(workspacePath)
-        res[name] = { ...scripts }
+    if (cmds.length) {
+      output.standard(`${title('Lifecycle scripts')} included ${pkgId}:`)
+      for (const [k, v] of cmds) {
+        output.standard(`  ${k}`)
+        output.standard(`    ${dim(v)}`)
       }
-      output.standard(JSON.stringify(res, null, 2))
-      return
     }
 
-    if (this.npm.config.get('parseable')) {
-      for (const workspacePath of this.workspacePaths) {
-        const { content: { scripts, name } } = await pkgJson.normalize(workspacePath)
-        for (const [script, cmd] of Object.entries(scripts || {})) {
-          output.standard(`${name}:${script}:${cmd}`)
-        }
+    if (runScripts.length) {
+      const via = `via \`${blue('npm run-script')}\`:`
+      if (!cmds.length) {
+        output.standard(`${title('Scripts')} available ${pkgId} ${via}`)
+      } else {
+        output.standard(`available ${via}`)
+      }
+      for (const [k, v] of runScripts) {
+        output.standard(`  ${k}`)
+        output.standard(`    ${dim(v)}`)
       }
-      return
     }
 
-    for (const workspacePath of this.workspacePaths) {
-      await this.list(args, workspacePath)
-    }
+    // Return true to indicate that something was output for this path
+    // that should be separated from others
+    return true
   }
 }
 
diff --git a/lib/commands/view.js b/lib/commands/view.js
index 316019772f606..8c6b5be3a0643 100644
--- a/lib/commands/view.js
+++ b/lib/commands/view.js
@@ -224,6 +224,11 @@ class View extends BaseCommand {
     })
 
     if (json) {
+      // TODO(BREAKING_CHANGE): all unwrapping should be removed. Users should know
+      // based on their arguments if they can expect an array or an object. And this
+      // unwrapping can break that assumption. Eg `npm view abbrev@^2` should always
+      // return an array, but currently since there is only one version matching `^2`
+      // this will return a single object instead.
       const first = Object.keys(res[0] || {})
       const jsonRes = first.length === 1 ? res.map(m => m[first[0]]) : res
       if (jsonRes.length === 0) {
diff --git a/lib/npm.js b/lib/npm.js
index 28acdbbdd6fd7..9d00b630857b1 100644
--- a/lib/npm.js
+++ b/lib/npm.js
@@ -301,6 +301,9 @@ class Npm {
 
     output.flush({
       [META]: true,
+      // json can be set during a command so we send the
+      // final value of it to the display layer here
+      json: this.loaded && this.config.get('json'),
       jsonError: jsonError(err, this),
     })
 
diff --git a/lib/utils/display.js b/lib/utils/display.js
index dbb1834645710..ede2588cacfc1 100644
--- a/lib/utils/display.js
+++ b/lib/utils/display.js
@@ -65,17 +65,6 @@ const LEVEL_METHODS = {
   },
 }
 
-const tryJsonParse = (value) => {
-  if (typeof value === 'string') {
-    try {
-      return JSON.parse(value)
-    } catch {
-      return
-    }
-  }
-  return value
-}
-
 const setBlocking = (stream) => {
   // Copied from https://github.com/yargs/set-blocking
   // https://raw.githubusercontent.com/yargs/set-blocking/master/LICENSE.txt
@@ -93,33 +82,45 @@ const ERROR_KEY = 'error'
 // is a json error that should be merged into the finished output
 const JSON_ERROR_KEY = 'jsonError'
 
-const mergeJson = (meta, buffer) => {
-  const buffered = buffer.reduce((acc, i) => {
-    // index 2 is the logged argument
-    acc[0].push(tryJsonParse(i[2]))
-    // index 1 is the meta object
-    acc[1].push(i[1][JSON_ERROR_KEY])
-    return acc
-  }, [
-    // meta also contains the meta object passed to flush
-    [], [meta[JSON_ERROR_KEY]],
-  ])
-
-  const items = buffered[0].filter(Boolean)
-  const errors = buffered[1].filter(Boolean)
+const getArrayOrObject = (items) => {
+  // Arrays cant be merged, if the first item is an array return that
+  if (Array.isArray(items[0])) {
+    return items[0]
+  }
+  // We use objects with 0,1,2,etc keys to merge array
+  if (items.length && items.every((o, i) => Object.hasOwn(o, i))) {
+    return Object.assign([], ...items)
+  }
+  // Otherwise its an object with all items merged together
+  return Object.assign({}, ...items)
+}
+
+const mergeJson = ({ [JSON_ERROR_KEY]: metaError }, buffer) => {
+  const items = []
+  // meta also contains the meta object passed to flush
+  const errors = metaError ? [metaError] : []
+  // index 1 is the meta, 2 is the logged argument
+  for (const [, { [JSON_ERROR_KEY]: error }, obj] of buffer) {
+    if (obj && typeof obj === 'object') {
+      items.push(obj)
+    }
+    /* istanbul ignore next - this is not used yet but will be */
+    if (error) {
+      errors.push(error)
+    }
+  }
+
+  if (!items.length && !errors.length) {
+    return null
+  }
 
   // If all items are keyed with array indexes, then we return the
   // array. This skips any error checking since we cant really set
   // an error property on an array in a way that can be stringified
   // XXX(BREAKING_CHANGE): remove this in favor of always returning an object
-  /* istanbul ignore next - premature optimization for a PR that comes next */
-  if (items.length && items.every((o, i) => Object.hasOwn(o, i))) {
-    return Object.assign([], ...items)
-  }
-
-  const res = Object.assign({}, ...items)
+  const res = getArrayOrObject(items)
 
-  if (errors.length) {
+  if (!Array.isArray(res) && errors.length) {
     // This is not ideal. JSON output has always been keyed at the root with an `error`
     // key, so we cant change that without it being a breaking change. At the same time
     // some commands output arbitrary keys at the top level of the output, such as package
@@ -130,11 +131,10 @@ const mergeJson = (meta, buffer) => {
     if (res[ERROR_KEY]) {
       log.warn('display', `overwriting existing ${ERROR_KEY} on json output`)
     }
-    res[ERROR_KEY] = Object.assign({}, ...errors)
+    res[ERROR_KEY] = getArrayOrObject(errors)
   }
 
-  // Only write output if we have some json buffered
-  return Object.keys(res).length ? res : null
+  return res
 }
 
 const withMeta = (handler) => (level, ...args) => {
@@ -290,6 +290,7 @@ class Display {
   // Arrow function assigned to a private class field so it can be passed
   // directly as a listener and still reference "this"
   #outputHandler = withMeta((level, meta, ...args) => {
+    this.#json = typeof meta.json === 'boolean' ? meta.json : this.#json
     switch (level) {
       case output.KEYS.flush: {
         this.#outputState.buffering = false
diff --git a/lib/utils/tar.js b/lib/utils/tar.js
index 9085d9dd35016..31f8bb089ed85 100644
--- a/lib/utils/tar.js
+++ b/lib/utils/tar.js
@@ -1,14 +1,17 @@
 const tar = require('tar')
 const ssri = require('ssri')
-const { log } = require('proc-log')
+const { log, output } = require('proc-log')
 const formatBytes = require('./format-bytes.js')
 const localeCompare = require('@isaacs/string-locale-compare')('en', {
   sensitivity: 'case',
   numeric: true,
 })
 
-const logTar = (tarball, opts = {}) => {
-  const { unicode = false } = opts
+const logTar = (tarball, { unicode = false, json, key } = {}) => {
+  if (json) {
+    output.buffer(key == null ? tarball : { [key]: tarball })
+    return
+  }
   log.notice('')
   log.notice('', `${unicode ? '📦 ' : 'package:'} ${tarball.name}@${tarball.version}`)
   log.notice('Tarball Contents')
diff --git a/tap-snapshots/test/lib/commands/run-script.js.test.cjs b/tap-snapshots/test/lib/commands/run-script.js.test.cjs
index a013fce282dd9..189e2f84d2901 100644
--- a/tap-snapshots/test/lib/commands/run-script.js.test.cjs
+++ b/tap-snapshots/test/lib/commands/run-script.js.test.cjs
@@ -13,7 +13,6 @@ Lifecycle scripts included in x@1.2.3:
     node server.js
   stop
     node kill-server.js
-
 available via \`npm run-script\`:
   preenv
     echo before the env
@@ -53,9 +52,10 @@ Scripts available in x@1.2.3 via \`npm run-script\`:
 
 exports[`test/lib/commands/run-script.js TAP workspaces failed workspace run with succeeded runs > should log error msgs for each workspace script 1`] = `
 Lifecycle script \`glorp\` failed with error:
-Error: ERR
-  in workspace: a@1.0.0
-  at location: {CWD}/prefix/packages/a
+code ERR
+workspace a@1.0.0
+location {CWD}/prefix/packages/a
+ERR
 `
 
 exports[`test/lib/commands/run-script.js TAP workspaces list all scripts --json > must match snapshot 1`] = `
@@ -109,7 +109,6 @@ Lifecycle scripts included in c@1.0.0:
     exit 0
   posttest
     echo posttest
-
 available via \`npm run-script\`:
   lorem
     echo c lorem
@@ -128,31 +127,30 @@ Lifecycle scripts included in e:
 `
 
 exports[`test/lib/commands/run-script.js TAP workspaces list all scripts with colors > must match snapshot 1`] = `
-Scripts available in a@1.0.0 via \`npm run-script\`:
+Scripts available in a@1.0.0 via \`npm run-script\`:
   glorp
     echo a doing the glerp glop
 
-Scripts available in b@2.0.0 via \`npm run-script\`:
+Scripts available in b@2.0.0 via \`npm run-script\`:
   glorp
     echo b doing the glerp glop
 
-Lifecycle scripts included in c@1.0.0:
+Lifecycle scripts included in c@1.0.0:
   test
     exit 0
   posttest
     echo posttest
-
 available via \`npm run-script\`:
   lorem
     echo c lorem
 
-Lifecycle scripts included in d@1.0.0:
+Lifecycle scripts included in d@1.0.0:
   test
     exit 0
   posttest
     echo posttest
 
-Lifecycle scripts included in e:
+Lifecycle scripts included in e:
   test
     exit 0
   start
@@ -183,7 +181,6 @@ Lifecycle scripts included in c@1.0.0:
     exit 0
   posttest
     echo posttest
-
 available via \`npm run-script\`:
   lorem
     echo c lorem
@@ -209,69 +206,69 @@ Scripts available in a@1.0.0 via \`npm run-script\`:
 
 exports[`test/lib/commands/run-script.js TAP workspaces missing scripts in all workspaces > should log error msgs for each workspace script 1`] = `
 Lifecycle script \`missing-script\` failed with error:
-Error: Missing script: "missing-script"
-
+workspace a@1.0.0
+location {CWD}/prefix/packages/a
+Missing script: "missing-script"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: a@1.0.0
-  at location: {CWD}/prefix/packages/a
+  npm run --workspace=a@1.0.0
 Lifecycle script \`missing-script\` failed with error:
-Error: Missing script: "missing-script"
-
+workspace b@2.0.0
+location {CWD}/prefix/packages/b
+Missing script: "missing-script"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: b@2.0.0
-  at location: {CWD}/prefix/packages/b
+  npm run --workspace=b@2.0.0
 Lifecycle script \`missing-script\` failed with error:
-Error: Missing script: "missing-script"
-
+workspace c@1.0.0
+location {CWD}/prefix/packages/c
+Missing script: "missing-script"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: c@1.0.0
-  at location: {CWD}/prefix/packages/c
+  npm run --workspace=c@1.0.0
 Lifecycle script \`missing-script\` failed with error:
-Error: Missing script: "missing-script"
-
+workspace d@1.0.0
+location {CWD}/prefix/packages/d
+Missing script: "missing-script"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: d@1.0.0
-  at location: {CWD}/prefix/packages/d
+  npm run --workspace=d@1.0.0
 Lifecycle script \`missing-script\` failed with error:
-Error: Missing script: "missing-script"
-
+workspace e
+location {CWD}/prefix/packages/e
+Missing script: "missing-script"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: e
-  at location: {CWD}/prefix/packages/e
+  npm run --workspace=e
 Lifecycle script \`missing-script\` failed with error:
-Error: Missing script: "missing-script"
-
+workspace noscripts@1.0.0
+location {CWD}/prefix/packages/noscripts
+Missing script: "missing-script"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: noscripts@1.0.0
-  at location: {CWD}/prefix/packages/noscripts
+  npm run --workspace=noscripts@1.0.0
 `
 
 exports[`test/lib/commands/run-script.js TAP workspaces missing scripts in some workspaces > should log error msgs for each workspace script 1`] = `
 Lifecycle script \`test\` failed with error:
-Error: Missing script: "test"
-
+workspace a@1.0.0
+location {CWD}/prefix/packages/a
+Missing script: "test"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: a@1.0.0
-  at location: {CWD}/prefix/packages/a
+  npm run --workspace=a@1.0.0
 Lifecycle script \`test\` failed with error:
-Error: Missing script: "test"
-
+workspace b@2.0.0
+location {CWD}/prefix/packages/b
+Missing script: "test"
+npm error
 To see a list of scripts, run:
-  npm run
-  in workspace: b@2.0.0
-  at location: {CWD}/prefix/packages/b
+  npm run --workspace=b@2.0.0
 `
 
 exports[`test/lib/commands/run-script.js TAP workspaces single failed workspace run > should log error msgs for each workspace script 1`] = `
 Lifecycle script \`test\` failed with error:
-Error: err
-  in workspace: c@1.0.0
-  at location: {CWD}/prefix/packages/c
+workspace c@1.0.0
+location {CWD}/prefix/packages/c
+err
 `
diff --git a/test/lib/cli/exit-handler.js b/test/lib/cli/exit-handler.js
index 0077596fab642..da050bbb0708e 100644
--- a/test/lib/cli/exit-handler.js
+++ b/test/lib/cli/exit-handler.js
@@ -276,7 +276,7 @@ t.test('merges output buffers errors with --json', async (t) => {
   })
 
   output.buffer({ output_data: 1 })
-  output.buffer(JSON.stringify({ more_data: 2 }))
+  output.buffer({ more_data: 2 })
   output.buffer('not json, will be ignored')
 
   await exitHandler()
diff --git a/test/lib/commands/pkg.js b/test/lib/commands/pkg.js
index 168870f59d9d1..2941faf90c584 100644
--- a/test/lib/commands/pkg.js
+++ b/test/lib/commands/pkg.js
@@ -7,7 +7,10 @@ const { cleanCwd } = require('../../fixtures/clean-snapshot')
 t.cleanSnapshot = (str) => cleanCwd(str)
 
 const mockNpm = async (t, { ...opts } = {}) => {
-  const res = await _mockNpm(t, opts)
+  const res = await _mockNpm(t, {
+    ...opts,
+    command: 'pkg',
+  })
 
   const readPackageJson = (dir = '') =>
     JSON.parse(readFileSync(resolve(res.prefix, dir, 'package.json'), 'utf8'))
@@ -575,7 +578,7 @@ t.test('delete nested field', async t => {
 })
 
 t.test('workspaces', async t => {
-  const { pkg, OUTPUT, readPackageJson } = await mockNpm(t, {
+  const mockWorkspaces = (t) => mockNpm(t, {
     prefixDir: {
       'package.json': JSON.stringify({
         name: 'root',
@@ -602,68 +605,74 @@ t.test('workspaces', async t => {
     config: { workspaces: true },
   })
 
-  await pkg('get', 'name', 'version')
+  t.test('get', async t => {
+    const { pkg, OUTPUT } = await mockWorkspaces(t)
+    await pkg('get', 'name', 'version')
+    t.strictSame(
+      JSON.parse(OUTPUT()),
+      {
+        a: {
+          name: 'a',
+          version: '1.0.0',
+        },
+        b: {
+          name: 'b',
+          version: '1.2.3',
+        },
+      },
+      'should return expected result for configured workspaces'
+    )
+  })
 
-  t.strictSame(
-    JSON.parse(OUTPUT()),
-    {
-      a: {
+  t.test('set', async t => {
+    const { pkg, readPackageJson } = await mockWorkspaces(t)
+
+    await pkg('set', 'funding=http://example.com')
+
+    t.strictSame(
+      readPackageJson('packages/a'),
+      {
         name: 'a',
         version: '1.0.0',
+        funding: 'http://example.com',
       },
-      b: {
+      'should add field to workspace a'
+    )
+
+    t.strictSame(
+      readPackageJson('packages/b'),
+      {
         name: 'b',
         version: '1.2.3',
+        funding: 'http://example.com',
       },
-    },
-    'should return expected result for configured workspaces'
-  )
+      'should add field to workspace b'
+    )
 
-  await pkg('set', 'funding=http://example.com')
+    await pkg('delete', 'version')
 
-  t.strictSame(
-    readPackageJson('packages/a'),
-    {
-      name: 'a',
-      version: '1.0.0',
-      funding: 'http://example.com',
-    },
-    'should add field to workspace a'
-  )
-
-  t.strictSame(
-    readPackageJson('packages/b'),
-    {
-      name: 'b',
-      version: '1.2.3',
-      funding: 'http://example.com',
-    },
-    'should add field to workspace b'
-  )
-
-  await pkg('delete', 'version')
-
-  t.strictSame(
-    readPackageJson('packages/a'),
-    {
-      name: 'a',
-      funding: 'http://example.com',
-    },
-    'should delete version field from workspace a'
-  )
+    t.strictSame(
+      readPackageJson('packages/a'),
+      {
+        name: 'a',
+        funding: 'http://example.com',
+      },
+      'should delete version field from workspace a'
+    )
 
-  t.strictSame(
-    readPackageJson('packages/b'),
-    {
-      name: 'b',
-      funding: 'http://example.com',
-    },
-    'should delete version field from workspace b'
-  )
+    t.strictSame(
+      readPackageJson('packages/b'),
+      {
+        name: 'b',
+        funding: 'http://example.com',
+      },
+      'should delete version field from workspace b'
+    )
+  })
 })
 
 t.test('single workspace', async t => {
-  const { pkg, OUTPUT } = await mockNpm(t, {
+  const mockWorkspace = (t) => mockNpm(t, {
     prefixDir: {
       'package.json': JSON.stringify({
         name: 'root',
@@ -690,13 +699,27 @@ t.test('single workspace', async t => {
     config: { workspace: ['packages/a'] },
   })
 
-  await pkg('get', 'name', 'version')
+  t.test('multiple args', async t => {
+    const { pkg, OUTPUT } = await mockWorkspace(t)
+    await pkg('get', 'name', 'version')
 
-  t.strictSame(
-    JSON.parse(OUTPUT()),
-    { a: { name: 'a', version: '1.0.0' } },
-    'should only return info for one workspace'
-  )
+    t.strictSame(
+      JSON.parse(OUTPUT()),
+      { a: { name: 'a', version: '1.0.0' } },
+      'should only return info for one workspace'
+    )
+  })
+
+  t.test('single arg', async t => {
+    const { pkg, OUTPUT } = await mockWorkspace(t)
+    await pkg('get', 'version')
+
+    t.strictSame(
+      JSON.parse(OUTPUT()),
+      { a: '1.0.0' },
+      'should only return info for one workspace'
+    )
+  })
 })
 
 t.test('fix', async t => {