Skip to content

Commit 50f9740

Browse files
ruyadornoisaacs
ruyadorno
authored and
isaacs
committed
fix: fund with multiple funding sources
`npm fund` human output was appending any items that had multiple funding sources to the current package title as comma-separated names. This commit fixes the problem by properly selecting the first item of a each funding element and only using that as its index for printing the human output tree representation. PR-URL: #1717 Credit: @ruyadorno Close: #1717 Reviewed-by: @isaacs
1 parent aa0152b commit 50f9740

3 files changed

Lines changed: 87 additions & 20 deletions

File tree

‎lib/fund.js‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,33 @@ function printHuman (fundingInfo, { color, unicode }) {
4242

4343
const result = depth({
4444
tree: fundingInfo,
45+
46+
// composes human readable package name
47+
// and creates a new archy item for readable output
4548
visit: ({ name, version, funding }) => {
46-
// composes human readable package name
47-
// and creates a new archy item for readable output
48-
const { url } = funding || {}
49+
const [fundingSource] = []
50+
.concat(normalizeFunding(funding))
51+
.filter(isValidFunding)
52+
const { url } = fundingSource || {}
4953
const pkgRef = getPrintableName({ name, version })
50-
const label = url ? tree({
51-
label: color ? chalk.bgBlack.white(url) : url,
52-
nodes: [pkgRef]
53-
}).trim() : pkgRef
5454
let item = {
55-
label
55+
label: pkgRef
5656
}
5757

58-
// stacks all packages together under the same item
59-
if (seenUrls.has(url)) {
60-
item = seenUrls.get(url)
61-
item.label += `, ${pkgRef}`
62-
return null
63-
} else {
64-
seenUrls.set(url, item)
58+
if (url) {
59+
item.label = tree({
60+
label: color ? chalk.bgBlack.white(url) : url,
61+
nodes: [pkgRef]
62+
}).trim()
63+
64+
// stacks all packages together under the same item
65+
if (seenUrls.has(url)) {
66+
item = seenUrls.get(url)
67+
item.label += `, ${pkgRef}`
68+
return null
69+
} else {
70+
seenUrls.set(url, item)
71+
}
6572
}
6673

6774
return item

‎tap-snapshots/test-lib-fund.js-TAP.test.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,14 @@ exports[`test/lib/fund.js TAP fund with no package containing funding > should p
8181
[email protected]
8282
8383
84+
`
85+
86+
exports[`test/lib/fund.js TAP sub dep with fund info and a parent with no funding info > should nest sub dep as child of root 1`] = `
87+
[email protected]
88+
+-- http://example.com/b
89+
| \`-- [email protected]
90+
\`-- http://example.com/c
91+
\`-- [email protected]
92+
93+
8494
`

‎test/lib/fund.js‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ const _flatOptions = {
190190
unicode: false,
191191
which: undefined
192192
}
193-
let openUrl = (url, msg, cb) => {
193+
const openUrl = (url, msg, cb) => {
194194
if (url === 'http://npmjs.org') {
195195
cb(new Error('ERROR'))
196196
return
@@ -212,7 +212,7 @@ const fund = requireInject('../../lib/fund.js', {
212212
},
213213
'../../lib/utils/open-url.js': openUrl,
214214
'../../lib/utils/output.js': msg => { result += msg + '\n' },
215-
'pacote': {
215+
pacote: {
216216
manifest: (arg) => arg.name === 'ntl'
217217
? Promise.resolve({
218218
funding: 'http://example.com/pacote'
@@ -221,7 +221,6 @@ const fund = requireInject('../../lib/fund.js', {
221221
}
222222
})
223223

224-
225224
test('fund with no package containing funding', t => {
226225
_flatOptions.prefix = t.testdir({
227226
'package.json': JSON.stringify({
@@ -472,7 +471,7 @@ test('fund using symlink ref', t => {
472471
name: 'using-symlink-ref',
473472
version: '1.0.0'
474473
}),
475-
'a': {
474+
a: {
476475
'package.json': JSON.stringify({
477476
name: 'a',
478477
version: '1.0.0',
@@ -497,6 +496,8 @@ test('fund using symlink ref', t => {
497496

498497
// using target ref
499498
fund(['./a'], (err) => {
499+
t.ifError(err, 'should not error out')
500+
500501
t.match(
501502
printUrl,
502503
'http://example.com/a',
@@ -779,7 +780,7 @@ test('fund colors', t => {
779780
version: '1.0.0',
780781
funding: 'http://example.com/e'
781782
})
782-
},
783+
}
783784
}
784785
})
785786
_flatOptions.color = true
@@ -793,3 +794,52 @@ test('fund colors', t => {
793794
t.end()
794795
})
795796
})
797+
798+
test('sub dep with fund info and a parent with no funding info', t => {
799+
_flatOptions.prefix = t.testdir({
800+
'package.json': JSON.stringify({
801+
name: 'test-multiple-funding-sources',
802+
version: '1.0.0',
803+
dependencies: {
804+
a: '^1.0.0',
805+
b: '^1.0.0'
806+
}
807+
}),
808+
node_modules: {
809+
a: {
810+
'package.json': JSON.stringify({
811+
name: 'a',
812+
version: '1.0.0',
813+
dependencies: {
814+
c: '^1.0.0'
815+
}
816+
})
817+
},
818+
b: {
819+
'package.json': JSON.stringify({
820+
name: 'b',
821+
version: '1.0.0',
822+
funding: 'http://example.com/b'
823+
})
824+
},
825+
c: {
826+
'package.json': JSON.stringify({
827+
name: 'c',
828+
version: '1.0.0',
829+
funding: [
830+
'http://example.com/c',
831+
'http://example.com/c-other'
832+
]
833+
})
834+
}
835+
}
836+
})
837+
838+
fund([], (err) => {
839+
t.ifError(err, 'should not error out')
840+
t.matchSnapshot(result, 'should nest sub dep as child of root')
841+
842+
result = ''
843+
t.end()
844+
})
845+
})

0 commit comments

Comments
 (0)