Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ls): report *why* something is invalid #3460

Merged
merged 1 commit into from
Jun 23, 2021
Merged

feat(ls): report *why* something is invalid #3460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions lib/ls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ const getHumanOutputItem = (node, { args, color, global, long }) => {
const targetLocation = node.root
? relative(node.root.realpath, node.realpath)
: node.targetLocation
const invalid = node[_invalid]
? `invalid: ${node[_invalid]}`
: ''
const label =
(
node[_missing]
Expand All @@ -321,8 +324,8 @@ const getHumanOutputItem = (node, { args, color, global, long }) => {
: ''
) +
(
node[_invalid]
? ' ' + (color ? chalk.red.bgBlack('invalid') : 'invalid')
invalid
? ' ' + (color ? chalk.red.bgBlack(invalid) : invalid)
: ''
) +
(
Expand Down Expand Up @@ -373,7 +376,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]
Expand Down Expand Up @@ -432,9 +435,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
}
Expand Down
31 changes: 21 additions & 10 deletions tap-snapshots/test/lib/ls.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 invalid: "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 invalid: "1" from the root project
+-- peer-wrong@3.2.1 invalid: "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 invalid: "1" from the root project

`

Expand Down Expand Up @@ -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 invalid: "^2.0.0" from the root project
| \`-- dog@1.0.0
\`-- UNMET DEPENDENCY ipsum@^1.0.0

Expand Down Expand Up @@ -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 invalid: "^2.0.0" from the root project, "^2.0.0" from node_modules/a
\`-- b@1.0.0 invalid: "^2.0.0" from the root project, "^2.0.0" from node_modules/a

`

Expand All @@ -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 invalid: "^2.0.0" from the root project
\`-- prod-dep@1.0.0
\`-- dog@2.0.0

Expand Down Expand Up @@ -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 invalid: "^2.0.0" from the root project
| \`-- dog@1.0.0
\`-- UNMET DEPENDENCY ipsum@^1.0.0

Expand Down Expand Up @@ -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 invalid: "^2.0.0" from the root project
+-- peer-dep@1.0.0
\`-- prod-dep@1.0.0
 \`-- dog@2.0.0
Expand Down Expand Up @@ -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 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
+-- 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
\`-- 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
\`-- cat@1.0.0 deduped invalid: "^2.0.0" from the root project

`
79 changes: 74 additions & 5 deletions test/lib/ls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ''

Expand Down Expand Up @@ -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',
],
Expand Down Expand Up @@ -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',
],
Expand Down Expand Up @@ -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',
],
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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',
],
Expand Down