From dd5921938b6c7938d149b5f903482271d37c2c04 Mon Sep 17 00:00:00 2001
From: Luke Karrys <luke@lukekarrys.com>
Date: Sun, 12 May 2024 13:03:39 -0700
Subject: [PATCH] fix(view): refactor to output from single method

---
 lib/commands/view.js   | 262 +++++++++++++++++++----------------------
 lib/utils/queryable.js |   6 +-
 2 files changed, 128 insertions(+), 140 deletions(-)

diff --git a/lib/commands/view.js b/lib/commands/view.js
index 155288c96ea85..29cfd1da24fda 100644
--- a/lib/commands/view.js
+++ b/lib/commands/view.js
@@ -77,11 +77,7 @@ class View extends BaseCommand {
   }
 
   async exec (args) {
-    if (!args.length) {
-      args = ['.']
-    }
-    let pkg = args.shift()
-    const local = /^\.@/.test(pkg) || pkg === '.'
+    let { pkg, local, rest } = parseArgs(args)
 
     if (local) {
       if (this.npm.global) {
@@ -96,93 +92,70 @@ class View extends BaseCommand {
       pkg = `${manifest.name}${pkg.slice(1)}`
     }
 
-    let wholePackument = false
-    if (!args.length) {
-      args = ['']
-      wholePackument = true
+    await this.#viewPackage(pkg, rest)
+  }
+
+  async execWorkspaces (args) {
+    const { pkg, local, rest } = parseArgs(args)
+
+    if (!local) {
+      log.warn('Ignoring workspaces for specified package(s)')
+      return this.exec([pkg, ...rest])
     }
-    const [pckmnt, data] = await this.getData(pkg, args)
 
-    if (!this.npm.config.get('json') && wholePackument) {
-      // pretty view (entire packument)
-      data.map((v) => this.prettyView(pckmnt, v[Object.keys(v)[0]]['']))
-    } else {
-      // JSON formatted output (JSON or specific attributes from packument)
-      let reducedData = data.reduce(reducer, {})
-      if (wholePackument) {
-        // No attributes
-        reducedData = cleanBlanks(reducedData)
-        log.silly('view', reducedData)
-      }
+    await this.setWorkspaces()
 
-      const msg = await this.jsonData(reducedData, pckmnt._id)
-      if (msg !== '') {
-        output.standard(msg)
-      }
+    for (const name of this.workspaceNames) {
+      await this.#viewPackage(`${name}${pkg.slice(1)}`, rest, { workspace: true })
     }
   }
 
-  async execWorkspaces (args) {
-    if (!args.length) {
-      args = ['.']
+  async #viewPackage (name, args, { workspace = false } = {}) {
+    const wholePackument = !args.length
+    const json = this.npm.config.get('json')
+
+    // If we are viewing many packages and outputting individual fields then
+    // output the name before doing any async activity
+    if (!json && !wholePackument && workspace) {
+      output.standard(`${name}:`)
     }
 
-    const pkg = args.shift()
+    const [pckmnt, data] = await this.#getData(name, args, wholePackument)
 
-    const local = /^\.@/.test(pkg) || pkg === '.'
-    if (!local) {
-      log.warn('Ignoring workspaces for specified package(s)')
-      return this.exec([pkg, ...args])
+    if (!json && wholePackument) {
+      // pretty view (entire packument)
+      for (const v of data) {
+        output.standard(this.#prettyView(pckmnt, Object.values(v)[0][Queryable.ALL]))
+      }
+      return
     }
-    let wholePackument = false
-    if (!args.length) {
-      wholePackument = true
-      args = [''] // getData relies on this
+
+    // JSON formatted output (JSON or specific attributes from packument)
+    let reducedData = data.reduce(reducer, {})
+    if (wholePackument) {
+      // No attributes
+      reducedData = cleanBlanks(reducedData)
+      log.silly('view', reducedData)
     }
-    const results = {}
-    await this.setWorkspaces()
-    for (const name of this.workspaceNames) {
-      const wsPkg = `${name}${pkg.slice(1)}`
-      const [pckmnt, data] = await this.getData(wsPkg, args)
-
-      let reducedData = data.reduce(reducer, {})
-      if (wholePackument) {
-        // No attributes
-        reducedData = cleanBlanks(reducedData)
-        log.silly('view', reducedData)
-      }
 
-      if (!this.npm.config.get('json')) {
-        if (wholePackument) {
-          data.map((v) => this.prettyView(pckmnt, v[Object.keys(v)[0]]['']))
-        } else {
-          output.standard(`${name}:`)
-          const msg = await this.jsonData(reducedData, pckmnt._id)
-          if (msg !== '') {
-            output.standard(msg)
-          }
-        }
+    const msg = this.#packageOutput(reducedData, pckmnt._id)
+    if (msg !== '') {
+      if (json && workspace) {
+        output.buffer({ [name]: JSON.parse(msg) })
       } else {
-        const msg = await this.jsonData(reducedData, pckmnt._id)
-        if (msg !== '') {
-          results[name] = JSON.parse(msg)
-        }
+        output.standard(msg)
       }
     }
-    if (Object.keys(results).length > 0) {
-      output.standard(JSON.stringify(results, null, 2))
-    }
   }
 
-  async getData (pkg, args) {
-    const json = this.npm.config.get('json')
-    const opts = {
+  async #getData (pkg, args) {
+    const spec = npa(pkg)
+
+    const pckmnt = await packument(spec, {
       ...this.npm.flatOptions,
       preferOnline: true,
       fullMetadata: true,
-    }
-
-    const spec = npa(pkg)
+    })
 
     // get the data about this package
     let version = this.npm.config.get('tag')
@@ -191,22 +164,19 @@ class View extends BaseCommand {
       version = spec.rawSpec
     }
 
-    const pckmnt = await packument(spec, opts)
-
     if (pckmnt['dist-tags']?.[version]) {
       version = pckmnt['dist-tags'][version]
     }
 
-    if (pckmnt.time && pckmnt.time.unpublished) {
+    if (pckmnt.time?.unpublished) {
       const u = pckmnt.time.unpublished
-      const er = new Error(`Unpublished on ${u.time}`)
-      er.statusCode = 404
-      er.code = 'E404'
-      er.pkgid = pckmnt._id
-      throw er
+      throw Object.assign(new Error(`Unpublished on ${u.time}`), {
+        statusCode: 404,
+        code: 'E404',
+        pkgid: pckmnt._id,
+      })
     }
 
-    const data = []
     const versions = pckmnt.versions || {}
     pckmnt.versions = Object.keys(versions).filter(v => {
       if (semver.valid(v)) {
@@ -221,41 +191,34 @@ class View extends BaseCommand {
       delete pckmnt.readme
     }
 
-    Object.keys(versions).forEach((v) => {
-      if (semver.satisfies(v, version, true)) {
-        args.forEach(arg => {
-          // remove readme unless we asked for it
-          if (args.indexOf('readme') !== -1) {
-            delete versions[v].readme
-          }
-
-          data.push(showFields({
-            data: pckmnt,
-            version: versions[v],
-            fields: arg,
-            json,
-          }))
+    const data = Object.keys(versions)
+      .filter(v => semver.satisfies(v, version, true))
+      .flatMap((v) => {
+        // remove readme unless we asked for it
+        if (args.indexOf('readme') !== -1) {
+          delete versions[v].readme
+        }
+        return showFields({
+          data: pckmnt,
+          version: versions[v],
+          fields: args,
+          json: this.npm.config.get('json'),
         })
-      }
-    })
+      })
 
     // No data has been pushed because no data is matching the specified version
-    if (data.length === 0 && version !== 'latest') {
-      const er = new Error(`No match found for version ${version}`)
-      er.statusCode = 404
-      er.code = 'E404'
-      er.pkgid = `${pckmnt._id}@${version}`
-      throw er
-    }
-
-    if (!json && args.length === 1 && args[0] === '') {
-      pckmnt.version = version
+    if (!data.length && version !== 'latest') {
+      throw Object.assign(new Error(`No match found for version ${version}`), {
+        statusCode: 404,
+        code: 'E404',
+        pkgid: `${pckmnt._id}@${version}`,
+      })
     }
 
     return [pckmnt, data]
   }
 
-  async jsonData (data, name) {
+  #packageOutput (data, name) {
     const versions = Object.keys(data)
     let msg = ''
     let msgJson = []
@@ -315,7 +278,7 @@ class View extends BaseCommand {
     return msg.trim()
   }
 
-  prettyView (packu, manifest) {
+  #prettyView (packu, manifest) {
     // More modern, pretty printing of default view
     const unicode = this.npm.config.get('unicode')
     const chalk = this.npm.chalk
@@ -329,8 +292,10 @@ class View extends BaseCommand {
       ? licenseField
       : (licenseField.type || 'Proprietary')
 
-    output.standard('')
-    output.standard([
+    const res = []
+
+    res.push('')
+    res.push([
       chalk.underline.cyan(`${manifest.name}@${manifest.version}`),
       license.toLowerCase().trim() === 'proprietary'
         ? chalk.red(license)
@@ -339,55 +304,55 @@ class View extends BaseCommand {
       `versions: ${chalk.cyan(packu.versions.length + '')}`,
     ].join(' | '))
 
-    manifest.description && output.standard(manifest.description)
+    manifest.description && res.push(manifest.description)
     if (site) {
-      output.standard(chalk.blue(site))
+      res.push(chalk.blue(site))
     }
 
-    manifest.deprecated && output.standard(
+    manifest.deprecated && res.push(
       `\n${chalk.redBright('DEPRECATED')}${unicode ? ' ⚠️ ' : '!!'} - ${manifest.deprecated}`
     )
 
     if (packu.keywords?.length) {
-      output.standard(`\nkeywords: ${
+      res.push(`\nkeywords: ${
         packu.keywords.map(k => chalk.cyan(k)).join(', ')
       }`)
     }
 
     if (bins.length) {
-      output.standard(`\nbin: ${chalk.cyan(bins.join(', '))}`)
+      res.push(`\nbin: ${chalk.cyan(bins.join(', '))}`)
     }
 
-    output.standard('\ndist')
-    output.standard(`.tarball: ${chalk.blue(manifest.dist.tarball)}`)
-    output.standard(`.shasum: ${chalk.green(manifest.dist.shasum)}`)
+    res.push('\ndist')
+    res.push(`.tarball: ${chalk.blue(manifest.dist.tarball)}`)
+    res.push(`.shasum: ${chalk.green(manifest.dist.shasum)}`)
     if (manifest.dist.integrity) {
-      output.standard(`.integrity: ${chalk.green(manifest.dist.integrity)}`)
+      res.push(`.integrity: ${chalk.green(manifest.dist.integrity)}`)
     }
     if (manifest.dist.unpackedSize) {
-      output.standard(`.unpackedSize: ${chalk.blue(formatBytes(manifest.dist.unpackedSize, true))}`)
+      res.push(`.unpackedSize: ${chalk.blue(formatBytes(manifest.dist.unpackedSize, true))}`)
     }
 
     if (deps.length) {
       const maxDeps = 24
-      output.standard('\ndependencies:')
-      output.standard(columns(deps.slice(0, maxDeps), { padding: 1 }))
+      res.push('\ndependencies:')
+      res.push(columns(deps.slice(0, maxDeps), { padding: 1 }))
       if (deps.length > maxDeps) {
-        output.standard(chalk.dim(`(...and ${deps.length - maxDeps} more.)`))
+        res.push(chalk.dim(`(...and ${deps.length - maxDeps} more.)`))
       }
     }
 
     if (packu.maintainers?.length) {
-      output.standard('\nmaintainers:')
+      res.push('\nmaintainers:')
       packu.maintainers.forEach(u =>
-        output.standard(`- ${unparsePerson({
+        res.push(`- ${unparsePerson({
           name: chalk.blue(u.name),
           email: chalk.dim(u.email) })}`)
       )
     }
 
-    output.standard('\ndist-tags:')
-    output.standard(columns(Object.keys(packu['dist-tags']).map(t =>
+    res.push('\ndist-tags:')
+    res.push(columns(Object.keys(packu['dist-tags']).map(t =>
       `${chalk.blue(t)}: ${packu['dist-tags'][t]}`
     )))
 
@@ -403,14 +368,30 @@ class View extends BaseCommand {
       if (publisher) {
         publishInfo += ` by ${publisher}`
       }
-      output.standard('')
-      output.standard(publishInfo)
+      res.push('')
+      res.push(publishInfo)
     }
+
+    return res.join('\n')
   }
 }
 
 module.exports = View
 
+function parseArgs (args) {
+  if (!args.length) {
+    args = ['.']
+  }
+
+  const pkg = args.shift()
+
+  return {
+    pkg,
+    local: /^\.@/.test(pkg) || pkg === '.',
+    rest: args,
+  }
+}
+
 function cleanBlanks (obj) {
   const clean = {}
   Object.keys(obj).forEach((version) => {
@@ -422,10 +403,10 @@ function cleanBlanks (obj) {
 // takes an array of objects and merges them into one object
 function reducer (acc, cur) {
   if (cur) {
-    Object.keys(cur).forEach((v) => {
-      acc[v] = acc[v] || {}
-      Object.keys(cur[v]).forEach((t) => {
-        acc[v][t] = cur[v][t]
+    Object.entries(cur).forEach(([k, v]) => {
+      acc[k] ||= {}
+      Object.keys(v).forEach((kk) => {
+        acc[k][kk] = cur[k][kk]
       })
     })
   }
@@ -443,12 +424,17 @@ function showFields ({ data, version, fields, json }) {
   })
 
   const queryable = new Queryable(o)
-  const s = queryable.query(fields, { unwrapSingleItemArrays: !json })
-  const res = { [version.version]: s }
 
-  if (s) {
-    return res
+  if (!fields.length) {
+    return { [version.version]: queryable.query(Queryable.ALL) }
   }
+
+  return fields.map((field) => {
+    const s = queryable.query(field, { unwrapSingleItemArrays: !json })
+    if (s) {
+      return { [version.version]: s }
+    }
+  })
 }
 
 function cleanup (data) {
diff --git a/lib/utils/queryable.js b/lib/utils/queryable.js
index 372cde91e1ce0..4fc1e3533eabc 100644
--- a/lib/utils/queryable.js
+++ b/lib/utils/queryable.js
@@ -231,6 +231,8 @@ const setter = ({ data, key, value, force }) => {
 }
 
 class Queryable {
+  static ALL = ''
+
   #data = null
 
   constructor (obj) {
@@ -247,8 +249,8 @@ class Queryable {
     // this ugly interface here is meant to be a compatibility layer
     // with the legacy API lib/view.js is consuming, if at some point
     // we refactor that command then we can revisit making this nicer
-    if (queries === '') {
-      return { '': this.#data }
+    if (queries === Queryable.ALL) {
+      return { [Queryable.ALL]: this.#data }
     }
 
     const q = query =>