fix(ls): correctly show deduped dependencies

lib/commands/ls.js

@@ -1,6 +1,5 @@
 const { resolve, relative, sep } = require('node:path')
 const archy = require('archy')
-const { breadth } = require('treeverse')
 const npa = require('npm-package-arg')
 const { output } = require('proc-log')
 const ArboristWorkspaceCmd = require('../arborist-cmd.js')
@@ -105,84 +104,91 @@ class LS extends ArboristWorkspaceCmd {
       return true
     }
 
-    const seenItems = new Set()
-    const seenNodes = new Map()
-    const problems = new Set()
+    const getChildren = (node, nodeResult, seenNodes) => {
+      const seenPaths = new Set()
+      const workspace = node.isWorkspace
+      const currentDepth = workspace ? 0 : node[_depth]
+      const target = (node.target)?.edgesOut
+
+      const shouldSkipChildren =
+       (currentDepth > depthToPrint) || !nodeResult
+
+      return (shouldSkipChildren || !target)
+        ? []
+        : [...target.values()]
+          .filter(filterBySelectedWorkspaces)
+          .filter(currentDepth === 0 ? filterByEdgesTypes({
+            link,
+            omit,
+          }) : () => true)
+          .map(mapEdgesToNodes({ seenPaths }))
+          .concat(appendExtraneousChildren({ node, seenPaths }))
+          .sort(sortAlphabetically)
+          .map(augmentNodesWithMetadata({
+            args,
+            currentDepth,
+            nodeResult,
+            seenNodes,
+          }))
+    }
+
+    const visit = (node, problems) => {
+      node[_problems] = getProblems(node, { global })
+
+      const item = json
+        ? getJsonOutputItem(node, { global, long })
+        : parseable
+          ? {
+            pkgid: node.pkgid,
+            path: node.path,
+            [_dedupe]: node[_dedupe],
+            [_parent]: node[_parent],
+          }
+          : getHumanOutputItem(node, { args, chalk, global, long })
+
+      // loop through list of node problems to add them to global list
+      if (node[_include]) {
+        for (const problem of node[_problems]) {
+          problems.add(problem)
+        }
+      }
 
+      // return a promise so we don't blow the stack
+      return item
+    }
     // defines special handling of printed depth when filtering with args
     const filterDefaultDepth = depth === null ? Infinity : depth
     const depthToPrint = (all || args.length)
       ? filterDefaultDepth
       : (depth || 0)
 
+    const seenNodes = new Map()
+    const problems = new Set()
+    const cache = new Map()
+
     // add root node of tree to list of seenNodes
     seenNodes.set(tree.path, tree)
 
