fix: ls to display all deduped nodes

Author: Milan Meva

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')
@@ -62,6 +61,12 @@ class LS extends ArboristWorkspaceCmd {
 
     const path = global ? resolve(this.npm.globalDir, '..') : this.npm.prefix
 
+    // 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 Arborist = require('@npmcli/arborist')
 
     const arb = new Arborist({
@@ -105,34 +110,23 @@ class LS extends ArboristWorkspaceCmd {
       return true
     }
 
-    const seenItems = new Set()
     const seenNodes = new Map()
     const problems = new Set()
 
-    // defines special handling of printed depth when filtering with args
-    const filterDefaultDepth = depth === null ? Infinity : depth
-    const depthToPrint = (all || args.length)
-      ? filterDefaultDepth
-      : (depth || 0)
-
-    // 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({
-      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`
+    const result = exploreDependencyGraph({
+      node: tree,
       getChildren (node, nodeResult) {
         const seenPaths = new Set()
         const workspace = node.isWorkspace
         const currentDepth = workspace ? 0 : node[_depth]
+        const target = (node.target)?.edgesOut
+
         const shouldSkipChildren =
-          !(node instanceof Arborist.Node) || (currentDepth > depthToPrint)
-        return (shouldSkipChildren)
+       (currentDepth > depthToPrint) || !nodeResult
+
+        return (shouldSkipChildren || !target)
           ? []
-          : [...(node.target).edgesOut.values()]
+          : [...target.values()]
             .filter(filterBySelectedWorkspaces)
             .filter(currentDepth === 0 ? filterByEdgesTypes({
               link,
@@ -148,15 +142,25 @@ class LS extends ArboristWorkspaceCmd {
               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) {
+        // add to seenNodes as soon as we visit and not when the children are calculated in previous call
+        if (!seenNodes.has(node.path)) {
+          seenNodes.set(node.path, node)
+        } else {
+          node[_dedupe] = !node[_missing]
+        }
+
         node[_problems] = getProblems(node, { global })
 
         const item = json
           ? getJsonOutputItem(node, { global, long })
           : parseable
-            ? null
+            ? {
+              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
@@ -165,24 +169,22 @@ class LS extends ArboristWorkspaceCmd {
             problems.add(problem)
           }
         }
-
-        seenItems.add(item)
-
-        // return a promise so we don't blow the stack
-        return Promise.resolve(item)
+        return item
       },
+      opts: { json, parseable },
+      seenNodes,
     })
 
     // 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 +227,70 @@ class LS extends ArboristWorkspaceCmd {
 
 module.exports = LS
 
+const exploreDependencyGraph = ({
+  node,
+  getChildren,
+  visit,
+  opts,
+  seenNodes,
+  cache = new Map(),
+  traversePathMap = new Map(),
+}) => {
+  const { json, parseable } = opts
+
+  // cahce is for already visited nodes results
+  // if the node is already seen, we can return it from cache
+  if (cache.has(node.path)) {
+    return cache.get(node.path)
+  }
+
+  const currentNodeResult = visit(node)
+
+  // 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 (node[_dedupe]) {
+    cache.set(node.path, currentNodeResult)
+  }
+
+  // Get children of current node
+  const children = isCircular
+    ? []
+    : getChildren(node, currentNodeResult)
+
+  // Recurse on each child node
+  for (const child of children) {
+    const childResult = exploreDependencyGraph({
+      node: child,
+      getChildren,
+      visit,
+      opts,
+      seenNodes,
+      cache,
+      traversePathMap,
+    })
+    // 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 +328,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 +385,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 +448,7 @@ const getJsonOutputItem = (node, { global, long }) => {
     item.problems = [...node[_problems]]
   }
 
-  return augmentItemWithIncludeMetadata(node, item)
+  return item
 }
 
 const filterByEdgesTypes = ({ link, omit }) => (edge) => {
@@ -467,27 +513,24 @@ const augmentNodesWithMetadata = ({
   // revisit that node in tree traversal logic, so we make it so that
   // we have a diff obj for deduped nodes:
   if (seenNodes.has(node.path)) {
-    const { realpath, root } = node
-    const targetLocation = root ? relative(root.realpath, realpath)
-      : node.targetLocation
     node = {
       name: node.name,
       version: node.version,
       pkgid: node.pkgid,
+      target: node.target,
+      edgesOut: node.edgesOut,
+      children: node.children,
       package: node.package,
       path: node.path,
       isLink: node.isLink,
       realpath: node.realpath,
-      targetLocation,
+      targetLocation: node.targetLocation,
       [_type]: node[_type],
       [_invalid]: node[_invalid],
       [_missing]: node[_missing],
       // if it's missing, it's not deduped, it's just missing
       [_dedupe]: !node[_missing],
     }
-  } else {
-    // keeps track of already seen nodes in order to check for dedupes
-    seenNodes.set(node.path, node)
   }
 
   // _parent is going to be a ref to a treeverse-visited node (returned from
@@ -499,7 +542,11 @@ const augmentNodesWithMetadata = ({
   // _filteredBy is used to apply extra color info to the item that
   // was used in args in order to filter
   node[_filteredBy] = node[_include] =
-    filterByPositionalArgs(args, { node: seenNodes.get(node.path) })
+    filterByPositionalArgs(args, {
+      node: seenNodes.has(node.path)
+        ? seenNodes.get(node.path)
+        : node,
+    })
   // _depth keeps track of how many levels deep tree traversal currently is
   // so that we can `npm ls --depth=1`
   node[_depth] = currentDepth + 1
@@ -509,17 +556,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 +565,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 +578,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
 }