From 30f170877954acd036cb234a581e4eb155049b82 Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Mon, 27 Jan 2020 15:29:04 -0800 Subject: [PATCH] fund: support multiple funding sources See https://github.com/npm/rfcs/pull/68 PR-URL: https://github.com/npm/cli/pull/731 Credit: @ Close: #731 Reviewed-by: @Darcy Clarke --- docs/content/cli-commands/npm-fund.md | 12 +- docs/content/configuring-npm/package-json.md | 21 ++- lib/fund.js | 128 +++++++------------ lib/utils/funding.js | 94 +++++++++----- tap-snapshots/test-tap-fund.js-TAP.test.js | 35 +++-- test/tap/fund.js | 96 +++++++++++++- test/tap/utils.funding.js | 43 +++++++ 7 files changed, 300 insertions(+), 129 deletions(-) diff --git a/docs/content/cli-commands/npm-fund.md b/docs/content/cli-commands/npm-fund.md index 64894e291fc4f..5a751eec46687 100644 --- a/docs/content/cli-commands/npm-fund.md +++ b/docs/content/cli-commands/npm-fund.md @@ -21,7 +21,8 @@ a given project. If no package name is provided, it will list all dependencies that are looking for funding in a tree-structure in which are listed the type of funding and the url to visit. If a package name is provided then it tries to open its funding url using the `--browser` -config param. +config param; if there are multiple funding sources for the package, the +user will be instructed to pass the `--which` command to disambiguate. The list will avoid duplicated entries and will stack all packages that share the same type/url as a single entry. Given this nature the @@ -38,8 +39,8 @@ The browser that is called by the `npm fund` command to open websites. #### json -* Default: false * Type: Boolean +* Default: false Show information in JSON format. @@ -51,6 +52,13 @@ Show information in JSON format. Whether to represent the tree structure using unicode characters. Set it to `false` in order to use all-ansi output. +#### which + +* Type: Number +* Default: undefined + +If there are multiple funding sources, which 1-indexed source URL to open. + ## See Also * [npm docs](/cli-commands/npm-docs) diff --git a/docs/content/configuring-npm/package-json.md b/docs/content/configuring-npm/package-json.md index ccdbe041fb063..9af70ea0ab2cc 100644 --- a/docs/content/configuring-npm/package-json.md +++ b/docs/content/configuring-npm/package-json.md @@ -197,7 +197,8 @@ npm also sets a top-level "maintainers" field with your npm user info. ### funding You can specify an object containing an URL that provides up-to-date -information about ways to help fund development of your package: +information about ways to help fund development of your package, or +a string URL, or an array of these: "funding": { "type" : "individual", @@ -209,10 +210,26 @@ information about ways to help fund development of your package: "url" : "https://www.patreon.com/my-account" } + "funding": "http://example.com/donate" + + "funding": [ + { + "type" : "individual", + "url" : "http://example.com/donate" + }, + "http://example.com/donateAlso", + { + "type" : "patreon", + "url" : "https://www.patreon.com/my-account" + } + ] + + Users can use the `npm fund` subcommand to list the `funding` URLs of all dependencies of their project, direct and indirect. A shortcut to visit each funding url is also available when providing the project name such as: -`npm fund `. +`npm fund ` (when there are multiple URLs, the first one will be +visited) ### files diff --git a/lib/fund.js b/lib/fund.js index 00954c844d798..e605ea8c3fd03 100644 --- a/lib/fund.js +++ b/lib/fund.js @@ -14,13 +14,14 @@ const readShrinkwrap = require('./install/read-shrinkwrap.js') const mutateIntoLogicalTree = require('./install/mutate-into-logical-tree.js') const output = require('./utils/output.js') const openUrl = require('./utils/open-url.js') -const { getFundingInfo, retrieveFunding, validFundingUrl } = require('./utils/funding.js') +const { getFundingInfo, retrieveFunding, validFundingField, flatCacheSymbol } = require('./utils/funding.js') const FundConfig = figgyPudding({ browser: {}, // used by ./utils/open-url global: {}, json: {}, - unicode: {} + unicode: {}, + which: {} }) module.exports = fundCmd @@ -29,7 +30,7 @@ const usage = require('./utils/usage') fundCmd.usage = usage( 'fund', 'npm fund [--json]', - 'npm fund [--browser] [[<@scope>/]' + 'npm fund [--browser] [[<@scope>/] [--which=]' ) fundCmd.completion = function (opts, cb) { @@ -52,96 +53,52 @@ function printJSON (fundingInfo) { // level possible, in that process they also carry their dependencies along // with them, moving those up in the visual tree function printHuman (fundingInfo, opts) { - // mapping logic that keeps track of seen items in order to be able - // to push all other items from the same type/url in the same place - const seen = new Map() + const flatCache = fundingInfo[flatCacheSymbol] - function seenKey ({ type, url } = {}) { - return url ? String(type) + String(url) : null - } - - function setStackedItem (funding, result) { - const key = seenKey(funding) - if (key && !seen.has(key)) seen.set(key, result) - } + const { name, version } = fundingInfo + const printableVersion = version ? `@${version}` : '' - function retrieveStackedItem (funding) { - const key = seenKey(funding) - if (key && seen.has(key)) return seen.get(key) - } + const items = Object.keys(flatCache).map((url) => { + const deps = flatCache[url] - // --- - - const getFundingItems = (fundingItems) => - Object.keys(fundingItems || {}).map((fundingItemName) => { - // first-level loop, prepare the pretty-printed formatted data - const fundingItem = fundingItems[fundingItemName] - const { version, funding } = fundingItem - const { type, url } = funding || {} + const packages = deps.map((dep) => { + const { name, version } = dep const printableVersion = version ? `@${version}` : '' - const printableType = type && { label: `type: ${funding.type}` } - const printableUrl = url && { label: `url: ${funding.url}` } - const result = { - fundingItem, - label: fundingItemName + printableVersion, - nodes: [] - } - - if (printableType) { - result.nodes.push(printableType) - } - - if (printableUrl) { - result.nodes.push(printableUrl) - } - - setStackedItem(funding, result) - - return result - }).reduce((res, result) => { - // recurse and exclude nodes that are going to be stacked together - const { fundingItem } = result - const { dependencies, funding } = fundingItem - const items = getFundingItems(dependencies) - const stackedResult = retrieveStackedItem(funding) - items.forEach(i => result.nodes.push(i)) - - if (stackedResult && stackedResult !== result) { - stackedResult.label += `, ${result.label}` - items.forEach(i => stackedResult.nodes.push(i)) - return res - } - - res.push(result) - - return res - }, []) - - const [ result ] = getFundingItems({ - [fundingInfo.name]: { - dependencies: fundingInfo.dependencies, - funding: fundingInfo.funding, - version: fundingInfo.version + return `${name}${printableVersion}` + }) + + return { + label: url, + nodes: [packages.join(', ')] } }) - return archy(result, '', { unicode: opts.unicode }) + return archy({ label: `${name}${printableVersion}`, nodes: items }, '', { unicode: opts.unicode }) } -function openFundingUrl (packageName, cb) { +function openFundingUrl (packageName, fundingSourceNumber, cb) { function getUrlAndOpen (packageMetadata) { const { funding } = packageMetadata - const { type, url } = retrieveFunding(funding) || {} - const noFundingError = - new Error(`No funding method available for: ${packageName}`) - noFundingError.code = 'ENOFUND' - const typePrefix = type ? `${type} funding` : 'Funding' - const msg = `${typePrefix} available at the following URL` - - if (validFundingUrl(funding)) { + const validSources = [].concat(retrieveFunding(funding)).filter(validFundingField) + + if (validSources.length === 1 || (fundingSourceNumber > 0 && fundingSourceNumber <= validSources.length)) { + const { type, url } = validSources[fundingSourceNumber ? fundingSourceNumber - 1 : 0] + const typePrefix = type ? `${type} funding` : 'Funding' + const msg = `${typePrefix} available at the following URL` openUrl(url, msg, cb) + } else if (!(fundingSourceNumber >= 1)) { + validSources.forEach(({ type, url }, i) => { + const typePrefix = type ? `${type} funding` : 'Funding' + const msg = `${typePrefix} available at the following URL` + console.log(`${i + 1}: ${msg}: ${url}`) + }) + console.log('Run `npm fund [<@scope>/] --which=1`, for example, to open the first funding URL listed in that package') + cb() } else { + const noFundingError = new Error(`No valid funding method available for: ${packageName}`) + noFundingError.code = 'ENOFUND' + throw noFundingError } } @@ -161,15 +118,24 @@ function fundCmd (args, cb) { const opts = FundConfig(npmConfig()) const dir = path.resolve(npm.dir, '..') const packageName = args[0] + const numberArg = opts.which + + const fundingSourceNumber = numberArg && parseInt(numberArg, 10) + + if (numberArg !== undefined && (String(fundingSourceNumber) !== numberArg || fundingSourceNumber < 1)) { + const err = new Error('`npm fund [<@scope>/] [--which=fundingSourceNumber]` must be given a positive integer') + err.code = 'EFUNDNUMBER' + throw err + } if (opts.global) { - const err = new Error('`npm fund` does not support globals') + const err = new Error('`npm fund` does not support global packages') err.code = 'EFUNDGLOBAL' throw err } if (packageName) { - openFundingUrl(packageName, cb) + openFundingUrl(packageName, fundingSourceNumber, cb) return } diff --git a/lib/utils/funding.js b/lib/utils/funding.js index c3d06b1089963..5375639109ee2 100644 --- a/lib/utils/funding.js +++ b/lib/utils/funding.js @@ -4,22 +4,31 @@ const URL = require('url').URL exports.getFundingInfo = getFundingInfo exports.retrieveFunding = retrieveFunding -exports.validFundingUrl = validFundingUrl +exports.validFundingField = validFundingField -// supports both object funding and string shorthand +const flatCacheSymbol = Symbol('npm flat cache') +exports.flatCacheSymbol = flatCacheSymbol + +// supports object funding and string shorthand, or an array of these +// if original was an array, returns an array; else returns the lone item function retrieveFunding (funding) { - return typeof funding === 'string' - ? { - url: funding - } - : funding + const sources = [].concat(funding || []).map(item => ( + typeof item === 'string' + ? { url: item } + : item + )) + return Array.isArray(funding) ? sources : sources[0] } // Is the value of a `funding` property of a `package.json` // a valid type+url for `npm fund` to display? -function validFundingUrl (funding) { +function validFundingField (funding) { if (!funding) return false + if (Array.isArray(funding)) { + return funding.every(f => !Array.isArray(f) && validFundingField(f)) + } + try { var parsed = new URL(funding.url || funding) } catch (error) { @@ -34,11 +43,13 @@ function validFundingUrl (funding) { return Boolean(parsed.host) } +const empty = () => Object.create(null) + function getFundingInfo (idealTree, opts) { - let length = 0 + let packageWithFundingCount = 0 + const flat = empty() const seen = new Set() const { countOnly } = opts || {} - const empty = () => Object.create(null) const _trailingDependencies = Symbol('trailingDependencies') function tracked (name, version) { @@ -70,52 +81,70 @@ function getFundingInfo (idealTree, opts) { ) } + function addToFlatCache (funding, dep) { + [].concat(funding || []).forEach((f) => { + const key = f.url + if (!Array.isArray(flat[key])) { + flat[key] = [] + } + flat[key].push(dep) + }) + } + + function attachFundingInfo (target, funding, dep) { + if (funding && validFundingField(funding)) { + target.funding = retrieveFunding(funding) + if (!countOnly) { + addToFlatCache(target.funding, dep) + } + + packageWithFundingCount++ + } + } + function getFundingDependencies (tree) { const deps = tree && tree.dependencies if (!deps) return empty() - // broken into two steps to make sure items appearance - // within top levels takes precedence over nested ones - return (Object.keys(deps)).map((key) => { + const directDepsWithFunding = Object.keys(deps).map((key) => { const dep = deps[key] const { name, funding, version } = dep - const fundingItem = {} - // avoids duplicated items within the funding tree if (tracked(name, version)) return empty() + const fundingItem = {} + if (version) { fundingItem.version = version } - if (funding && validFundingUrl(funding)) { - fundingItem.funding = retrieveFunding(funding) - length++ - } + attachFundingInfo(fundingItem, funding, dep) return { dep, fundingItem } - }).reduce((res, { dep, fundingItem }, i) => { - if (!fundingItem) return res + }) + + return directDepsWithFunding.reduce((res, { dep: directDep, fundingItem }, i) => { + if (!fundingItem || fundingItem.length === 0) return res // recurse - const dependencies = dep.dependencies && - Object.keys(dep.dependencies).length > 0 && - getFundingDependencies(dep) + const transitiveDependencies = directDep.dependencies && + Object.keys(directDep.dependencies).length > 0 && + getFundingDependencies(directDep) // if we're only counting items there's no need // to add all the data to the resulting object if (countOnly) return null - if (hasDependencies(dependencies)) { - fundingItem.dependencies = retrieveDependencies(dependencies) + if (hasDependencies(transitiveDependencies)) { + fundingItem.dependencies = retrieveDependencies(transitiveDependencies) } - if (fundingItem.funding) { - res[dep.name] = fundingItem + if (fundingItem.funding && fundingItem.funding.length !== 0) { + res[directDep.name] = fundingItem } else if (fundingItem.dependencies) { res[_trailingDependencies] = Object.assign( @@ -126,12 +155,12 @@ function getFundingInfo (idealTree, opts) { } return res - }, empty()) + }, countOnly ? null : empty()) } const idealTreeDependencies = getFundingDependencies(idealTree) const result = { - length + length: packageWithFundingCount } if (!countOnly) { @@ -145,8 +174,9 @@ function getFundingInfo (idealTree, opts) { result.funding = retrieveFunding(idealTree.funding) } - result.dependencies = - retrieveDependencies(idealTreeDependencies) + result.dependencies = retrieveDependencies(idealTreeDependencies) + + result[flatCacheSymbol] = flat } return result diff --git a/tap-snapshots/test-tap-fund.js-TAP.test.js b/tap-snapshots/test-tap-fund.js-TAP.test.js index eb3183a27d7b4..963c59f3e1286 100644 --- a/tap-snapshots/test-tap-fund.js-TAP.test.js +++ b/tap-snapshots/test-tap-fund.js-TAP.test.js @@ -7,13 +7,12 @@ 'use strict' exports[`test/tap/fund.js TAP fund containing multi-level nested deps with no funding > should omit dependencies with no funding declared 1`] = ` nested-no-funding-packages@1.0.0 -+-- lorem@1.0.0 -| \`-- url: https://example.com/lorem -\`-- bar@1.0.0 - +-- type: individual - +-- url: http://example.com/donate ++-- https://example.com/lorem +| \`-- lorem@1.0.0 ++-- http://example.com/donate +| \`-- bar@1.0.0 +\`-- https://example.com/sponsor \`-- sub-bar@1.0.0 - \`-- url: https://example.com/sponsor ` @@ -24,20 +23,34 @@ exports[`test/tap/fund.js TAP fund does not support global > should throw EFUNDG exports[`test/tap/fund.js TAP fund does not support global > should write error msgs to stderr 1`] = ` npm ERR! code EFUNDGLOBAL -npm ERR! \`npm fund\` does not support globals +npm ERR! \`npm fund\` does not support global packages ` exports[`test/tap/fund.js TAP fund does not support global, using --json option > should write error msgs to stderr 1`] = ` npm ERR! code EFUNDGLOBAL -npm ERR! \`npm fund\` does not support globals +npm ERR! \`npm fund\` does not support global packages ` exports[`test/tap/fund.js TAP fund in which same maintainer owns all its deps > should print stack packages together 1`] = ` -maintainer-owns-all-deps@1.0.0, dep-bar@1.0.0, dep-sub-foo@1.0.0, dep-foo@1.0.0 -+-- type: individual -\`-- url: http://example.com/donate +maintainer-owns-all-deps@1.0.0 +\`-- http://example.com/donate + \`-- dep-bar@1.0.0, dep-foo@1.0.0, dep-sub-foo@1.0.0 +` + +exports[`test/tap/fund.js TAP fund using nested packages with multiple sources > should prompt with all available URLs 1`] = ` +1: Funding available at the following URL: https://one.example.com +2: Funding available at the following URL: https://two.example.com +Run \`npm fund [<@scope>/] --which=1\`, for example, to open the first funding URL listed in that package + +` + +exports[`test/tap/fund.js TAP fund using nested packages with multiple sources, with a source number > should open the numbered URL 1`] = ` +Funding available at the following URL: + +https://one.example.com + ` exports[`test/tap/fund.js TAP fund using package argument with no browser > should open funding url 1`] = ` diff --git a/test/tap/fund.js b/test/tap/fund.js index 97b414bf6e017..48d903f9897f6 100644 --- a/test/tap/fund.js +++ b/test/tap/fund.js @@ -14,6 +14,7 @@ const base = common.pkg const noFunding = path.join(base, 'no-funding-package') const maintainerOwnsAllDeps = path.join(base, 'maintainer-owns-all-deps') const nestedNoFundingPackages = path.join(base, 'nested-no-funding-packages') +const nestedMultipleFundingPackages = path.join(base, 'nested-multiple-funding-packages') const fundingStringShorthand = path.join(base, 'funding-string-shorthand') function getFixturePackage ({ name, version, dependencies, funding }, extras) { @@ -113,6 +114,37 @@ const fixture = new Tacks(Dir({ } }) }) + }), + 'nested-multiple-funding-packages': getFixturePackage({ + name: 'nested-multiple-funding-packages', + funding: [ + 'https://one.example.com', + 'https://two.example.com' + ], + dependencies: { + foo: '*' + }, + devDependencies: { + bar: '*' + } + }, { + node_modules: Dir({ + foo: getFixturePackage({ + name: 'foo', + funding: [ + 'http://example.com', + { url: 'http://sponsors.example.com/me' }, + 'http://collective.example.com' + ] + }), + bar: getFixturePackage({ + name: 'bar', + funding: [ + 'http://collective.example.com', + { url: 'http://sponsors.example.com/you' } + ] + }) + }) }) })) @@ -224,6 +256,54 @@ testFundCmd({ } }) +testFundCmd({ + title: 'fund containing multi-level nested deps with multiple funding sources, using --json option', + assertionMsg: 'should omit dependencies with no funding declared', + args: ['--json'], + opts: { cwd: nestedMultipleFundingPackages }, + assertion: jsonTest, + expected: { + length: 2, + name: 'nested-multiple-funding-packages', + version: '1.0.0', + funding: [ + { + url: 'https://one.example.com' + }, + { + url: 'https://two.example.com' + } + ], + dependencies: { + bar: { + version: '1.0.0', + funding: [ + { + url: 'http://collective.example.com' + }, + { + url: 'http://sponsors.example.com/you' + } + ] + }, + foo: { + version: '1.0.0', + funding: [ + { + url: 'http://example.com' + }, + { + url: 'http://sponsors.example.com/me' + }, + { + url: 'http://collective.example.com' + } + ] + } + } + } +}) + testFundCmd({ title: 'fund does not support global', assertionMsg: 'should throw EFUNDGLOBAL error', @@ -248,7 +328,7 @@ testFundCmd({ expected: { error: { code: 'EFUNDGLOBAL', - summary: '`npm fund` does not support globals', + summary: '`npm fund` does not support global packages', detail: '' } } @@ -268,6 +348,20 @@ testFundCmd({ opts: { cwd: fundingStringShorthand } }) +testFundCmd({ + title: 'fund using nested packages with multiple sources', + assertionMsg: 'should prompt with all available URLs', + args: ['.'], + opts: { cwd: nestedMultipleFundingPackages } +}) + +testFundCmd({ + title: 'fund using nested packages with multiple sources, with a source number', + assertionMsg: 'should open the numbered URL', + args: ['.', '--which=1', '--no-browser'], + opts: { cwd: nestedMultipleFundingPackages } +}) + testFundCmd({ title: 'fund using package argument with no browser, using --json option', assertionMsg: 'should open funding url', diff --git a/test/tap/utils.funding.js b/test/tap/utils.funding.js index 709762eacb7bb..4276f3e43a5cf 100644 --- a/test/tap/utils.funding.js +++ b/test/tap/utils.funding.js @@ -612,3 +612,46 @@ test('retrieve funding info string shorthand', (t) => { ) t.end() }) + +test('retrieve funding info from an array', (t) => { + t.deepEqual( + retrieveFunding([ + 'http://example.com', + { + url: 'http://two.example.com' + }, + 'http://three.example.com', + { + url: 'http://three.example.com', + type: 'dos' + }, + { + url: 'http://three.example.com', + type: 'third copy!', + extra: 'extra metadata!' + } + ]), + [ + { + url: 'http://example.com' + }, + { + url: 'http://two.example.com' + }, + { + url: 'http://three.example.com' + }, + { + url: 'http://three.example.com', + type: 'dos' + }, + { + url: 'http://three.example.com', + type: 'third copy!', + extra: 'extra metadata!' + } + ], + 'should accept and normalize multiple funding sources' + ) + t.end() +})