ls: do not warn on missing optional deps

There was code checking node[_type], but we didn't include that field on
the object that is actually checked when we are looking for problems.

Fix: #3137

PR-URL: https://github.com/npm/cli/pull/3140
Credit: @isaacs
Close: #3140
Reviewed-by: @ruyadorno

5 files changed, 152 insertions, 10 deletions

diff --git a/lib/ls.js b/lib/ls.js
index a6f3fbcd9..ccd8b2ff9 100644
--- a/lib/ls.js
+++ b/lib/ls.js
@@ -414,9 +414,11 @@ const augmentNodesWithMetadata = ({
       path: node.path,
       isLink: node.isLink,
       realpath: node.realpath,
+      [_type]: node[_type],
       [_invalid]: node[_invalid],
       [_missing]: node[_missing],
-      [_dedupe]: true,
+      // 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
diff --git a/package-lock.json b/package-lock.json
index a54833e4a..2f9e960c9 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -161,7 +161,7 @@
         "jsdom": "^16.5.2",
         "licensee": "^8.1.0",
         "marked-man": "^0.7.0",
-        "tap": "^15.0.2",
+        "tap": "^15.0.5",
         "yaml": "^1.10.2"
       },
       "engines": {
@@ -7360,9 +7360,9 @@
       }
     },
     "node_modules/tap": {
-      "version": "15.0.4",
-      "resolved": "https://registry.npmjs.org/tap/-/tap-15.0.4.tgz",
-      "integrity": "sha512-OCOm+CaXU1OleruihHk/9pX3xFYFfwQIEo+mbSsMZTIqG2b8qMrzJ/g3+3cEWx63/2u7SYJQ5/TwmYfFLElHbQ==",
+      "version": "15.0.5",
+      "resolved": "https://registry.npmjs.org/tap/-/tap-15.0.5.tgz",
+      "integrity": "sha512-9txwiDv7Sj8e4O15bjWtjSsPSJOiBGFbQj8gW0Wc7GXPXe6Zy46q9Eax71Ez7gAwhElnu7f9qgtfiCYzH8U2/w==",
       "bundleDependencies": [
         "ink",
         "treport",
@@ -15745,9 +15745,9 @@
       }
     },
     "tap": {
-      "version": "15.0.4",
-      "resolved": "https://registry.npmjs.org/tap/-/tap-15.0.4.tgz",
-      "integrity": "sha512-OCOm+CaXU1OleruihHk/9pX3xFYFfwQIEo+mbSsMZTIqG2b8qMrzJ/g3+3cEWx63/2u7SYJQ5/TwmYfFLElHbQ==",
+      "version": "15.0.5",
+      "resolved": "https://registry.npmjs.org/tap/-/tap-15.0.5.tgz",
+      "integrity": "sha512-9txwiDv7Sj8e4O15bjWtjSsPSJOiBGFbQj8gW0Wc7GXPXe6Zy46q9Eax71Ez7gAwhElnu7f9qgtfiCYzH8U2/w==",
       "dev": true,
       "requires": {
         "@types/react": "^16.9.23",
diff --git a/package.json b/package.json
index 766856daf..0f5ff2258 100644
--- a/package.json
+++ b/package.json
@@ -190,7 +190,7 @@
     "jsdom": "^16.5.2",
     "licensee": "^8.1.0",
     "marked-man": "^0.7.0",
-    "tap": "^15.0.2",
+    "tap": "^15.0.5",
     "yaml": "^1.10.2"
   },
   "scripts": {
diff --git a/tap-snapshots/test/lib/ls.js.test.cjs b/tap-snapshots/test/lib/ls.js.test.cjs
index 77226971e..3e4137aa8 100644
--- a/tap-snapshots/test/lib/ls.js.test.cjs
+++ b/tap-snapshots/test/lib/ls.js.test.cjs
@@ -5,6 +5,46 @@
  * Make sure to inspect the output below.  Do not ignore changes!
  */
 'use strict'
+exports[`test/lib/ls.js TAP ignore missing optional deps --json > ls --json problems 1`] = `
+Array [
+  "invalid: optional-wrong@3.2.1 {project}/node_modules/optional-wrong",
+  "missing: peer-missing@1, required by test-npm-ls-ignore-missing-optional@1.2.3",
+  "invalid: peer-optional-wrong@3.2.1 {project}/node_modules/peer-optional-wrong",
+  "invalid: peer-wrong@3.2.1 {project}/node_modules/peer-wrong",
+  "missing: prod-missing@1, required by test-npm-ls-ignore-missing-optional@1.2.3",
+  "invalid: prod-wrong@3.2.1 {project}/node_modules/prod-wrong",
+]
+`
+
+exports[`test/lib/ls.js TAP ignore missing optional deps --parseable > ls --parseable result 1`] = `
+{project}
+{project}/node_modules/optional-ok
+{project}/node_modules/optional-wrong
+{project}/node_modules/peer-ok
+{project}/node_modules/peer-optional-ok
+{project}/node_modules/peer-optional-wrong
+{project}/node_modules/peer-wrong
+{project}/node_modules/prod-ok
+{project}/node_modules/prod-wrong
+`
+
+exports[`test/lib/ls.js TAP ignore missing optional deps human output > ls result 1`] = `
+test-npm-ls-ignore-missing-optional@1.2.3 {project}
++-- unmet optional dependency optional-missing@1
++-- optional-ok@1.2.3
++-- optional-wrong@3.2.1 invalid
++-- unmet dependency peer-missing@1
++-- peer-ok@1.2.3
++-- unmet optional dependency peer-optional-missing@1
++-- peer-optional-ok@1.2.3
++-- peer-optional-wrong@3.2.1 invalid
++-- peer-wrong@3.2.1 invalid
++-- unmet dependency prod-missing@1
++-- prod-ok@1.2.3
+\`-- prod-wrong@3.2.1 invalid
+
+`
+
 exports[`test/lib/ls.js TAP ls --depth=0 > should output tree containing only top-level dependencies 1`] = `
 test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls---depth-0
 +-- foo@1.0.0
@@ -341,7 +381,7 @@ exports[`test/lib/ls.js TAP ls cycle deps with filter args > should print tree o
 exports[`test/lib/ls.js TAP ls deduped missing dep > should output parseable signaling missing peer dep in problems 1`] = `
 test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls-deduped-missing-dep
 +-- a@1.0.0
-| \`-- UNMET DEPENDENCY b@^1.0.0 deduped
+| \`-- UNMET DEPENDENCY b@^1.0.0
 \`-- UNMET DEPENDENCY b@^1.0.0
 
 `
diff --git a/test/lib/ls.js b/test/lib/ls.js
index 2918fd482..6eeaf0ad6 100644
--- a/test/lib/ls.js
+++ b/test/lib/ls.js
@@ -2352,6 +2352,106 @@ t.test('ls --parseable', (t) => {
   t.end()
 })
 
+t.test('ignore missing optional deps', async t => {
+  t.beforeEach(cleanUpResult)
+  npm.prefix = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'test-npm-ls-ignore-missing-optional',
+      version: '1.2.3',
+      peerDependencies: {
+        'peer-ok': '1',
+        'peer-missing': '1',
+        'peer-wrong': '1',
+        'peer-optional-ok': '1',
+        'peer-optional-missing': '1',
+        'peer-optional-wrong': '1',
+      },
+      peerDependenciesMeta: {
+        'peer-optional-ok': {
+          optional: true,
+        },
+        'peer-optional-missing': {
+          optional: true,
+        },
+        'peer-optional-wrong': {
+          optional: true,
+        },
+      },
+      optionalDependencies: {
+        'optional-ok': '1',
+        'optional-missing': '1',
+        'optional-wrong': '1',
+      },
+      dependencies: {
+        'prod-ok': '1',
+        'prod-missing': '1',
+        'prod-wrong': '1',
+      },
+    }),
+    node_modules: {
+      'prod-ok': {
+        'package.json': JSON.stringify({name: 'prod-ok', version: '1.2.3' }),
+      },
+      'prod-wrong': {
+        'package.json': JSON.stringify({name: 'prod-wrong', version: '3.2.1' }),
+      },
+      'optional-ok': {
+        'package.json': JSON.stringify({name: 'optional-ok', version: '1.2.3' }),
+      },
+      'optional-wrong': {
+        'package.json': JSON.stringify({name: 'optional-wrong', version: '3.2.1' }),
+      },
+      'peer-optional-ok': {
+        'package.json': JSON.stringify({name: 'peer-optional-ok', version: '1.2.3' }),
+      },
+      'peer-optional-wrong': {
+        'package.json': JSON.stringify({name: 'peer-optional-wrong', version: '3.2.1' }),
+      },
+      'peer-ok': {
+        'package.json': JSON.stringify({name: 'peer-ok', version: '1.2.3' }),
+      },
+      'peer-wrong': {
+        'package.json': JSON.stringify({name: 'peer-wrong', version: '3.2.1' }),
+      },
+    },
+  })
+
+  config.all = true
+  const prefix = npm.prefix.toLowerCase().replace(/\\/g, '/')
+  const cleanupPaths = str =>
+    str.toLowerCase().replace(/\\/g, '/').split(prefix).join('{project}')
+
+  t.test('--json', t => {
+    config.json = true
+    config.parseable = false
+    ls.exec([], (err) => {
+      t.match(err, { code: 'ELSPROBLEMS' })
+      result = JSON.parse(result)
+      const problems = result.problems.map(cleanupPaths)
+      t.matchSnapshot(problems, 'ls --json problems')
+      t.end()
+    })
+  })
+  t.test('--parseable', t => {
+    config.json = false
+    config.parseable = true
+    ls.exec([], (err) => {
+      t.match(err, { code: 'ELSPROBLEMS' })
+      t.matchSnapshot(cleanupPaths(result), 'ls --parseable result')
+      t.end()
+    })
+  })
+  t.test('human output', t => {
+    config.json = false
+    config.parseable = false
+    ls.exec([], (err) => {
+      t.match(err, { code: 'ELSPROBLEMS' })
+      t.matchSnapshot(cleanupPaths(result), 'ls result')
+      t.end()
+    })
+  })
+})
+
 t.test('ls --json', (t) => {
   t.beforeEach(cleanUpResult)
   config.json = true