From 81d2994ebe5ceac9973bcdbef01c024bff2d088a Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 23 Jun 2021 10:32:48 -0700 Subject: [PATCH] feat(ls): report *why* something is invalid This is a papercut that has been driving me crazy when debugging ERESOLVE issues. It's not particularly useful to just say something is "invalid", without showing which module's dependency is not being met. With this commit, it prints all the packages that depend on that dependency and do not have their required version met. --- lib/ls.js | 16 ++++-- tap-snapshots/test/lib/ls.js.test.cjs | 31 +++++++---- test/lib/ls.js | 79 +++++++++++++++++++++++++-- 3 files changed, 106 insertions(+), 20 deletions(-) diff --git a/lib/ls.js b/lib/ls.js index c8d84651e2113..b385cb6e9f914 100644 --- a/lib/ls.js +++ b/lib/ls.js @@ -321,8 +321,8 @@ const getHumanOutputItem = (node, { args, color, global, long }) => { : '' ) + ( - node[_invalid] - ? ' ' + (color ? chalk.red.bgBlack('invalid') : 'invalid') + node[_invalid] && !node[_dedupe] + ? ' ' + (color ? chalk.red.bgBlack(node[_invalid]) : node[_invalid]) : '' ) + ( @@ -373,7 +373,7 @@ const getJsonOutputItem = (node, { global, long }) => { item.extraneous = true if (node[_invalid]) - item.invalid = true + item.invalid = node[_invalid] if (node[_missing] && !isOptional(node)) { item.required = node[_required] @@ -432,9 +432,15 @@ const mapEdgesToNodes = ({ seenPaths }) => (edge) => { if (node.path) seenPaths.add(node.path) - node[_required] = edge.spec + node[_required] = edge.spec || '*' node[_type] = edge.type - node[_invalid] = edge.invalid + + if (edge.invalid) { + const spec = JSON.stringify(node[_required]) + const from = edge.from.location || 'the root project' + node[_invalid] = (node[_invalid] ? node[_invalid] + ', ' : '') + + (`${spec} from ${from}`) + } return node } diff --git a/tap-snapshots/test/lib/ls.js.test.cjs b/tap-snapshots/test/lib/ls.js.test.cjs index 3d56d1f432731..35a6cf99cb115 100644 --- a/tap-snapshots/test/lib/ls.js.test.cjs +++ b/tap-snapshots/test/lib/ls.js.test.cjs @@ -32,16 +32,16 @@ exports[`test/lib/ls.js TAP ignore missing optional deps human output > ls resul 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 ++-- optional-wrong@3.2.1 "1" from the root project +-- 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 ++-- peer-optional-wrong@3.2.1 "1" from the root project ++-- peer-wrong@3.2.1 "1" from the root project +-- unmet dependency prod-missing@1 +-- prod-ok@1.2.3 -\`-- prod-wrong@3.2.1 invalid +\`-- prod-wrong@3.2.1 "1" from the root project ` @@ -356,7 +356,7 @@ npm-broken-resolved-field-test@1.0.0 {CWD}/tap-testdir-ls-ls-broken-resolved-fie exports[`test/lib/ls.js TAP ls colored output > should output tree containing color info 1`] = ` test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls-colored-output +-- chai@1.0.0 extraneous -+-- foo@1.0.0 invalid ++-- foo@1.0.0 "^2.0.0" from the root project | \`-- dog@1.0.0 \`-- UNMET DEPENDENCY ipsum@^1.0.0  @@ -454,8 +454,8 @@ exports[`test/lib/ls.js TAP ls global > should print tree and not mark top-level exports[`test/lib/ls.js TAP ls invalid deduped dep > should output tree signaling mismatching peer dep in problems 1`] = ` invalid-deduped-dep@1.0.0 {CWD}/tap-testdir-ls-ls-invalid-deduped-dep +-- a@1.0.0 -| \`-- b@1.0.0 deduped invalid -\`-- b@1.0.0 invalid +| \`-- b@1.0.0 deduped +\`-- b@1.0.0 "^2.0.0" from the root project, "^2.0.0" from node_modules/a  ` @@ -466,7 +466,7 @@ test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls-invalid-peer-dep | \`-- foo@1.0.0 | \`-- dog@1.0.0 +-- optional-dep@1.0.0 -+-- peer-dep@1.0.0 invalid ++-- peer-dep@1.0.0 "^2.0.0" from the root project \`-- prod-dep@1.0.0 \`-- dog@2.0.0 @@ -567,7 +567,7 @@ exports[`test/lib/ls.js TAP ls missing package.json > should output tree missing exports[`test/lib/ls.js TAP ls missing/invalid/extraneous > should output tree containing missing, invalid, extraneous labels 1`] = ` test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls-missing-invalid-extraneous +-- chai@1.0.0 extraneous -+-- foo@1.0.0 invalid ++-- foo@1.0.0 "^2.0.0" from the root project | \`-- dog@1.0.0 \`-- UNMET DEPENDENCY ipsum@^1.0.0 @@ -602,7 +602,7 @@ exports[`test/lib/ls.js TAP ls unmet optional dep > should output tree with empt | \`-- foo@1.0.0 | \`-- dog@1.0.0 +-- UNMET OPTIONAL DEPENDENCY missing-optional-dep@^1.0.0 -+-- optional-dep@1.0.0 invalid ++-- optional-dep@1.0.0 "^2.0.0" from the root project +-- peer-dep@1.0.0 \`-- prod-dep@1.0.0  \`-- dog@2.0.0 @@ -691,3 +691,14 @@ dedupe-entries@1.0.0 {CWD}/tap-testdir-ls-ls-with-no-args-dedupe-entries-and-not \`-- @npmcli/c@1.0.0 ` + +exports[`test/lib/ls.js TAP show multiple invalid reasons > ls result 1`] = ` +test-npm-ls@1.0.0 {cwd}/tap-testdir-ls-show-multiple-invalid-reasons ++-- cat@1.0.0 "^2.0.0" from the root project +| \`-- dog@1.0.0 deduped ++-- chai@1.0.0 extraneous +| \`-- dog@1.0.0 deduped +\`-- dog@1.0.0 "^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 + +` diff --git a/test/lib/ls.js b/test/lib/ls.js index 4d6fef52b629e..5f196501e55d1 100644 --- a/test/lib/ls.js +++ b/test/lib/ls.js @@ -123,7 +123,23 @@ const ls = new LS(npm) const redactCwd = res => res && res.replace(/\\+/g, '/').replace(new RegExp(__dirname.replace(/\\+/g, '/'), 'gi'), '{CWD}') -const jsonParse = res => JSON.parse(redactCwd(res)) +const redactCwdObj = obj => { + if (Array.isArray(obj)) + return obj.map(o => redactCwdObj(o)) + else if (typeof obj === 'string') + return redactCwd(obj) + else if (!obj) + return obj + else if (typeof obj === 'object') { + return Object.keys(obj).reduce((o, k) => { + o[k] = redactCwdObj(obj[k]) + return o + }, {}) + } else + return obj +} + +const jsonParse = res => redactCwdObj(JSON.parse(res)) const cleanUpResult = () => result = '' @@ -3060,7 +3076,7 @@ t.test('ls --json', (t) => { dependencies: { foo: { version: '1.0.0', - invalid: true, + invalid: '"^2.0.0" from the root project', problems: [ 'invalid: foo@1.0.0 {CWD}/tap-testdir-ls-ls---json-missing-invalid-extraneous/node_modules/foo', ], @@ -3756,7 +3772,7 @@ t.test('ls --json', (t) => { dependencies: { 'peer-dep': { version: '1.0.0', - invalid: true, + invalid: '"^2.0.0" from the root project', problems: [ 'invalid: peer-dep@1.0.0 {CWD}/tap-testdir-ls-ls---json-unmet-peer-dep/node_modules/peer-dep', ], @@ -3817,7 +3833,7 @@ t.test('ls --json', (t) => { dependencies: { 'optional-dep': { version: '1.0.0', - invalid: true, + invalid: '"^2.0.0" from the root project', problems: [ 'invalid: optional-dep@1.0.0 {CWD}/tap-testdir-ls-ls---json-unmet-optional-dep/node_modules/optional-dep', ], @@ -4175,6 +4191,59 @@ t.test('ls --json', (t) => { t.end() }) +t.test('show multiple invalid reasons', (t) => { + config.json = false + config.all = true + config.depth = Infinity + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-npm-ls', + version: '1.0.0', + dependencies: { + cat: '^2.0.0', + dog: '^1.2.3', + }, + }), + node_modules: { + cat: { + 'package.json': JSON.stringify({ + name: 'cat', + version: '1.0.0', + dependencies: { + dog: '^2.0.0', + }, + }), + }, + dog: { + 'package.json': JSON.stringify({ + name: 'dog', + version: '1.0.0', + dependencies: { + cat: '', + }, + }), + }, + chai: { + 'package.json': JSON.stringify({ + name: 'chai', + version: '1.0.0', + dependencies: { + dog: '2.x', + }, + }), + }, + }, + }) + + const cleanupPaths = str => + redactCwd(str).toLowerCase().replace(/\\/g, '/') + ls.exec([], (err) => { + t.match(err, { code: 'ELSPROBLEMS' }, 'should list dep problems') + t.matchSnapshot(cleanupPaths(result), 'ls result') + t.end() + }) +}) + t.test('ls --package-lock-only', (t) => { config['package-lock-only'] = true t.test('ls --package-lock-only --json', (t) => { @@ -4751,7 +4820,7 @@ t.test('ls --package-lock-only', (t) => { dependencies: { foo: { version: '1.0.0', - invalid: true, + invalid: '"^2.0.0" from the root project', problems: [ 'invalid: foo@1.0.0 {CWD}/tap-testdir-ls-ls---package-lock-only-ls---package-lock-only---json-missing-invalid-extraneous/node_modules/foo', ],