Skip to content

Commit c371f18

Browse files
isaacsnlf
authored andcommitted
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: #3140 Credit: @isaacs Close: #3140 Reviewed-by: @ruyadorno
1 parent 8f8f71e commit c371f18

File tree

5 files changed

+152
-10
lines changed

5 files changed

+152
-10
lines changed

lib/ls.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -414,9 +414,11 @@ const augmentNodesWithMetadata = ({
414414
path: node.path,
415415
isLink: node.isLink,
416416
realpath: node.realpath,
417+
[_type]: node[_type],
417418
[_invalid]: node[_invalid],
418419
[_missing]: node[_missing],
419-
[_dedupe]: true,
420+
// if it's missing, it's not deduped, it's just missing
421+
[_dedupe]: !node[_missing],
420422
}
421423
} else {
422424
// keeps track of already seen nodes in order to check for dedupes

package-lock.json

+7-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@
190190
"jsdom": "^16.5.2",
191191
"licensee": "^8.1.0",
192192
"marked-man": "^0.7.0",
193-
"tap": "^15.0.2",
193+
"tap": "^15.0.5",
194194
"yaml": "^1.10.2"
195195
},
196196
"scripts": {

tap-snapshots/test/lib/ls.js.test.cjs

+41-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,46 @@
55
* Make sure to inspect the output below. Do not ignore changes!
66
*/
77
'use strict'
8+
exports[`test/lib/ls.js TAP ignore missing optional deps --json > ls --json problems 1`] = `
9+
Array [
10+
"invalid: [email protected] {project}/node_modules/optional-wrong",
11+
"missing: peer-missing@1, required by [email protected]",
12+
"invalid: [email protected] {project}/node_modules/peer-optional-wrong",
13+
"invalid: [email protected] {project}/node_modules/peer-wrong",
14+
"missing: prod-missing@1, required by [email protected]",
15+
"invalid: [email protected] {project}/node_modules/prod-wrong",
16+
]
17+
`
18+
19+
exports[`test/lib/ls.js TAP ignore missing optional deps --parseable > ls --parseable result 1`] = `
20+
{project}
21+
{project}/node_modules/optional-ok
22+
{project}/node_modules/optional-wrong
23+
{project}/node_modules/peer-ok
24+
{project}/node_modules/peer-optional-ok
25+
{project}/node_modules/peer-optional-wrong
26+
{project}/node_modules/peer-wrong
27+
{project}/node_modules/prod-ok
28+
{project}/node_modules/prod-wrong
29+
`
30+
31+
exports[`test/lib/ls.js TAP ignore missing optional deps human output > ls result 1`] = `
32+
33+
+-- unmet optional dependency optional-missing@1
34+
35+
+-- [email protected] invalid
36+
+-- unmet dependency peer-missing@1
37+
38+
+-- unmet optional dependency peer-optional-missing@1
39+
40+
+-- [email protected] invalid
41+
+-- [email protected] invalid
42+
+-- unmet dependency prod-missing@1
43+
44+
\`-- [email protected] invalid
45+
46+
`
47+
848
exports[`test/lib/ls.js TAP ls --depth=0 > should output tree containing only top-level dependencies 1`] = `
949
[email protected] {CWD}/tap-testdir-ls-ls---depth-0
1050
@@ -341,7 +381,7 @@ exports[`test/lib/ls.js TAP ls cycle deps with filter args > should print tree o
341381
exports[`test/lib/ls.js TAP ls deduped missing dep > should output parseable signaling missing peer dep in problems 1`] = `
342382
[email protected] {CWD}/tap-testdir-ls-ls-deduped-missing-dep
343383
344-
| \`-- UNMET DEPENDENCY b@^1.0.0 deduped
384+
| \`-- UNMET DEPENDENCY b@^1.0.0
345385
\`-- UNMET DEPENDENCY b@^1.0.0
346386
347387
`

test/lib/ls.js

+100
Original file line numberDiff line numberDiff line change
@@ -2352,6 +2352,106 @@ t.test('ls --parseable', (t) => {
23522352
t.end()
23532353
})
23542354

2355+
t.test('ignore missing optional deps', async t => {
2356+
t.beforeEach(cleanUpResult)
2357+
npm.prefix = t.testdir({
2358+
'package.json': JSON.stringify({
2359+
name: 'test-npm-ls-ignore-missing-optional',
2360+
version: '1.2.3',
2361+
peerDependencies: {
2362+
'peer-ok': '1',
2363+
'peer-missing': '1',
2364+
'peer-wrong': '1',
2365+
'peer-optional-ok': '1',
2366+
'peer-optional-missing': '1',
2367+
'peer-optional-wrong': '1',
2368+
},
2369+
peerDependenciesMeta: {
2370+
'peer-optional-ok': {
2371+
optional: true,
2372+
},
2373+
'peer-optional-missing': {
2374+
optional: true,
2375+
},
2376+
'peer-optional-wrong': {
2377+
optional: true,
2378+
},
2379+
},
2380+
optionalDependencies: {
2381+
'optional-ok': '1',
2382+
'optional-missing': '1',
2383+
'optional-wrong': '1',
2384+
},
2385+
dependencies: {
2386+
'prod-ok': '1',
2387+
'prod-missing': '1',
2388+
'prod-wrong': '1',
2389+
},
2390+
}),
2391+
node_modules: {
2392+
'prod-ok': {
2393+
'package.json': JSON.stringify({name: 'prod-ok', version: '1.2.3' }),
2394+
},
2395+
'prod-wrong': {
2396+
'package.json': JSON.stringify({name: 'prod-wrong', version: '3.2.1' }),
2397+
},
2398+
'optional-ok': {
2399+
'package.json': JSON.stringify({name: 'optional-ok', version: '1.2.3' }),
2400+
},
2401+
'optional-wrong': {
2402+
'package.json': JSON.stringify({name: 'optional-wrong', version: '3.2.1' }),
2403+
},
2404+
'peer-optional-ok': {
2405+
'package.json': JSON.stringify({name: 'peer-optional-ok', version: '1.2.3' }),
2406+
},
2407+
'peer-optional-wrong': {
2408+
'package.json': JSON.stringify({name: 'peer-optional-wrong', version: '3.2.1' }),
2409+
},
2410+
'peer-ok': {
2411+
'package.json': JSON.stringify({name: 'peer-ok', version: '1.2.3' }),
2412+
},
2413+
'peer-wrong': {
2414+
'package.json': JSON.stringify({name: 'peer-wrong', version: '3.2.1' }),
2415+
},
2416+
},
2417+
})
2418+
2419+
config.all = true
2420+
const prefix = npm.prefix.toLowerCase().replace(/\\/g, '/')
2421+
const cleanupPaths = str =>
2422+
str.toLowerCase().replace(/\\/g, '/').split(prefix).join('{project}')
2423+
2424+
t.test('--json', t => {
2425+
config.json = true
2426+
config.parseable = false
2427+
ls.exec([], (err) => {
2428+
t.match(err, { code: 'ELSPROBLEMS' })
2429+
result = JSON.parse(result)
2430+
const problems = result.problems.map(cleanupPaths)
2431+
t.matchSnapshot(problems, 'ls --json problems')
2432+
t.end()
2433+
})
2434+
})
2435+
t.test('--parseable', t => {
2436+
config.json = false
2437+
config.parseable = true
2438+
ls.exec([], (err) => {
2439+
t.match(err, { code: 'ELSPROBLEMS' })
2440+
t.matchSnapshot(cleanupPaths(result), 'ls --parseable result')
2441+
t.end()
2442+
})
2443+
})
2444+
t.test('human output', t => {
2445+
config.json = false
2446+
config.parseable = false
2447+
ls.exec([], (err) => {
2448+
t.match(err, { code: 'ELSPROBLEMS' })
2449+
t.matchSnapshot(cleanupPaths(result), 'ls result')
2450+
t.end()
2451+
})
2452+
})
2453+
})
2454+
23552455
t.test('ls --json', (t) => {
23562456
t.beforeEach(cleanUpResult)
23572457
config.json = true

0 commit comments

Comments
 (0)