-    // tree traversal happens here, using treeverse.breadth
-    const result = await breadth({
+    const result = exploreDependencyGraph(
       tree,
-      // recursive method, `node` is going to be the current elem (starting from
-      // the `tree` obj) that was just visited in the `visit` method below
-      // `nodeResult` is going to be the returned `item` from `visit`
-      getChildren (node, nodeResult) {
-        const seenPaths = new Set()
-        const workspace = node.isWorkspace
-        const currentDepth = workspace ? 0 : node[_depth]
-        const shouldSkipChildren =
-          !(node instanceof Arborist.Node) || (currentDepth > depthToPrint)
-        return (shouldSkipChildren)
-          ? []
-          : [...(node.target).edgesOut.values()]
-            .filter(filterBySelectedWorkspaces)
-            .filter(currentDepth === 0 ? filterByEdgesTypes({
-              link,
-              omit,
-            }) : () => true)
-            .map(mapEdgesToNodes({ seenPaths }))
-            .concat(appendExtraneousChildren({ node, seenPaths }))
-            .sort(sortAlphabetically)
-            .map(augmentNodesWithMetadata({
-              args,
-              currentDepth,
-              nodeResult,
-              seenNodes,
-            }))
-      },
-      // visit each `node` of the `tree`, returning an `item` - these are
-      // the elements that will be used to build the final output
-      visit (node) {
-        node[_problems] = getProblems(node, { global })
-
-        const item = json
-          ? getJsonOutputItem(node, { global, long })
-          : parseable
-            ? null
-            : getHumanOutputItem(node, { args, chalk, global, long })
-
-        // loop through list of node problems to add them to global list
-        if (node[_include]) {
-          for (const problem of node[_problems]) {
-            problems.add(problem)
-          }
-        }
-
-        seenItems.add(item)
-
-        // return a promise so we don't blow the stack
-        return Promise.resolve(item)
-      },
-    })
+      getChildren,
+      visit,
+      { json, parseable },
+      seenNodes,
+      problems,
+      cache
+    )
 
     // handle the special case of a broken package.json in the root folder
     const [rootError] = tree.errors.filter(e =>
       e.code === 'EJSONPARSE' && e.path === resolve(path, 'package.json'))
 
     if (json) {
-      output.buffer(jsonOutput({ path, problems, result, rootError, seenItems }))
+      output.buffer(jsonOutput({ path, problems, result, rootError }))
     } else {
       output.standard(parseable
         ? parseableOutput({ seenNodes, global, long })
-        : humanOutput({ chalk, result, seenItems, unicode })
+        : humanOutput({ chalk, result, unicode })
       )
     }
 
