Skip to content

Commit

Permalink
ls: do not warn on missing optional deps
Browse files Browse the repository at this point in the history 
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: #3140
Credit: @isaacs
Close: #3140
Reviewed-by: @ruyadorno
  • Loading branch information
@isaacs @nlf
isaacs authored and nlf committed Apr 29, 2021
1 parent 8f8f71e commit c371f18
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 10 deletions.
4 changes: 3 additions & 1 deletion lib/ls.js
View file
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -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": {
Expand Down
42 changes: 41 additions & 1 deletion tap-snapshots/test/lib/ls.js.test.cjs
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
`
Expand Down
100 changes: 100 additions & 0 deletions test/lib/ls.js
View file
Expand Up @@ -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
Expand Down

0 comments on commit c371f18

Please sign in to comment.