Skip to content

Commit

Permalink
fix(fund): open url for string shorthand
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ruyadorno committed Nov 19, 2019
1 parent b829d62 commit 23f2411
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 11 deletions.
4 changes: 2 additions & 2 deletions lib/fund.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down
18 changes: 10 additions & 8 deletions lib/utils/funding.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down
7 changes: 7 additions & 0 deletions tap-snapshots/test-tap-fund.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
15 changes: 15 additions & 0 deletions test/tap/fund.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
69 changes: 68 additions & 1 deletion test/tap/utils.funding.js
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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()
})

0 comments on commit 23f2411

Please sign in to comment.