fix(fund): open url for string shorthand

Trying to open url for a package that is using the string shorthand is
currently broken using: `npm fund <pkg>`

This commit fixes the issue and adds the missing unit and integration
tests covering that usecase.

Fixes #498

PR-URL: #501
Credit: @ruyadorno
Close: #501
Reviewed-by: @mikemimik

Author: ruyadorno

lib/fund.js

@@ -14,7 +14,7 @@ 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, validFundingUrl } = require('./utils/funding.js')
+const { getFundingInfo, retrieveFunding, validFundingUrl } = require('./utils/funding.js')
 
 const FundConfig = figgyPudding({
   browser: {}, // used by ./utils/open-url
@@ -132,7 +132,7 @@ function printHuman (fundingInfo, opts) {
 function openFundingUrl (packageName, cb) {
   function getUrlAndOpen (packageMetadata) {
     const { funding } = packageMetadata
-    const { type, url } = funding || {}
+    const { type, url } = retrieveFunding(funding) || {}
     const noFundingError =
       new Error(`No funding method available for: ${packageName}`)
     noFundingError.code = 'ENOFUND'

lib/utils/funding.js

@@ -3,8 +3,18 @@
 const URL = require('url').URL
 
 exports.getFundingInfo = getFundingInfo
+exports.retrieveFunding = retrieveFunding
 exports.validFundingUrl = validFundingUrl
 
+// supports both object funding and string shorthand
+function retrieveFunding (funding) {
+  return typeof funding === 'string'
+    ? {
+      url: funding
+    }
+    : funding
+}
+
 // Is the value of a `funding` property of a `package.json`
 // a valid type+url for `npm fund` to display?
 function validFundingUrl (funding) {
@@ -60,14 +70,6 @@ function getFundingInfo (idealTree, opts) {
     )
   }
 
-  function retrieveFunding (funding) {
-    return typeof funding === 'string'
-      ? {
-        url: funding
-      }
-      : funding
-  }
-
   function getFundingDependencies (tree) {
     const deps = tree && tree.dependencies
     if (!deps) return empty()

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

@@ -47,6 +47,13 @@ http://example.com/donate
 
 `
 
+exports[`test/tap/fund.js TAP fund using string shorthand > should open string-only url 1`] = `
+Funding available at the following URL:
+
+https://example.com/sponsor
+
+`
+
 exports[`test/tap/fund.js TAP fund with no package containing funding > should print empty funding info 1`] = `
 [email protected]
 

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 fundingStringShorthand = path.join(base, 'funding-string-shorthand')
 
 function getFixturePackage ({ name, version, dependencies, funding }, extras) {
   const getDeps = () => Object
@@ -36,6 +37,13 @@ function getFixturePackage ({ name, version, dependencies, funding }, extras) {
 }
 
 const fixture = new Tacks(Dir({
+  'funding-string-shorthand': Dir({
+    'package.json': File({
+      name: 'funding-string-shorthand',
+      version: '0.0.0',
+      funding: 'https://example.com/sponsor'
+    })
+  }),
   'no-funding-package': Dir({
     'package.json': File({
       name: 'no-funding-package',
@@ -253,6 +261,13 @@ testFundCmd({
   opts: { cwd: maintainerOwnsAllDeps }
 })
 
+testFundCmd({
+  title: 'fund using string shorthand',
+  assertionMsg: 'should open string-only url',
+  args: ['.', '--no-browser'],
+  opts: { cwd: fundingStringShorthand }
+})
+
 testFundCmd({
   title: 'fund using package argument with no browser, using --json option',
   assertionMsg: 'should open funding url',

test/tap/utils.funding.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { test } = require('tap')
-const { getFundingInfo } = require('../../lib/utils/funding')
+const { retrieveFunding, getFundingInfo } = require('../../lib/utils/funding')
 
 test('empty tree', (t) => {
   t.deepEqual(
@@ -545,3 +545,70 @@ test('handle different versions', (t) => {
   )
   t.end()
 })
+
+test('retrieve funding info from valid objects', (t) => {
+  t.deepEqual(
+    retrieveFunding({
+      url: 'http://example.com',
+      type: 'Foo'
+    }),
+    {
+      url: 'http://example.com',
+      type: 'Foo'
+    },
+    'should return standard object fields'
+  )
+  t.deepEqual(
+    retrieveFunding({
+      extra: 'Foo',
+      url: 'http://example.com',
+      type: 'Foo'
+    }),
+    {
+      extra: 'Foo',
+      url: 'http://example.com',
+      type: 'Foo'
+    },
+    'should leave untouched extra fields'
+  )
+  t.deepEqual(
+    retrieveFunding({
+      url: 'http://example.com'
+    }),
+    {
+      url: 'http://example.com'
+    },
+    'should accept url-only objects'
+  )
+  t.end()
+})
+
+test('retrieve funding info from invalid objects', (t) => {
+  t.deepEqual(
+    retrieveFunding({}),
+    {},
+    'should passthrough empty objects'
+  )
+  t.deepEqual(
+    retrieveFunding(),
+    undefined,
+    'should not care about undefined'
+  )
+  t.deepEqual(
+    retrieveFunding(),
+    null,
+    'should not care about null'
+  )
+  t.end()
+})
+
+test('retrieve funding info string shorthand', (t) => {
+  t.deepEqual(
+    retrieveFunding('http://example.com'),
+    {
+      url: 'http://example.com'
+    },
+    'should accept string shorthand'
+  )
+  t.end()
+})