From 1c65d26ac9f10ac0037094c207d216fbf0e969bf Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Tue, 19 Nov 2019 16:32:57 -0500 Subject: [PATCH] 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 ` This commit fixes the issue and adds the missing unit and integration tests covering that usecase. Fixes #498 PR-URL: https://github.com/npm/cli/pull/501 Credit: @ruyadorno Close: #501 Reviewed-by: @mikemimik --- lib/fund.js | 4 +- lib/utils/funding.js | 18 +++--- tap-snapshots/test-tap-fund.js-TAP.test.js | 7 +++ test/tap/fund.js | 15 +++++ test/tap/utils.funding.js | 69 +++++++++++++++++++++- 5 files changed, 102 insertions(+), 11 deletions(-) diff --git a/lib/fund.js b/lib/fund.js index 4981e461596c0..00954c844d798 100644 --- a/lib/fund.js +++ b/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' diff --git a/lib/utils/funding.js b/lib/utils/funding.js index dce40147642c5..c3d06b1089963 100644 --- a/lib/utils/funding.js +++ b/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() diff --git a/tap-snapshots/test-tap-fund.js-TAP.test.js b/tap-snapshots/test-tap-fund.js-TAP.test.js index e351a21c66919..eb3183a27d7b4 100644 --- a/tap-snapshots/test-tap-fund.js-TAP.test.js +++ b/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`] = ` no-funding-package@0.0.0 diff --git a/test/tap/fund.js b/test/tap/fund.js index 364dc1b6f8179..97b414bf6e017 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 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', diff --git a/test/tap/utils.funding.js b/test/tap/utils.funding.js index 51b89e5f8d340..709762eacb7bb 100644 --- a/test/tap/utils.funding.js +++ b/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() +})