From fa468c18b2021bb8e4e5ea7fbf097b6428e978be Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 23 Apr 2021 17:49:51 -0700 Subject: [PATCH] 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 --- lib/ls.js | 4 +- package-lock.json | 14 ++-- package.json | 2 +- tap-snapshots/test/lib/ls.js.test.cjs | 42 ++++++++++- test/lib/ls.js | 100 ++++++++++++++++++++++++++ 5 files changed, 152 insertions(+), 10 deletions(-) diff --git a/lib/ls.js b/lib/ls.js index a6f3fbcd9e96d..ccd8b2ff9dea7 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 a54833e4aa4af..2f9e960c91dee 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 766856daf7265..0f5ff225819d4 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 77226971e3310..3e4137aa82e51 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 2918fd4820d1d..6eeaf0ad671bc 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