fix output for incorrect dedupe

Author: milaninfy

lib/commands/ls.js

@@ -138,7 +138,12 @@ class LS extends ArboristWorkspaceCmd {
       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
@@ -157,13 +162,11 @@ class LS extends ArboristWorkspaceCmd {
       ? filterDefaultDepth
       : (depth || 0)
 
-    // add root node of tree to list of seenNodes
-
     const seenNodes = new Map()
     const problems = new Set()
     const cache = new Map()
-    const traversePathMap = new Map()
 
+    // add root node of tree to list of seenNodes
     seenNodes.set(tree.path, tree)
 
     const result = exploreDependencyGraph(
@@ -173,8 +176,7 @@ class LS extends ArboristWorkspaceCmd {
       { json, parseable },
       seenNodes,
       problems,
-      cache,
-      traversePathMap
+      cache
     )
 
     // handle the special case of a broken package.json in the root folder
@@ -237,35 +239,36 @@ const exploreDependencyGraph = (
   seenNodes,
   problems,
   cache,
-  traversePathMap,
-  encounterCount = new Map()
+  traversePathMap = 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 (node.path && cache.has(node.path)) {
+  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)
 
-  if (count > 1) {
+  // we want to start using cache after node is identified as a deduped
+  if (currentNodeResult[_dedupe]) {
     cache.set(node.path, currentNodeResult)
   }
 
   // Get children of current node
   const children = isCircular
     ? []
-    : getChildren(node, currentNodeResult, seenNodes, traversePathMap)
+    : getChildren(node, currentNodeResult, seenNodes)
 
-  // Recurse on each child
+  // Recurse on each child node
   for (const child of children) {
     const childResult = exploreDependencyGraph(
       child,
@@ -275,10 +278,11 @@ const exploreDependencyGraph = (
       seenNodes,
       problems,
       cache,
-      traversePathMap,
-      encounterCount
+      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 || {}
@@ -386,7 +390,7 @@ 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 ({ label, nodes: [], [_include]: node[_include], [_parent]: node[_parent] })
 }
@@ -560,16 +564,6 @@ const augmentNodesWithMetadata = ({
 const sortAlphabetically = ({ pkgid: a }, { pkgid: b }) => localeCompare(a, b)
 
 const humanOutput = ({ chalk, result, 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)
-  //   }
-  // }
-
   if (!result.nodes.length) {
     result.nodes = ['(empty)']
   }
@@ -591,11 +585,6 @@ const jsonOutput = ({ path, problems, result, rootError }) => {
     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
-
   return result
 }
 

tap-snapshots/test/lib/commands/ls.js.test.cjs

@@ -600,7 +600,7 @@ exports[`test/lib/commands/ls.js TAP ls with args and dedupe entries > should pr
 | \`-- @npmcli/b@1.1.2 deduped
 +-- @npmcli/b@1.1.2
 \`-- @npmcli/c@1.0.0
-[0m  \`-- [33m@npmcli/b@1.1.2[39m[0m
+[0m  \`-- [33m@npmcli/b@1.1.2[39m [2mdeduped[22m[0m
 
 `
 
@@ -610,7 +610,7 @@ dedupe-entries@1.0.0 {CWD}/prefix
 | \`-- @npmcli/c@1.0.0 deduped
 +-- @npmcli/b@1.1.2
 | \`-- @npmcli/c@1.0.0 deduped
-\`-- @npmcli/c@1.0.0 deduped
+\`-- @npmcli/c@1.0.0
 `
 
 exports[`test/lib/commands/ls.js TAP ls with dot filter arg > should output tree contaning only occurrences of filtered by package and colored output 1`] = `
@@ -648,7 +648,7 @@ dedupe-entries@1.0.0 {CWD}/prefix
 | \`-- @npmcli/b@1.1.2 deduped
 +-- @npmcli/b@1.1.2
 \`-- @npmcli/c@1.0.0
-  \`-- @npmcli/b@1.1.2
+  \`-- @npmcli/b@1.1.2 deduped
 `
 
 exports[`test/lib/commands/ls.js TAP ls with no args dedupe entries and not displaying all > should print tree output containing deduped ref 1`] = `
@@ -672,6 +672,8 @@ test-npm-ls@1.0.0 {CWD}/prefix
 +-- chai@1.0.0 extraneous
 | \`-- dog@1.0.0 deduped invalid: "^1.2.3" from the root project, "^2.0.0" from node_modules/cat, "2.x" from node_modules/chai
 |   \`-- cat@1.0.0 deduped invalid: "^2.0.0" from the root project
-\`-- dog@1.0.0 deduped invalid: "^1.2.3" from the root project, "^2.0.0" from node_modules/cat, "2.x" from node_modules/chai
+|     \`-- dog@1.0.0 deduped invalid: "^1.2.3" from the root project, "^2.0.0" from node_modules/cat, "2.x" from node_modules/chai, "^2.0.0" from node_modules/cat
+\`-- dog@1.0.0 invalid: "^1.2.3" from the root project, "^2.0.0" from node_modules/cat, "2.x" from node_modules/chai, "^2.0.0" from node_modules/cat
   \`-- cat@1.0.0 deduped invalid: "^2.0.0" from the root project
+    \`-- dog@1.0.0 deduped invalid: "^1.2.3" from the root project, "^2.0.0" from node_modules/cat, "2.x" from node_modules/chai, "^2.0.0" from node_modules/cat, "^2.0.0" from node_modules/cat
 `