Skip to content

Commit

Permalink
fix: fund with multiple funding sources
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
ruyadorno authored and isaacs committed Aug 25, 2020
1 parent aa0152b commit 50f9740
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 20 deletions.
37 changes: 22 additions & 15 deletions lib/fund.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,26 +42,33 @@ function printHuman (fundingInfo, { color, unicode }) {

const result = depth({
tree: fundingInfo,

// composes human readable package name
// and creates a new archy item for readable output
visit: ({ name, version, funding }) => {
// composes human readable package name
// and creates a new archy item for readable output
const { url } = funding || {}
const [fundingSource] = []
.concat(normalizeFunding(funding))
.filter(isValidFunding)
const { url } = fundingSource || {}
const pkgRef = getPrintableName({ name, version })
const label = url ? tree({
label: color ? chalk.bgBlack.white(url) : url,
nodes: [pkgRef]
}).trim() : pkgRef
let item = {
label
label: pkgRef
}

// stacks all packages together under the same item
if (seenUrls.has(url)) {
item = seenUrls.get(url)
item.label += `, ${pkgRef}`
return null
} else {
seenUrls.set(url, item)
if (url) {
item.label = tree({
label: color ? chalk.bgBlack.white(url) : url,
nodes: [pkgRef]
}).trim()

// stacks all packages together under the same item
if (seenUrls.has(url)) {
item = seenUrls.get(url)
item.label += `, ${pkgRef}`
return null
} else {
seenUrls.set(url, item)
}
}

return item
Expand Down
10 changes: 10 additions & 0 deletions tap-snapshots/test-lib-fund.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,14 @@ exports[`test/lib/fund.js TAP fund with no package containing funding > should p
no-funding-package@0.0.0
`

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`] = `
test-multiple-funding-sources@1.0.0
+-- http://example.com/b
| \`-- b@1.0.0
\`-- http://example.com/c
\`-- c@1.0.0
`
60 changes: 55 additions & 5 deletions test/lib/fund.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ const _flatOptions = {
unicode: false,
which: undefined
}
let openUrl = (url, msg, cb) => {
const openUrl = (url, msg, cb) => {
if (url === 'http://npmjs.org') {
cb(new Error('ERROR'))
return
Expand All @@ -212,7 +212,7 @@ const fund = requireInject('../../lib/fund.js', {
},
'../../lib/utils/open-url.js': openUrl,
'../../lib/utils/output.js': msg => { result += msg + '\n' },
'pacote': {
pacote: {
manifest: (arg) => arg.name === 'ntl'
? Promise.resolve({
funding: 'http://example.com/pacote'
Expand All @@ -221,7 +221,6 @@ const fund = requireInject('../../lib/fund.js', {
}
})


test('fund with no package containing funding', t => {
_flatOptions.prefix = t.testdir({
'package.json': JSON.stringify({
Expand Down Expand Up @@ -472,7 +471,7 @@ test('fund using symlink ref', t => {
name: 'using-symlink-ref',
version: '1.0.0'
}),
'a': {
a: {
'package.json': JSON.stringify({
name: 'a',
version: '1.0.0',
Expand All @@ -497,6 +496,8 @@ test('fund using symlink ref', t => {

// using target ref
fund(['./a'], (err) => {
t.ifError(err, 'should not error out')

t.match(
printUrl,
'http://example.com/a',
Expand Down Expand Up @@ -779,7 +780,7 @@ test('fund colors', t => {
version: '1.0.0',
funding: 'http://example.com/e'
})
},
}
}
})
_flatOptions.color = true
Expand All @@ -793,3 +794,52 @@ test('fund colors', t => {
t.end()
})
})

test('sub dep with fund info and a parent with no funding info', t => {
_flatOptions.prefix = t.testdir({
'package.json': JSON.stringify({
name: 'test-multiple-funding-sources',
version: '1.0.0',
dependencies: {
a: '^1.0.0',
b: '^1.0.0'
}
}),
node_modules: {
a: {
'package.json': JSON.stringify({
name: 'a',
version: '1.0.0',
dependencies: {
c: '^1.0.0'
}
})
},
b: {
'package.json': JSON.stringify({
name: 'b',
version: '1.0.0',
funding: 'http://example.com/b'
})
},
c: {
'package.json': JSON.stringify({
name: 'c',
version: '1.0.0',
funding: [
'http://example.com/c',
'http://example.com/c-other'
]
})
}
}
})

fund([], (err) => {
t.ifError(err, 'should not error out')
t.matchSnapshot(result, 'should nest sub dep as child of root')

result = ''
t.end()
})
})

0 comments on commit 50f9740

Please sign in to comment.