@@ -225,6 +231,76 @@ class LS extends ArboristWorkspaceCmd {
 
 module.exports = LS
 
+const exploreDependencyGraph = (
+  node,
+  getChildren,
+  visit,
+  { json, parseable },
+  seenNodes,
+  problems,
+  cache,
+  traversePathMap = new Map(),
+  encounterCount = new Map()
+) => {
+  // Track the number of encounters for the current node
+  // Why because we want to start storing/caching after the node is identified as a deduped edge
+
+  const count = node.path ? (encounterCount.get(node.path) || 0) + 1 : 0
+  node.path && encounterCount.set(node.path, count)
+
+  if (cache.has(node.path)) {
+    return cache.get(node.path)
+  }
+
+  const currentNodeResult = visit(node, problems)
+
+  // how the this node is explored
+  // so if the explored path contains this node again then it's a cycle
+  // and we don't want to explore it again
+  const traversePath = [...(traversePathMap.get(currentNodeResult[_parent]) || [])]
+  const isCircular = traversePath?.includes(node.pkgid)
+  traversePath.push(node.pkgid)
+  traversePathMap.set(currentNodeResult, traversePath)
+
+  // we want to start using cache after node is identified as a deduped
+  if (count > 1 && node.path && node[_dedupe]) {
+    cache.set(node.path, currentNodeResult)
+  }
+
+  // Get children of current node
+  const children = isCircular
+    ? []
+    : getChildren(node, currentNodeResult, seenNodes)
+
+  // Recurse on each child node
+  for (const child of children) {
+    const childResult = exploreDependencyGraph(
+      child,
+      getChildren,
+      visit,
+      { json, parseable },
+      seenNodes,
+      problems,
+      cache,
+      traversePathMap,
+      encounterCount
+    )
+    // include current node if any of its children are included
+    currentNodeResult[_include] = currentNodeResult[_include] || childResult[_include]
+
+    if (childResult[_include] && !parseable) {
+      if (json) {
+        currentNodeResult.dependencies = currentNodeResult.dependencies || {}
+        currentNodeResult.dependencies[childResult[_name]] = childResult
+      } else {
+        currentNodeResult.nodes.push(childResult)
+      }
+    }
+  }
+
+  return currentNodeResult
+}
+
 const isGitNode = (node) => {
   if (!node.resolved) {
     return
@@ -262,26 +338,6 @@ const getProblems = (node, { global }) => {
   return problems
 }
 
-// annotates _parent and _include metadata into the resulting
-// item obj allowing for filtering out results during output
-const augmentItemWithIncludeMetadata = (node, item) => {
-  item[_parent] = node[_parent]
-  item[_include] = node[_include]
-
-  // append current item to its parent.nodes which is the
-  // structure expected by archy in order to print tree
-  if (node[_include]) {
-    // includes all ancestors of included node
-    let p = node[_parent]
-    while (p) {
-      p[_include] = true
-      p = p[_parent]
-    }
-  }
-
-  return item
-}
-
 const getHumanOutputItem = (node, { args, chalk, global, long }) => {
   const { pkgid, path } = node
   const workspacePkgId = chalk.blueBright(pkgid)
@@ -339,13 +395,13 @@ const getHumanOutputItem = (node, { args, chalk, global, long }) => {
     ) +
     (isGitNode(node) ? ` (${node.resolved})` : '') +
     (node.isLink ? ` -> ${relativePrefix}${targetLocation}` : '') +
-    (long ? `\n${node.package.description || ''}` : '')
+    (long ? `\n${node.package?.description || ''}` : '')
 
-  return augmentItemWithIncludeMetadata(node, { label, nodes: [] })
+  return ({ label, nodes: [], [_include]: node[_include], [_parent]: node[_parent] })
 }
 
 const getJsonOutputItem = (node, { global, long }) => {
-  const item = {}
+  const item = { [_include]: node[_include], [_parent]: node[_parent] }
 
   if (node.version) {
     item.version = node.version
@@ -402,7 +458,7 @@ const getJsonOutputItem = (node, { global, long }) => {
     item.problems = [...node[_problems]]
   }
 
-  return augmentItemWithIncludeMetadata(node, item)
+  return item
 }
 
 const filterByEdgesTypes = ({ link, omit }) => (edge) => {
@@ -474,6 +530,9 @@ const augmentNodesWithMetadata = ({
       name: node.name,
       version: node.version,
       pkgid: node.pkgid,
+      target: node.target || node,
+      edgesOut: node.edgesOut,
+      children: node.children,
       package: node.package,
       path: node.path,
       isLink: node.isLink,
@@ -509,17 +568,7 @@ const augmentNodesWithMetadata = ({
 
 const sortAlphabetically = ({ pkgid: a }, { pkgid: b }) => localeCompare(a, b)
 
-const humanOutput = ({ chalk, result, seenItems, unicode }) => {
-  // we need to traverse the entire tree in order to determine which items
-  // should be included (since a nested transitive included dep will make it
-  // so that all its ancestors should be displayed)
-  // here is where we put items in their expected place for archy output
-  for (const item of seenItems) {
-    if (item[_include] && item[_parent]) {
-      item[_parent].nodes.push(item)
-    }
-  }
-
+const humanOutput = ({ chalk, result, unicode }) => {
   if (!result.nodes.length) {
     result.nodes = ['(empty)']
   }
@@ -528,7 +577,7 @@ const humanOutput = ({ chalk, result, seenItems, unicode }) => {
   return chalk.reset(archyOutput)
 }
 
-const jsonOutput = ({ path, problems, result, rootError, seenItems }) => {
+const jsonOutput = ({ path, problems, result, rootError }) => {
   if (problems.size) {
     result.problems = [...problems]
   }
@@ -541,22 +590,6 @@ const jsonOutput = ({ path, problems, result, rootError, seenItems }) => {
     result.invalid = true
   }
 
-  // we need to traverse the entire tree in order to determine which items
-  // should be included (since a nested transitive included dep will make it
-  // so that all its ancestors should be displayed)
-  // here is where we put items in their expected place for json output
-  for (const item of seenItems) {
-    // append current item to its parent item.dependencies obj in order
-    // to provide a json object structure that represents the installed tree
-    if (item[_include] && item[_parent]) {
-      if (!item[_parent].dependencies) {
-        item[_parent].dependencies = {}
-      }
-
-      item[_parent].dependencies[item[_name]] = item
-    }
-  }
-
   return result
 }