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

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 64894e291..5a751eec4 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 ccdbe041f..9af70ea0a 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 <projectname>`.
+`npm fund <projectname>` (when there are multiple URLs, the first one will be
+visited)
 
 ### files
 
diff --git a/lib/fund.js b/lib/fund.js
index 00954c844..e605ea8c3 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>/]<pkg>'
+  'npm fund [--browser] [[<@scope>/]<pkg> [--which=<fundingSourceNumber>]'
 )
 
 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>/]<pkg> --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>/]<pkg> [--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 c3d06b108..537563910 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 eb3183a27..963c59f3e 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,22 +23,36 @@ 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>/]<pkg> --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`] = `
 individual funding available at the following URL:
 
diff --git a/test/tap/fund.js b/test/tap/fund.js
index 97b414bf6..48d903f98 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' }
+        ]
+      })
+    })
   })
 }))
 
@@ -225,6 +257,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',
   args: ['--global'],
@@ -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: ''
     }
   }
@@ -269,6 +349,20 @@ testFundCmd({
 })
 
 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',
   args: ['.', '--json', '--no-browser'],
diff --git a/test/tap/utils.funding.js b/test/tap/utils.funding.js
index 709762eac..4276f3e43 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()
+})