diff options
author | Ruy Adorno <ruyadorno@hotmail.com> | 2020-08-24 23:07:19 +0300 |
---|---|---|
committer | isaacs <i@izs.me> | 2020-08-25 21:08:23 +0300 |
commit | 50f9740ca8000b1c4bd3155bf1bc3d58fb6f0e20 (patch) | |
tree | 4e02431e70b22ac48bb5580503612c77141fd69c | |
parent | aa0152b58f34f8cdae05be63853c6e0ace03236a (diff) |
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: https://github.com/npm/cli/pull/1717
Credit: @ruyadorno
Close: #1717
Reviewed-by: @isaacs
-rw-r--r-- | lib/fund.js | 37 | ||||
-rw-r--r-- | tap-snapshots/test-lib-fund.js-TAP.test.js | 10 | ||||
-rw-r--r-- | test/lib/fund.js | 60 |
3 files changed, 87 insertions, 20 deletions
diff --git a/lib/fund.js b/lib/fund.js index fde01f469..d1c58d9b9 100644 --- a/lib/fund.js +++ b/lib/fund.js @@ -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 diff --git a/tap-snapshots/test-lib-fund.js-TAP.test.js b/tap-snapshots/test-lib-fund.js-TAP.test.js index 4ff676ca1..7ad86ebee 100644 --- a/tap-snapshots/test-lib-fund.js-TAP.test.js +++ b/tap-snapshots/test-lib-fund.js-TAP.test.js @@ -82,3 +82,13 @@ 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 + + +` diff --git a/test/lib/fund.js b/test/lib/fund.js index 9025d7ee0..fc6a63aa1 100644 --- a/test/lib/fund.js +++ b/test/lib/fund.js @@ -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 @@ -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' @@ -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({ @@ -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', @@ -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', @@ -779,7 +780,7 @@ test('fund colors', t => { version: '1.0.0', funding: 'http://example.com/e' }) - }, + } } }) _flatOptions.color = true @@ -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() + }) +}) |