From c757289ec1f66b97a61e686ba3adf53c274c6a7c Mon Sep 17 00:00:00 2001 From: "Kyle E. Mitchell" Date: Fri, 30 Aug 2019 09:10:35 -0700 Subject: [PATCH 1/5] support: add support subcommand PR-URL: https://github.com/npm/cli/pull/246 Credit: @kemitchell Close: #246 Reviewed-by: @ruyadorno Thanks @kemitchell for providing the initial work that served as a base for `npm fund`, its original commits messages are preserved as such: - support: add support subcommand - support: fix request caching - support: further sanitize contributor data - doc: Fix typo - support: simplify to just collecting and showing URLs - install: improve `npm support` test - install: drop "the" before "projects you depend on" - doc: Reword mention of `npm support` in `package.json` spec --- doc/files/package.json.md | 10 ++++ lib/config/cmd-list.js | 1 + lib/install.js | 15 ++++- lib/support.js | 88 +++++++++++++++++++++++++++++ lib/utils/valid-support-url.js | 19 +++++++ package-lock.json | 20 ++++--- test/tap/install-mention-support.js | 39 +++++++++++++ test/tap/support.js | 77 +++++++++++++++++++++++++ 8 files changed, 259 insertions(+), 10 deletions(-) create mode 100644 lib/support.js create mode 100644 lib/utils/valid-support-url.js create mode 100644 test/tap/install-mention-support.js create mode 100644 test/tap/support.js diff --git a/doc/files/package.json.md b/doc/files/package.json.md index 6324caf64a517..19d4704d0752d 100644 --- a/doc/files/package.json.md +++ b/doc/files/package.json.md @@ -170,6 +170,16 @@ Both email and url are optional either way. npm also sets a top-level "maintainers" field with your npm user info. +## support + +You can specify a URL for up-to-date information about ways to support +development of your package: + + { "support": "https://example.com/project/support" } + +Users can use the `npm support` subcommand to list the `support` URLs +of all dependencies of the project, direct and indirect. + ## files The optional `files` field is an array of file patterns that describes diff --git a/lib/config/cmd-list.js b/lib/config/cmd-list.js index fa4390fcdcba7..c11c8c30982b8 100644 --- a/lib/config/cmd-list.js +++ b/lib/config/cmd-list.js @@ -91,6 +91,7 @@ var cmdList = [ 'token', 'profile', 'audit', + 'support', 'org', 'help', diff --git a/lib/install.js b/lib/install.js index 8cc6d16bdd169..52fe96c47f5ff 100644 --- a/lib/install.js +++ b/lib/install.js @@ -119,6 +119,7 @@ var unlock = locker.unlock var parseJSON = require('./utils/parse-json.js') var output = require('./utils/output.js') var saveMetrics = require('./utils/metrics.js').save +var validSupportURL = require('./utils/valid-support-url') // install specific libraries var copyTree = require('./install/copy-tree.js') @@ -802,6 +803,8 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { var added = 0 var updated = 0 var moved = 0 + // Check if any installed packages have support properties. + var haveSupportable = false // Count the number of contributors to packages added, tracking // contributors we've seen, so we can produce a running unique count. var contributors = new Set() @@ -809,6 +812,11 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { var mutation = action[0] var pkg = action[1] if (pkg.failed) return + if ( + mutation !== 'remove' && validSupportURL(pkg.package.support) + ) { + haveSupportable = true + } if (mutation === 'remove') { ++removed } else if (mutation === 'move') { @@ -872,7 +880,12 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { report += ' in ' + ((Date.now() - this.started) / 1000) + 's' output(report) - return auditResult && audit.printInstallReport(auditResult) + if (haveSupportable) { + output('Run `npm support` to support projects you depend on.') + } + if (auditResult) { + audit.printInstallReport(auditResult) + } function packages (num) { return num + ' package' + (num > 1 ? 's' : '') diff --git a/lib/support.js b/lib/support.js new file mode 100644 index 0000000000000..5813df93ff2a6 --- /dev/null +++ b/lib/support.js @@ -0,0 +1,88 @@ +'use strict' + +const npm = require('./npm.js') +const output = require('./utils/output.js') +const path = require('path') +const readPackageTree = require('read-package-tree') +const semver = require('semver') +const validSupportURL = require('./utils/valid-support-url') + +module.exports = support + +const usage = require('./utils/usage') +support.usage = usage( + 'support', + '\nnpm support [--json]' +) + +support.completion = function (opts, cb) { + const argv = opts.conf.argv.remain + switch (argv[2]) { + case 'support': + return cb(null, []) + default: + return cb(new Error(argv[2] + ' not recognized')) + } +} + +// Compare lib/ls.js. +function support (args, silent, cb) { + if (typeof cb !== 'function') { + cb = silent + silent = false + } + const dir = path.resolve(npm.dir, '..') + readPackageTree(dir, function (err, tree) { + if (err) { + process.exitCode = 1 + return cb(err) + } + const data = findPackages(tree) + if (silent) return cb(null, data) + var out + if (npm.config.get('json')) { + out = JSON.stringify(data, null, 2) + } else { + out = data.map(displayPackage).join('\n\n') + } + output(out) + cb(err, data) + }) +} + +function findPackages (root) { + const set = new Set() + iterate(root) + return Array.from(set).sort(function (a, b) { + const comparison = a.name + .toLowerCase() + .localeCompare(b.name.toLowerCase()) + return comparison === 0 + ? semver.compare(a.version, b.version) + : comparison + }) + + function iterate (node) { + node.children.forEach(recurse) + } + + function recurse (node) { + const metadata = node.package + const support = metadata.support + if (support && validSupportURL(support)) { + set.add({ + name: metadata.name, + version: metadata.version, + path: node.path, + homepage: metadata.homepage, + repository: metadata.repository, + support: metadata.support + }) + } + if (node.children) iterate(node) + } +} + +function displayPackage (entry) { + return entry.name + '@' + entry.version + ': ' + entry.support +} diff --git a/lib/utils/valid-support-url.js b/lib/utils/valid-support-url.js new file mode 100644 index 0000000000000..d575dcdf03b52 --- /dev/null +++ b/lib/utils/valid-support-url.js @@ -0,0 +1,19 @@ +const URL = require('url').URL + +// Is the value of a `support` property of a `package.json` object +// a valid URL for `npm support` to display? +module.exports = function (argument) { + if (typeof argument !== 'string' || argument.length === 0) { + return false + } + try { + var parsed = new URL(argument) + } catch (error) { + return false + } + if ( + parsed.protocol !== 'https:' && + parsed.protocol !== 'http:' + ) return false + return parsed.host +} diff --git a/package-lock.json b/package-lock.json index 92e0c660ba44b..763567fc2b36a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -438,6 +438,17 @@ "has-ansi": "^2.0.0", "strip-ansi": "^3.0.0", "supports-color": "^2.0.0" + }, + "dependencies": { + "has-ansi": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/has-ansi/-/has-ansi-2.0.0.tgz", + "integrity": "sha1-NPUEnOHs3ysGSa8+8k5F7TVBbZE=", + "dev": true, + "requires": { + "ansi-regex": "^2.0.0" + } + } } }, "supports-color": { @@ -2426,15 +2437,6 @@ "function-bind": "^1.1.1" } }, - "has-ansi": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/has-ansi/-/has-ansi-2.0.0.tgz", - "integrity": "sha1-NPUEnOHs3ysGSa8+8k5F7TVBbZE=", - "dev": true, - "requires": { - "ansi-regex": "^2.0.0" - } - }, "has-flag": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", diff --git a/test/tap/install-mention-support.js b/test/tap/install-mention-support.js new file mode 100644 index 0000000000000..a1cb2c4aaf7d7 --- /dev/null +++ b/test/tap/install-mention-support.js @@ -0,0 +1,39 @@ +'use strict' +var test = require('tap').test +var Tacks = require('tacks') +var Dir = Tacks.Dir +var File = Tacks.File +var common = require('../common-tap.js') + +var fixturepath = common.pkg +var fixture = new Tacks(Dir({ + 'package.json': File({}), + 'hassupport': Dir({ + 'package.json': File({ + name: 'hassupport', + version: '7.7.7', + support: 'http://example.com/project/support' + }) + }) +})) + +test('setup', function (t) { + fixture.remove(fixturepath) + fixture.create(fixturepath) + t.end() +}) + +test('install-report', function (t) { + common.npm(['install', '--no-save', './hassupport'], {cwd: fixturepath}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'installed successfully') + t.is(stderr, '', 'no warnings') + t.includes(stdout, '`npm support`', 'mentions `npm support`') + t.end() + }) +}) + +test('cleanup', function (t) { + fixture.remove(fixturepath) + t.end() +}) diff --git a/test/tap/support.js b/test/tap/support.js new file mode 100644 index 0000000000000..93d4887423a13 --- /dev/null +++ b/test/tap/support.js @@ -0,0 +1,77 @@ +'use strict' +var test = require('tap').test +var Tacks = require('tacks') +var path = require('path') +var Dir = Tacks.Dir +var File = Tacks.File +var common = require('../common-tap.js') + +var fixturepath = common.pkg +var fixture = new Tacks(Dir({ + 'package.json': File({ + name: 'a', + version: '0.0.0', + dependencies: { 'hassupport': '7.7.7' } + }), + 'node_modules': Dir({ + hassupport: Dir({ + 'package.json': File({ + name: 'hassupport', + version: '7.7.7', + homepage: 'http://example.com/project', + support: 'http://example.com/project/donate' + }) + }) + }) +})) + +test('setup', function (t) { + fixture.remove(fixturepath) + fixture.create(fixturepath) + t.end() +}) + +test('support --json', function (t) { + common.npm(['support', '--json'], {cwd: fixturepath}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'exited 0') + t.is(stderr, '', 'no warnings') + var parsed + t.doesNotThrow(function () { + parsed = JSON.parse(stdout) + }, 'valid JSON') + t.deepEqual( + parsed, + [ + { + name: 'hassupport', + version: '7.7.7', + homepage: 'http://example.com/project', + support: 'http://example.com/project/donate', + path: path.resolve(fixturepath, 'node_modules', 'hassupport') + } + ], + 'output data' + ) + t.end() + }) +}) + +test('support', function (t) { + common.npm(['support'], {cwd: fixturepath}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'exited 0') + t.is(stderr, '', 'no warnings') + t.includes(stdout, 'hassupport', 'outputs project name') + t.includes(stdout, '7.7.7', 'outputs project version') + t.includes(stdout, 'http://example.com/project', 'outputs contributor homepage') + t.includes(stdout, 'http://example.com/project/donate', 'outputs support link') + t.end() + }) +}) + +test('cleanup', function (t) { + t.pass(fixturepath) + fixture.remove(fixturepath) + t.end() +}) From 24baa17b6ee234815283e0615398297021a6afac Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Wed, 23 Oct 2019 17:23:24 -0400 Subject: [PATCH 2/5] fund: add fund command This commit introduces the `npm fund` command that lists all `funding` info provided by the installed dependencies of a given project. Notes on implementation: - `lib/utils/funding.js` Provides helpers to validate funding info and return a tree-shaped structure containing the funding data for all deps. - `lib/fund.js` Implements `npm fund ` command - Added tests - `npm install` mention of funding - `npm fund ` variations - unit tests for added `lib/utils` and `lib/install` helpers - Added docs for `npm fund`, `funding` `package.json` property - Fixed `lib/utils/open-url` to support `--json` config - Documented `unicode` on `npm install` docs Refs: https://github.com/npm/rfcs/blob/2d2f00457ab19b3003eb6ac5ab3d250259fd5a81/accepted/0017-add-funding-support.md --- doc/cli/npm-fund.md | 51 +++ doc/cli/npm-install.md | 5 + doc/cli/npm-ls.md | 8 + doc/files/package.json.md | 22 +- doc/misc/npm-config.md | 9 + lib/config/cmd-list.js | 2 +- lib/config/defaults.js | 3 + lib/fund.js | 202 +++++++++ lib/install.js | 31 +- lib/install/fund.js | 48 +++ lib/support.js | 88 ---- lib/utils/funding.js | 145 +++++++ lib/utils/open-url.js | 9 +- lib/utils/valid-support-url.js | 19 - package-lock.json | 20 +- tap-snapshots/test-tap-fund.js-TAP.test.js | 126 ++++++ tap-snapshots/test-tap-repo.js-TAP.test.js | 14 + test/tap/fund.js | 211 +++++++++ test/tap/install-mention-funding.js | 127 ++++++ test/tap/install-mention-support.js | 39 -- test/tap/install.fund.js | 100 +++++ test/tap/repo.js | 18 + test/tap/support.js | 77 ---- test/tap/utils.funding.js | 480 +++++++++++++++++++++ 24 files changed, 1601 insertions(+), 253 deletions(-) create mode 100644 doc/cli/npm-fund.md create mode 100644 lib/fund.js create mode 100644 lib/install/fund.js delete mode 100644 lib/support.js create mode 100644 lib/utils/funding.js delete mode 100644 lib/utils/valid-support-url.js create mode 100644 tap-snapshots/test-tap-fund.js-TAP.test.js create mode 100644 tap-snapshots/test-tap-repo.js-TAP.test.js create mode 100644 test/tap/fund.js create mode 100644 test/tap/install-mention-funding.js delete mode 100644 test/tap/install-mention-support.js create mode 100644 test/tap/install.fund.js delete mode 100644 test/tap/support.js create mode 100644 test/tap/utils.funding.js diff --git a/doc/cli/npm-fund.md b/doc/cli/npm-fund.md new file mode 100644 index 0000000000000..cbd848a53b1a5 --- /dev/null +++ b/doc/cli/npm-fund.md @@ -0,0 +1,51 @@ +npm-fund(1) -- Retrieve funding information +======================================================== + +## SYNOPSIS + + npm fund [] + +## DESCRIPTION + +This command retrieves information on how to fund the dependencies of +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. + +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 +list is not going to have the same shape of the output from `npm ls`. + +## CONFIGURATION + +### browser + +* Default: OS X: `"open"`, Windows: `"start"`, Others: `"xdg-open"` +* Type: String + +The browser that is called by the `npm fund` command to open websites. + +### json + +* Default: false +* Type: Boolean + +Show information in JSON format. + +### unicode + +* Type: Boolean +* Default: true + +Whether to represent the tree structure using unicode characters. +Set it to `false` in order to use all-ansi output. + +## SEE ALSO + +* npm-docs(1) +* npm-config(1) +* npm-install(1) +* npm-ls(1) + diff --git a/doc/cli/npm-install.md b/doc/cli/npm-install.md index 4ff4a47cbcf4e..640dc882b07d6 100644 --- a/doc/cli/npm-install.md +++ b/doc/cli/npm-install.md @@ -326,6 +326,10 @@ local copy exists on disk. npm install sax --force +The `--no-fund` argument will hide the message displayed at the end of each +install that aknowledges the number of dependencies looking for funding. +See `npm-fund(1)` + The `-g` or `--global` argument will cause npm to install the package globally rather than locally. See `npm-folders(5)`. @@ -443,6 +447,7 @@ affects a real use-case, it will be investigated. * npm-folders(5) * npm-update(1) * npm-audit(1) +* npm-fund(1) * npm-link(1) * npm-rebuild(1) * npm-scripts(7) diff --git a/doc/cli/npm-ls.md b/doc/cli/npm-ls.md index 7b10a19d69b2c..055dd763f3d16 100644 --- a/doc/cli/npm-ls.md +++ b/doc/cli/npm-ls.md @@ -98,6 +98,14 @@ When "prod" or "production", is an alias to `production`. Display only dependencies which are linked +### unicode + +* Type: Boolean +* Default: true + +Whether to represent the tree structure using unicode characters. +Set it to false in order to use all-ansi output. + ## SEE ALSO * npm-config(1) diff --git a/doc/files/package.json.md b/doc/files/package.json.md index 19d4704d0752d..ee018f190ff2f 100644 --- a/doc/files/package.json.md +++ b/doc/files/package.json.md @@ -170,15 +170,25 @@ Both email and url are optional either way. npm also sets a top-level "maintainers" field with your npm user info. -## support +## funding -You can specify a URL for up-to-date information about ways to support -development of your package: +You can specify an object containing an URL that provides up-to-date +information about ways to help fund development of your package: - { "support": "https://example.com/project/support" } + "funding": { + "type" : "individual", + "url" : "http://example.com/donate" + } + + "funding": { + "type" : "patreon", + "url" : "https://www.patreon.com/my-account" + } -Users can use the `npm support` subcommand to list the `support` URLs -of all dependencies of the project, direct and indirect. +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 `. ## files diff --git a/doc/misc/npm-config.md b/doc/misc/npm-config.md index f1055a56edbc7..49694581c5090 100644 --- a/doc/misc/npm-config.md +++ b/doc/misc/npm-config.md @@ -426,6 +426,15 @@ packages. The "maxTimeout" config for the `retry` module to use when fetching packages. +### fund + +* Default: true +* Type: Boolean + +When "true" displays the message at the end of each `npm install` +aknowledging the number of dependencies looking for funding. +See `npm-fund(1)` for details. + ### git * Default: `"git"` diff --git a/lib/config/cmd-list.js b/lib/config/cmd-list.js index c11c8c30982b8..d9d0d85b7d520 100644 --- a/lib/config/cmd-list.js +++ b/lib/config/cmd-list.js @@ -91,7 +91,7 @@ var cmdList = [ 'token', 'profile', 'audit', - 'support', + 'fund', 'org', 'help', diff --git a/lib/config/defaults.js b/lib/config/defaults.js index 57d373df1e10c..e07da3aaf97f4 100644 --- a/lib/config/defaults.js +++ b/lib/config/defaults.js @@ -143,6 +143,8 @@ Object.defineProperty(exports, 'defaults', {get: function () { force: false, 'format-package-lock': true, + fund: true, + 'fetch-retries': 2, 'fetch-retry-factor': 10, 'fetch-retry-mintimeout': 10000, @@ -284,6 +286,7 @@ exports.types = { editor: String, 'engine-strict': Boolean, force: Boolean, + fund: Boolean, 'format-package-lock': Boolean, 'fetch-retries': Number, 'fetch-retry-factor': Number, diff --git a/lib/fund.js b/lib/fund.js new file mode 100644 index 0000000000000..4981e461596c0 --- /dev/null +++ b/lib/fund.js @@ -0,0 +1,202 @@ +'use strict' + +const path = require('path') + +const archy = require('archy') +const figgyPudding = require('figgy-pudding') +const readPackageTree = require('read-package-tree') + +const npm = require('./npm.js') +const npmConfig = require('./config/figgy-config.js') +const fetchPackageMetadata = require('./fetch-package-metadata.js') +const computeMetadata = require('./install/deps.js').computeMetadata +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 FundConfig = figgyPudding({ + browser: {}, // used by ./utils/open-url + global: {}, + json: {}, + unicode: {} +}) + +module.exports = fundCmd + +const usage = require('./utils/usage') +fundCmd.usage = usage( + 'fund', + 'npm fund [--json]', + 'npm fund [--browser] [[<@scope>/]' +) + +fundCmd.completion = function (opts, cb) { + const argv = opts.conf.argv.remain + switch (argv[2]) { + case 'fund': + return cb(null, []) + default: + return cb(new Error(argv[2] + ' not recognized')) + } +} + +function printJSON (fundingInfo) { + return JSON.stringify(fundingInfo, null, 2) +} + +// the human-printable version does some special things that turned out to +// be very verbose but hopefully not hard to follow: we stack up items +// that have a shared url/type and make sure they're printed at the highest +// 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() + + 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) + } + + function retrieveStackedItem (funding) { + const key = seenKey(funding) + if (key && seen.has(key)) return seen.get(key) + } + + // --- + + 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 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 archy(result, '', { unicode: opts.unicode }) +} + +function openFundingUrl (packageName, cb) { + function getUrlAndOpen (packageMetadata) { + const { funding } = packageMetadata + const { type, url } = 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)) { + openUrl(url, msg, cb) + } else { + throw noFundingError + } + } + + fetchPackageMetadata( + packageName, + '.', + { fullMetadata: true }, + function (err, packageMetadata) { + if (err) return cb(err) + getUrlAndOpen(packageMetadata) + } + ) +} + +function fundCmd (args, cb) { + const opts = FundConfig(npmConfig()) + const dir = path.resolve(npm.dir, '..') + const packageName = args[0] + + if (opts.global) { + const err = new Error('`npm fund` does not support globals') + err.code = 'EFUNDGLOBAL' + throw err + } + + if (packageName) { + openFundingUrl(packageName, cb) + return + } + + readPackageTree(dir, function (err, tree) { + if (err) { + process.exitCode = 1 + return cb(err) + } + + readShrinkwrap.andInflate(tree, function () { + const fundingInfo = getFundingInfo( + mutateIntoLogicalTree.asReadInstalled( + computeMetadata(tree) + ) + ) + + const print = opts.json + ? printJSON + : printHuman + + output( + print( + fundingInfo, + opts + ) + ) + cb(err, tree) + }) + }) +} diff --git a/lib/install.js b/lib/install.js index 52fe96c47f5ff..a4cf2b186de51 100644 --- a/lib/install.js +++ b/lib/install.js @@ -119,7 +119,6 @@ var unlock = locker.unlock var parseJSON = require('./utils/parse-json.js') var output = require('./utils/output.js') var saveMetrics = require('./utils/metrics.js').save -var validSupportURL = require('./utils/valid-support-url') // install specific libraries var copyTree = require('./install/copy-tree.js') @@ -139,6 +138,10 @@ var validateArgs = require('./install/validate-args.js') var saveRequested = require('./install/save.js').saveRequested var saveShrinkwrap = require('./install/save.js').saveShrinkwrap var audit = require('./install/audit.js') +var { + getPrintFundingReport, + getPrintFundingReportJSON +} = require('./install/fund.js') var getSaveType = require('./install/save.js').getSaveType var doSerialActions = require('./install/actions.js').doSerial var doReverseSerialActions = require('./install/actions.js').doReverseSerial @@ -241,6 +244,7 @@ function Installer (where, dryrun, args, opts) { this.saveOnlyLock = opts.saveOnlyLock this.global = opts.global != null ? opts.global : this.where === path.resolve(npm.globalDir, '..') this.audit = npm.config.get('audit') && !this.global + this.fund = npm.config.get('fund') && !this.global this.started = Date.now() } Installer.prototype = {} @@ -803,8 +807,6 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { var added = 0 var updated = 0 var moved = 0 - // Check if any installed packages have support properties. - var haveSupportable = false // Count the number of contributors to packages added, tracking // contributors we've seen, so we can produce a running unique count. var contributors = new Set() @@ -812,11 +814,6 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { var mutation = action[0] var pkg = action[1] if (pkg.failed) return - if ( - mutation !== 'remove' && validSupportURL(pkg.package.support) - ) { - haveSupportable = true - } if (mutation === 'remove') { ++removed } else if (mutation === 'move') { @@ -880,9 +877,6 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { report += ' in ' + ((Date.now() - this.started) / 1000) + 's' output(report) - if (haveSupportable) { - output('Run `npm support` to support projects you depend on.') - } if (auditResult) { audit.printInstallReport(auditResult) } @@ -907,9 +901,23 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { if (argument.url) returned += ' (' + argument.email + ')' return returned } + + const { fund, idealTree } = this + const printFundingReport = getPrintFundingReport({ + fund, + idealTree + }) + if (printFundingReport.length) { + output(printFundingReport) + } } Installer.prototype.printInstalledForJSON = function (diffs, auditResult) { + const { fund, idealTree } = this + const printFundingReport = getPrintFundingReportJSON({ + fund, + idealTree + }) var result = { added: [], removed: [], @@ -918,6 +926,7 @@ Installer.prototype.printInstalledForJSON = function (diffs, auditResult) { failed: [], warnings: [], audit: auditResult, + funding: printFundingReport, elapsed: Date.now() - this.started } var self = this diff --git a/lib/install/fund.js b/lib/install/fund.js new file mode 100644 index 0000000000000..55a167a95583a --- /dev/null +++ b/lib/install/fund.js @@ -0,0 +1,48 @@ +'use strict' + +const { EOL } = require('os') + +const computeMetadata = require('./deps.js').computeMetadata +const mutateIntoLogicalTree = require('./mutate-into-logical-tree.js') +var { getFundingInfo } = require('../utils/funding.js') + +exports.getPrintFundingReport = getPrintFundingReport +exports.getPrintFundingReportJSON = getPrintFundingReportJSON + +function getFundingResult ({ fund, idealTree }) { + if (fund) { + const fundingInfoTree = + mutateIntoLogicalTree.asReadInstalled( + computeMetadata(idealTree) + ) + const fundResult = getFundingInfo(fundingInfoTree, { countOnly: true }) + return fundResult + } else { + return {} + } +} + +function getPrintFundingReport ({ fund, idealTree }, opts) { + const fundResult = getFundingResult({ fund, idealTree }) + const { length } = fundResult || {} + const { json } = opts || {} + + function padding (msg) { + return json ? '' : (EOL + msg) + } + + function packageQuantity (amount) { + return `package${amount > 1 ? 's are' : ' is'}` + } + + if (!length) return '' + + return padding('') + length + ' ' + + packageQuantity(length) + + ' looking for funding.' + + padding('Run "npm fund" to find out more.') +} + +function getPrintFundingReportJSON ({ fund, idealTree }) { + return getPrintFundingReport({ fund, idealTree }, { json: true }) +} diff --git a/lib/support.js b/lib/support.js deleted file mode 100644 index 5813df93ff2a6..0000000000000 --- a/lib/support.js +++ /dev/null @@ -1,88 +0,0 @@ -'use strict' - -const npm = require('./npm.js') -const output = require('./utils/output.js') -const path = require('path') -const readPackageTree = require('read-package-tree') -const semver = require('semver') -const validSupportURL = require('./utils/valid-support-url') - -module.exports = support - -const usage = require('./utils/usage') -support.usage = usage( - 'support', - '\nnpm support [--json]' -) - -support.completion = function (opts, cb) { - const argv = opts.conf.argv.remain - switch (argv[2]) { - case 'support': - return cb(null, []) - default: - return cb(new Error(argv[2] + ' not recognized')) - } -} - -// Compare lib/ls.js. -function support (args, silent, cb) { - if (typeof cb !== 'function') { - cb = silent - silent = false - } - const dir = path.resolve(npm.dir, '..') - readPackageTree(dir, function (err, tree) { - if (err) { - process.exitCode = 1 - return cb(err) - } - const data = findPackages(tree) - if (silent) return cb(null, data) - var out - if (npm.config.get('json')) { - out = JSON.stringify(data, null, 2) - } else { - out = data.map(displayPackage).join('\n\n') - } - output(out) - cb(err, data) - }) -} - -function findPackages (root) { - const set = new Set() - iterate(root) - return Array.from(set).sort(function (a, b) { - const comparison = a.name - .toLowerCase() - .localeCompare(b.name.toLowerCase()) - return comparison === 0 - ? semver.compare(a.version, b.version) - : comparison - }) - - function iterate (node) { - node.children.forEach(recurse) - } - - function recurse (node) { - const metadata = node.package - const support = metadata.support - if (support && validSupportURL(support)) { - set.add({ - name: metadata.name, - version: metadata.version, - path: node.path, - homepage: metadata.homepage, - repository: metadata.repository, - support: metadata.support - }) - } - if (node.children) iterate(node) - } -} - -function displayPackage (entry) { - return entry.name + '@' + entry.version + ': ' + entry.support -} diff --git a/lib/utils/funding.js b/lib/utils/funding.js new file mode 100644 index 0000000000000..5ea690164e802 --- /dev/null +++ b/lib/utils/funding.js @@ -0,0 +1,145 @@ +'use strict' + +const URL = require('url').URL + +exports.getFundingInfo = getFundingInfo +exports.validFundingUrl = validFundingUrl + +// Is the value of a `funding` property of a `package.json` +// a valid type+url for `npm fund` to display? +function validFundingUrl (funding) { + if (!funding.url) { + return false + } + + try { + var parsed = new URL(funding.url) + } catch (error) { + return false + } + + if ( + parsed.protocol !== 'https:' && + parsed.protocol !== 'http:' + ) return false + + return Boolean(parsed.host) +} + +function getFundingInfo (idealTree, opts) { + let length = 0 + const seen = new Set() + const { countOnly } = opts || {} + const empty = () => Object.create(null) + const _trailingDependencies = Symbol('trailingDependencies') + + function tracked (name, version) { + const key = String(name) + String(version) + if (seen.has(key)) { + return true + } + seen.add(key) + } + + function retrieveDependencies (dependencies) { + const trailing = dependencies[_trailingDependencies] + + if (trailing) { + return Object.assign( + empty(), + dependencies, + trailing + ) + } + + return dependencies + } + + function hasDependencies (dependencies) { + return dependencies && ( + Object.keys(dependencies).length || + dependencies[_trailingDependencies] + ) + } + + 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 dep = deps[key] + const { name, funding, version } = dep + + const fundingItem = {} + + // avoids duplicated items within the funding tree + if (tracked(name, version)) return empty() + + if (version) { + fundingItem.version = version + } + + if (funding && validFundingUrl(funding)) { + fundingItem.funding = funding + length++ + } + + return { + dep, + fundingItem + } + }).reduce((res, { dep, fundingItem }, i) => { + if (!fundingItem) return res + + // recurse + const dependencies = dep.dependencies && + Object.keys(dep.dependencies).length > 0 && + getFundingDependencies(dep) + + // 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 (fundingItem.funding) { + res[dep.name] = fundingItem + } else if (fundingItem.dependencies) { + res[_trailingDependencies] = + Object.assign( + empty(), + res[_trailingDependencies], + fundingItem.dependencies + ) + } + + return res + }, empty()) + } + + const idealTreeDependencies = getFundingDependencies(idealTree) + const result = { + length + } + + if (!countOnly) { + result.name = idealTree.name || idealTree.path + + if (idealTree && idealTree.version) { + result.version = idealTree.version + } + + if (idealTree && idealTree.funding) { + result.funding = idealTree.funding + } + + result.dependencies = + retrieveDependencies(idealTreeDependencies) + } + + return result +} diff --git a/lib/utils/open-url.js b/lib/utils/open-url.js index 7a48d2e868959..b8f21b6cdbaf6 100644 --- a/lib/utils/open-url.js +++ b/lib/utils/open-url.js @@ -7,7 +7,14 @@ const opener = require('opener') module.exports = function open (url, errMsg, cb, browser = npm.config.get('browser')) { opener(url, { command: npm.config.get('browser') }, (er) => { if (er && er.code === 'ENOENT') { - output(`${errMsg}:\n\n${url}`) + const alternateMsg = npm.config.get('json') + ? JSON.stringify({ + title: errMsg, + url + }, null, 2) + : `${errMsg}:\n\n${url}` + + output(alternateMsg) return cb() } else { return cb(er) diff --git a/lib/utils/valid-support-url.js b/lib/utils/valid-support-url.js deleted file mode 100644 index d575dcdf03b52..0000000000000 --- a/lib/utils/valid-support-url.js +++ /dev/null @@ -1,19 +0,0 @@ -const URL = require('url').URL - -// Is the value of a `support` property of a `package.json` object -// a valid URL for `npm support` to display? -module.exports = function (argument) { - if (typeof argument !== 'string' || argument.length === 0) { - return false - } - try { - var parsed = new URL(argument) - } catch (error) { - return false - } - if ( - parsed.protocol !== 'https:' && - parsed.protocol !== 'http:' - ) return false - return parsed.host -} diff --git a/package-lock.json b/package-lock.json index 763567fc2b36a..92e0c660ba44b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -438,17 +438,6 @@ "has-ansi": "^2.0.0", "strip-ansi": "^3.0.0", "supports-color": "^2.0.0" - }, - "dependencies": { - "has-ansi": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/has-ansi/-/has-ansi-2.0.0.tgz", - "integrity": "sha1-NPUEnOHs3ysGSa8+8k5F7TVBbZE=", - "dev": true, - "requires": { - "ansi-regex": "^2.0.0" - } - } } }, "supports-color": { @@ -2437,6 +2426,15 @@ "function-bind": "^1.1.1" } }, + "has-ansi": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/has-ansi/-/has-ansi-2.0.0.tgz", + "integrity": "sha1-NPUEnOHs3ysGSa8+8k5F7TVBbZE=", + "dev": true, + "requires": { + "ansi-regex": "^2.0.0" + } + }, "has-flag": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", diff --git a/tap-snapshots/test-tap-fund.js-TAP.test.js b/tap-snapshots/test-tap-fund.js-TAP.test.js new file mode 100644 index 0000000000000..cf0680dd576ab --- /dev/null +++ b/tap-snapshots/test-tap-fund.js-TAP.test.js @@ -0,0 +1,126 @@ +/* IMPORTANT + * This snapshot file is auto-generated, but designed for humans. + * It should be checked into source control and tracked carefully. + * Re-generate by setting TAP_SNAPSHOT=1 and running tests. + * Make sure to inspect the output below. Do not ignore changes! + */ +'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 + └─┬ sub-bar@1.0.0 + └── url: https://example.com/sponsor + + +` + +exports[`test/tap/fund.js TAP fund containing multi-level nested deps with no funding, using --json option > should omit dependencies with no funding declared 1`] = ` +{ + length: 3, + name: 'nested-no-funding-packages', + version: '1.0.0', + dependencies: { + lorem: { version: '1.0.0', funding: { url: 'https://example.com/lorem' } }, + bar: { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' }, + dependencies: { + 'sub-bar': { + version: '1.0.0', + funding: { url: 'https://example.com/sponsor' } + } + } + } + } +} +` + +exports[`test/tap/fund.js TAP fund does not support global > should throw EFUNDGLOBAL error 1`] = ` + +` + +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 +` + +exports[`test/tap/fund.js TAP fund does not support global, using --json option > should throw EFUNDGLOBAL error 1`] = ` +{ + error: { + code: 'EFUNDGLOBAL', + summary: '\`npm fund\` does not support globals', + detail: '' + } +} +` + +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 +` + +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 + + +` + +exports[`test/tap/fund.js TAP fund in which same maintainer owns all its deps, using --json option > should print stack packages together 1`] = ` +{ + length: 3, + name: 'maintainer-owns-all-deps', + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' }, + dependencies: { + 'dep-bar': { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' } + }, + 'dep-foo': { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' }, + dependencies: { + 'dep-sub-foo': { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' } + } + } + } + } +} +` + +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: + +http://example.com/donate + +` + +exports[`test/tap/fund.js TAP fund using package argument with no browser, using --json option > should open funding url 1`] = ` +{ + title: 'individual funding available at the following URL', + url: 'http://example.com/donate' +} +` + +exports[`test/tap/fund.js TAP fund with no package containing funding > should print empty funding info 1`] = ` +no-funding-package@0.0.0 + + +` + +exports[`test/tap/fund.js TAP fund with no package containing funding, using --json option > should print empty funding info 1`] = ` +{ + length: 0, + name: 'no-funding-package', + version: '0.0.0', + dependencies: {} +} +` diff --git a/tap-snapshots/test-tap-repo.js-TAP.test.js b/tap-snapshots/test-tap-repo.js-TAP.test.js new file mode 100644 index 0000000000000..408119c37b137 --- /dev/null +++ b/tap-snapshots/test-tap-repo.js-TAP.test.js @@ -0,0 +1,14 @@ +/* IMPORTANT + * This snapshot file is auto-generated, but designed for humans. + * It should be checked into source control and tracked carefully. + * Re-generate by setting TAP_SNAPSHOT=1 and running tests. + * Make sure to inspect the output below. Do not ignore changes! + */ +'use strict' +exports[`test/tap/repo.js TAP npm repo underscore --json > should print json result 1`] = ` +{ + "title": "repository available at the following URL", + "url": "https://github.com/jashkenas/underscore" +} + +` diff --git a/test/tap/fund.js b/test/tap/fund.js new file mode 100644 index 0000000000000..f6c028440facf --- /dev/null +++ b/test/tap/fund.js @@ -0,0 +1,211 @@ +'use strict' + +const fs = require('fs') +const path = require('path') + +const test = require('tap').test +const Tacks = require('tacks') +const Dir = Tacks.Dir +const File = Tacks.File +const common = require('../common-tap.js') + +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') + +function getFixturePackage ({ name, version, dependencies, funding }, extras) { + const getDeps = () => Object + .keys(dependencies) + .reduce((res, dep) => (Object.assign({}, res, { + [dep]: '*' + })), {}) + + return Dir(Object.assign({ + 'package.json': File({ + name, + version: version || '1.0.0', + funding: (funding === undefined) ? { + type: 'individual', + url: 'http://example.com/donate' + } : funding, + dependencies: dependencies && getDeps(dependencies) + }) + }, extras)) +} + +const fixture = new Tacks(Dir({ + 'no-funding-package': Dir({ + 'package.json': File({ + name: 'no-funding-package', + version: '0.0.0' + }) + }), + 'maintainer-owns-all-deps': getFixturePackage({ + name: 'maintainer-owns-all-deps', + dependencies: { + 'dep-foo': '*', + 'dep-bar': '*' + } + }, { + node_modules: Dir({ + 'dep-foo': getFixturePackage({ + name: 'dep-foo', + dependencies: { + 'dep-sub-foo': '*' + } + }, { + node_modules: Dir({ + 'dep-sub-foo': getFixturePackage({ + name: 'dep-sub-foo' + }) + }) + }), + 'dep-bar': getFixturePackage({ + name: 'dep-bar' + }) + }) + }), + 'nested-no-funding-packages': getFixturePackage({ + name: 'nested-no-funding-packages', + funding: null, + dependencies: { + foo: '*' + }, + devDependencies: { + lorem: '*' + } + }, { + node_modules: Dir({ + foo: getFixturePackage({ + name: 'foo', + dependencies: { + bar: '*' + }, + funding: null + }, { + node_modules: Dir({ + bar: getFixturePackage({ + name: 'bar' + }, { + node_modules: Dir({ + 'sub-bar': getFixturePackage({ + name: 'sub-bar', + funding: { + url: 'https://example.com/sponsor' + } + }) + }) + }) + }) + }), + lorem: getFixturePackage({ + name: 'lorem', + funding: { + url: 'https://example.com/lorem' + } + }) + }) + }) +})) + +function checkOutput (t, { code, stdout, stderr }) { + t.is(code, 0, `exited code 0`) + t.is(stderr, '', 'no warnings') +} + +function testFundCmd ({ title, expect, args = [], opts = {}, output = checkOutput }) { + const validate = (t, json) => (err, code, stdout, stderr) => { + if (err) throw err + + output(t, { code, stdout, stderr }) + + let parsed = stdout + if (json) { + t.doesNotThrow(function () { + parsed = JSON.parse(stdout) + }, 'valid JSON') + } + + t.matchSnapshot(parsed, expect) + t.end() + } + + test(title, (t) => { + common.npm(['fund'].concat(args), opts, validate(t)) + }) + + test(`${title}, using --json option`, (t) => { + common.npm(['fund', '--json'].concat(args), opts, validate(t, true)) + }) +} + +test('setup', function (t) { + fixture.remove(base) + fixture.create(base) + t.end() +}) + +testFundCmd({ + title: 'fund with no package containing funding', + expect: 'should print empty funding info', + opts: { cwd: noFunding } +}) + +testFundCmd({ + title: 'fund in which same maintainer owns all its deps', + expect: 'should print stack packages together', + opts: { cwd: maintainerOwnsAllDeps } +}) + +testFundCmd({ + title: 'fund containing multi-level nested deps with no funding', + expect: 'should omit dependencies with no funding declared', + opts: { cwd: nestedNoFundingPackages } +}) + +testFundCmd({ + title: 'fund does not support global', + expect: 'should throw EFUNDGLOBAL error', + args: ['--global'], + output: (t, { code, stdout, stderr }) => { + t.is(code, 1, `exited code 0`) + const [ errCode, errCmd ] = stderr.split('\n') + t.matchSnapshot(`${errCode}\n${errCmd}`, 'should write error msgs to stderr') + } +}) + +testFundCmd({ + title: 'fund using package argument with no browser', + expect: 'should open funding url', + args: ['--browser', 'undefined', '.'], + opts: { cwd: maintainerOwnsAllDeps } +}) + +test('fund using package argument', function (t) { + const fakeBrowser = path.join(common.pkg, '_script.sh') + const outFile = path.join(common.pkg, '_output') + + const s = '#!/usr/bin/env bash\n' + + 'echo "$@" > ' + JSON.stringify(common.pkg) + '/_output\n' + fs.writeFileSync(fakeBrowser, s) + fs.chmodSync(fakeBrowser, '0755') + + common.npm([ + 'fund', '.', + '--loglevel=silent', + '--browser=' + fakeBrowser + ], { cwd: maintainerOwnsAllDeps }, function (err, code, stdout, stderr) { + t.ifError(err, 'repo command ran without error') + t.equal(code, 0, 'exit ok') + var res = fs.readFileSync(outFile, 'utf8') + t.equal(res, 'http://example.com/donate\n') + t.end() + }) +}) + +test('cleanup', function (t) { + t.pass(base) + fixture.remove(base) + t.end() +}) diff --git a/test/tap/install-mention-funding.js b/test/tap/install-mention-funding.js new file mode 100644 index 0000000000000..ebd25a57324c1 --- /dev/null +++ b/test/tap/install-mention-funding.js @@ -0,0 +1,127 @@ +'use strict' +const path = require('path') +const test = require('tap').test +const Tacks = require('tacks') +const Dir = Tacks.Dir +const File = Tacks.File +const common = require('../common-tap.js') + +const base = common.pkg +const singlePackage = path.join(base, 'single-funding-package') +const multiplePackages = path.join(base, 'top-level-funding') + +function getFixturePackage ({ name, version, dependencies, funding }) { + return Dir({ + 'package.json': File({ + name, + version: version || '1.0.0', + funding: funding || { + type: 'individual', + url: 'http://example.com/donate' + }, + dependencies: dependencies || {} + }) + }) +} + +const fixture = new Tacks(Dir({ + 'package.json': File({}), + 'single-funding-package': getFixturePackage({ + name: 'single-funding-package' + }), + 'top-level-funding': getFixturePackage({ + name: 'top-level-funding', + dependencies: { + 'dep-foo': 'file:../dep-foo', + 'dep-bar': 'file:../dep-bar' + } + }), + 'dep-foo': getFixturePackage({ + name: 'dep-foo', + funding: { + type: 'corporate', + url: 'https://corp.example.com/sponsor' + }, + dependencies: { + 'sub-dep-bar': 'file:../sub-dep-bar' + } + }), + 'dep-bar': getFixturePackage({ + name: 'dep-bar', + version: '2.1.0', + dependencies: { + 'sub-dep-bar': 'file:../sub-dep-bar' + } + }), + 'sub-dep-bar': getFixturePackage({ + name: 'sub-dep-bar', + funding: { + type: 'foo', + url: 'http://example.com/foo' + } + }) +})) + +test('mention npm fund upon installing single dependency', function (t) { + setup(t) + common.npm(['install', '--no-save', singlePackage], {cwd: base}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'installed successfully') + t.is(stderr, '', 'no warnings') + t.includes(stdout, '1 package is looking for funding.', 'should print amount of packages needing funding') + t.includes(stdout, 'Run "npm fund" to find out more.', 'should print npm fund mention') + t.end() + }) +}) + +test('mention npm fund upon installing multiple dependencies', function (t) { + setup(t) + common.npm(['install', '--no-save', multiplePackages], {cwd: base}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'installed successfully') + t.is(stderr, '', 'no warnings') + t.includes(stdout, '4 packages are looking for funding.', 'should print amount of packages needing funding') + t.includes(stdout, 'Run "npm fund" to find out more.', 'should print npm fund mention') + t.end() + }) +}) + +test('skips mention npm fund using --no-fund option', function (t) { + setup(t) + common.npm(['install', '--no-save', '--no-fund', multiplePackages], {cwd: base}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'installed successfully') + t.is(stderr, '', 'no warnings') + t.doesNotHave(stdout, '4 packages are looking for funding.', 'should print amount of packages needing funding') + t.doesNotHave(stdout, 'Run "npm fund" to find out more.', 'should print npm fund mention') + t.end() + }) +}) + +test('mention packages looking for funding using --json', function (t) { + setup(t) + common.npm(['install', '--no-save', '--json', multiplePackages], {cwd: base}, function (err, code, stdout, stderr) { + if (err) throw err + t.is(code, 0, 'installed successfully') + t.is(stderr, '', 'no warnings') + const res = JSON.parse(stdout) + t.match(res.funding, '4 packages are looking for funding.', 'should print amount of packages needing funding') + t.end() + }) +}) + +test('cleanup', function (t) { + cleanup() + t.end() +}) + +function setup (t) { + fixture.create(base) + t.teardown(() => { + cleanup() + }) +} + +function cleanup () { + fixture.remove(base) +} diff --git a/test/tap/install-mention-support.js b/test/tap/install-mention-support.js deleted file mode 100644 index a1cb2c4aaf7d7..0000000000000 --- a/test/tap/install-mention-support.js +++ /dev/null @@ -1,39 +0,0 @@ -'use strict' -var test = require('tap').test -var Tacks = require('tacks') -var Dir = Tacks.Dir -var File = Tacks.File -var common = require('../common-tap.js') - -var fixturepath = common.pkg -var fixture = new Tacks(Dir({ - 'package.json': File({}), - 'hassupport': Dir({ - 'package.json': File({ - name: 'hassupport', - version: '7.7.7', - support: 'http://example.com/project/support' - }) - }) -})) - -test('setup', function (t) { - fixture.remove(fixturepath) - fixture.create(fixturepath) - t.end() -}) - -test('install-report', function (t) { - common.npm(['install', '--no-save', './hassupport'], {cwd: fixturepath}, function (err, code, stdout, stderr) { - if (err) throw err - t.is(code, 0, 'installed successfully') - t.is(stderr, '', 'no warnings') - t.includes(stdout, '`npm support`', 'mentions `npm support`') - t.end() - }) -}) - -test('cleanup', function (t) { - fixture.remove(fixturepath) - t.end() -}) diff --git a/test/tap/install.fund.js b/test/tap/install.fund.js new file mode 100644 index 0000000000000..37a61e42891af --- /dev/null +++ b/test/tap/install.fund.js @@ -0,0 +1,100 @@ +'use strict' + +const { EOL } = require('os') +const { test } = require('tap') +const { getPrintFundingReport } = require('../../lib/install/fund') + +test('message when there are no funding found', (t) => { + t.deepEqual( + getPrintFundingReport({}), + '', + 'should not print any message if missing info' + ) + t.deepEqual( + getPrintFundingReport({ + name: 'foo', + version: '1.0.0', + dependencies: {} + }), + '', + 'should not print any message if package has no dependencies' + ) + t.deepEqual( + getPrintFundingReport({ + fund: true, + idealTree: { + name: 'foo', + version: '1.0.0', + dependencies: { + bar: {}, + lorem: {} + } + } + }), + '', + 'should not print any message if no package has funding info' + ) + t.end() +}) + +test('print appropriate message for a single package', (t) => { + t.deepEqual( + getPrintFundingReport({ + fund: true, + idealTree: { + name: 'foo', + version: '1.0.0', + children: [ + { + package: { + name: 'bar', + version: '1.0.0', + funding: { type: 'foo', url: 'http://example.com' } + } + } + ] + } + }), + `${EOL}1 package is looking for funding.${EOL}Run "npm fund" to find out more.`, + 'should print single package message' + ) + t.end() +}) + +test('print appropriate message for many packages', (t) => { + t.deepEqual( + getPrintFundingReport({ + fund: true, + idealTree: { + name: 'foo', + version: '1.0.0', + children: [ + { + package: { + name: 'bar', + version: '1.0.0', + funding: { type: 'foo', url: 'http://example.com' } + } + }, + { + package: { + name: 'lorem', + version: '1.0.0', + funding: { type: 'foo', url: 'http://example.com' } + } + }, + { + package: { + name: 'ipsum', + version: '1.0.0', + funding: { type: 'foo', url: 'http://example.com' } + } + } + ] + } + }), + `${EOL}3 packages are looking for funding.${EOL}Run "npm fund" to find out more.`, + 'should print many package message' + ) + t.end() +}) diff --git a/test/tap/repo.js b/test/tap/repo.js index 0ee50af192cb1..e68acb5b979ca 100644 --- a/test/tap/repo.js +++ b/test/tap/repo.js @@ -41,6 +41,24 @@ test('npm repo underscore', function (t) { }) }) +test('npm repo underscore --json', function (t) { + mr({ port: common.port }, function (er, s) { + common.npm([ + 'repo', 'underscore', + '--json', + '--registry=' + common.registry, + '--loglevel=silent', + '--browser=undefined' + ], opts, function (err, code, stdout, stderr) { + t.ifError(err, 'repo command ran without error') + t.equal(code, 0, 'exit ok') + t.matchSnapshot(stdout, 'should print json result') + s.close() + t.end() + }) + }) +}) + test('npm repo optimist - github (https://)', function (t) { mr({ port: common.port }, function (er, s) { common.npm([ diff --git a/test/tap/support.js b/test/tap/support.js deleted file mode 100644 index 93d4887423a13..0000000000000 --- a/test/tap/support.js +++ /dev/null @@ -1,77 +0,0 @@ -'use strict' -var test = require('tap').test -var Tacks = require('tacks') -var path = require('path') -var Dir = Tacks.Dir -var File = Tacks.File -var common = require('../common-tap.js') - -var fixturepath = common.pkg -var fixture = new Tacks(Dir({ - 'package.json': File({ - name: 'a', - version: '0.0.0', - dependencies: { 'hassupport': '7.7.7' } - }), - 'node_modules': Dir({ - hassupport: Dir({ - 'package.json': File({ - name: 'hassupport', - version: '7.7.7', - homepage: 'http://example.com/project', - support: 'http://example.com/project/donate' - }) - }) - }) -})) - -test('setup', function (t) { - fixture.remove(fixturepath) - fixture.create(fixturepath) - t.end() -}) - -test('support --json', function (t) { - common.npm(['support', '--json'], {cwd: fixturepath}, function (err, code, stdout, stderr) { - if (err) throw err - t.is(code, 0, 'exited 0') - t.is(stderr, '', 'no warnings') - var parsed - t.doesNotThrow(function () { - parsed = JSON.parse(stdout) - }, 'valid JSON') - t.deepEqual( - parsed, - [ - { - name: 'hassupport', - version: '7.7.7', - homepage: 'http://example.com/project', - support: 'http://example.com/project/donate', - path: path.resolve(fixturepath, 'node_modules', 'hassupport') - } - ], - 'output data' - ) - t.end() - }) -}) - -test('support', function (t) { - common.npm(['support'], {cwd: fixturepath}, function (err, code, stdout, stderr) { - if (err) throw err - t.is(code, 0, 'exited 0') - t.is(stderr, '', 'no warnings') - t.includes(stdout, 'hassupport', 'outputs project name') - t.includes(stdout, '7.7.7', 'outputs project version') - t.includes(stdout, 'http://example.com/project', 'outputs contributor homepage') - t.includes(stdout, 'http://example.com/project/donate', 'outputs support link') - t.end() - }) -}) - -test('cleanup', function (t) { - t.pass(fixturepath) - fixture.remove(fixturepath) - t.end() -}) diff --git a/test/tap/utils.funding.js b/test/tap/utils.funding.js new file mode 100644 index 0000000000000..60a7a1e67eb5a --- /dev/null +++ b/test/tap/utils.funding.js @@ -0,0 +1,480 @@ +'use strict' + +const { test } = require('tap') +const { getFundingInfo } = require('../../lib/utils/funding') + +test('empty tree', (t) => { + t.deepEqual( + getFundingInfo({}), + { + name: null, + dependencies: {}, + length: 0 + }, + 'should return empty list' + ) + t.end() +}) + +test('single item missing funding', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + dependencies: { + 'single-item': { + name: 'single-item', + version: '1.0.0' + } + }}), + { + name: 'project', + dependencies: {}, + length: 0 + }, + 'should return empty list' + ) + t.end() +}) + +test('use path if name is missing', (t) => { + t.deepEqual( + getFundingInfo({ name: undefined, + path: '/tmp/foo', + children: { + 'single-item': { + name: 'single-item', + version: '1.0.0' + } + }}), + { + name: '/tmp/foo', + dependencies: {}, + length: 0 + }, + 'should use path as top level name' + ) + t.end() +}) + +test('single item tree', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + dependencies: { + 'single-item': { + name: 'single-item', + version: '1.0.0', + funding: { + type: 'foo', + url: 'http://example.com' + } + } + }}), + { + name: 'project', + dependencies: { + 'single-item': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'http://example.com' + } + } + }, + length: 1 + }, + 'should return list with a single item' + ) + t.end() +}) + +test('duplicate items along the tree', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + version: '2.3.4', + dependencies: { + 'single-item': { + name: 'single-item', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'shared-top-first': { + name: 'shared-top-first', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + }, + 'sub-dep': { + name: 'sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'shared-nested-first': { + name: 'shared-nested-first', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'shared-top-first': { + name: 'shared-top-first', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + } + }, + 'shared-nested-first': { + name: 'shared-nested-first', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + }}), + { + name: 'project', + version: '2.3.4', + dependencies: { + 'single-item': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'shared-top-first': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + }, + 'sub-dep': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + }, + 'shared-nested-first': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + }, + length: 4 + }, + 'should return list with a single item' + ) + t.end() +}) + +test('multi-level nested items tree', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + dependencies: { + 'first-level-dep': { + name: 'first-level-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'sub-dep': { + name: 'sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + package: { + name: 'sub-sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: {} + } + } + } + } + } + }}), + { + name: 'project', + dependencies: { + 'first-level-dep': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'sub-dep': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'sub-sub-dep': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + } + } + }, + length: 3 + }, + 'should return list with all items' + ) + t.end() +}) + +test('missing fund nested items tree', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + dependencies: { + 'first-level-dep': { + name: 'first-level-dep', + version: '1.0.0', + funding: { + type: 'foo' + }, + dependencies: { + 'sub-dep': { + name: 'sub-dep', + version: '1.0.0', + dependencies: { + 'sub-sub-dep-01': { + name: 'sub-sub-dep-01', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'non-funding-child': { + name: 'non-funding-child', + version: '1.0.0', + dependencies: { + 'sub-sub-sub-dep': { + name: 'sub-sub-sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + } + }, + 'sub-sub-dep-02': { + name: 'sub-sub-dep-02', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: {} + }, + 'sub-sub-dep-03': { + name: 'sub-sub-dep-03', + version: '1.0.0', + funding: { + type: 'foo', + url: 'git://example.git' + }, + dependencies: { + 'sub-sub-sub-dep-03': { + name: 'sub-sub-sub-dep-03', + version: '1.0.0', + dependencies: { + 'sub-sub-sub-sub-dep': { + name: 'sub-sub-sub-sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'http://example.com' + } + } + } + } + } + } + } + } + } + } + }}), + { + name: 'project', + dependencies: { + 'sub-sub-dep-01': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'sub-sub-sub-dep': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + }, + 'sub-sub-dep-02': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + }, + 'sub-sub-sub-sub-dep': { + version: '1.0.0', + funding: { + type: 'foo', + url: 'http://example.com' + } + } + }, + length: 4 + }, + 'should return list excluding missing funding items' + ) + t.end() +}) + +test('countOnly option', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + dependencies: { + 'first-level-dep': { + name: 'first-level-dep', + version: '1.0.0', + funding: { + type: 'foo' + }, + dependencies: { + 'sub-dep': { + name: 'sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'sub-sub-dep': { + name: 'sub-sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + }, + dependencies: {} + } + }, + 'sub-sub-dep': { + name: 'sub-sub-dep', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + }}, { countOnly: true }), + { + length: 2 + }, + 'should return only the length property' + ) + t.end() +}) + +test('handle different versions', (t) => { + t.deepEqual( + getFundingInfo({ name: 'project', + dependencies: { + foo: { + name: 'foo', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + bar: { + name: 'bar', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + }, + lorem: { + dependencies: { + fooo: { + name: 'foo', + version: '2.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + }, + dependencies: { + 'foo-bar': { + name: 'foo-bar', + version: '1.0.0', + funding: { + type: 'foo', + url: 'https://example.com' + } + } + } + } + } + } + } + }, { countOnly: true }), + { + length: 4 + }, + 'should treat different versions as diff packages' + ) + t.end() +}) From ebda4caefd52632d70c4d7d2f6ddce91aa632df3 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Thu, 31 Oct 2019 11:04:06 -0400 Subject: [PATCH 3/5] fix tests --- tap-snapshots/test-tap-fund.js-TAP.test.js | 90 ++------------ test/tap/fund.js | 130 +++++++++++++++++---- 2 files changed, 119 insertions(+), 101 deletions(-) diff --git a/tap-snapshots/test-tap-fund.js-TAP.test.js b/tap-snapshots/test-tap-fund.js-TAP.test.js index cf0680dd576ab..e351a21c66919 100644 --- a/tap-snapshots/test-tap-fund.js-TAP.test.js +++ b/tap-snapshots/test-tap-fund.js-TAP.test.js @@ -7,38 +7,17 @@ '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 - └─┬ sub-bar@1.0.0 - └── url: https://example.com/sponsor ++-- lorem@1.0.0 +| \`-- url: https://example.com/lorem +\`-- bar@1.0.0 + +-- type: individual + +-- url: http://example.com/donate + \`-- sub-bar@1.0.0 + \`-- url: https://example.com/sponsor ` -exports[`test/tap/fund.js TAP fund containing multi-level nested deps with no funding, using --json option > should omit dependencies with no funding declared 1`] = ` -{ - length: 3, - name: 'nested-no-funding-packages', - version: '1.0.0', - dependencies: { - lorem: { version: '1.0.0', funding: { url: 'https://example.com/lorem' } }, - bar: { - version: '1.0.0', - funding: { type: 'individual', url: 'http://example.com/donate' }, - dependencies: { - 'sub-bar': { - version: '1.0.0', - funding: { url: 'https://example.com/sponsor' } - } - } - } - } -} -` - exports[`test/tap/fund.js TAP fund does not support global > should throw EFUNDGLOBAL error 1`] = ` ` @@ -48,16 +27,6 @@ npm ERR! code EFUNDGLOBAL npm ERR! \`npm fund\` does not support globals ` -exports[`test/tap/fund.js TAP fund does not support global, using --json option > should throw EFUNDGLOBAL error 1`] = ` -{ - error: { - code: 'EFUNDGLOBAL', - summary: '\`npm fund\` does not support globals', - detail: '' - } -} -` - 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 @@ -65,35 +34,10 @@ npm ERR! \`npm fund\` does not support globals 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 - ++-- type: individual +\`-- url: http://example.com/donate -` -exports[`test/tap/fund.js TAP fund in which same maintainer owns all its deps, using --json option > should print stack packages together 1`] = ` -{ - length: 3, - name: 'maintainer-owns-all-deps', - version: '1.0.0', - funding: { type: 'individual', url: 'http://example.com/donate' }, - dependencies: { - 'dep-bar': { - version: '1.0.0', - funding: { type: 'individual', url: 'http://example.com/donate' } - }, - 'dep-foo': { - version: '1.0.0', - funding: { type: 'individual', url: 'http://example.com/donate' }, - dependencies: { - 'dep-sub-foo': { - version: '1.0.0', - funding: { type: 'individual', url: 'http://example.com/donate' } - } - } - } - } -} ` exports[`test/tap/fund.js TAP fund using package argument with no browser > should open funding url 1`] = ` @@ -103,24 +47,8 @@ http://example.com/donate ` -exports[`test/tap/fund.js TAP fund using package argument with no browser, using --json option > should open funding url 1`] = ` -{ - title: 'individual funding available at the following URL', - url: 'http://example.com/donate' -} -` - exports[`test/tap/fund.js TAP fund with no package containing funding > should print empty funding info 1`] = ` no-funding-package@0.0.0 ` - -exports[`test/tap/fund.js TAP fund with no package containing funding, using --json option > should print empty funding info 1`] = ` -{ - length: 0, - name: 'no-funding-package', - version: '0.0.0', - dependencies: {} -} -` diff --git a/test/tap/fund.js b/test/tap/fund.js index f6c028440facf..97fa31ce29d82 100644 --- a/test/tap/fund.js +++ b/test/tap/fund.js @@ -114,29 +114,32 @@ function checkOutput (t, { code, stdout, stderr }) { t.is(stderr, '', 'no warnings') } -function testFundCmd ({ title, expect, args = [], opts = {}, output = checkOutput }) { - const validate = (t, json) => (err, code, stdout, stderr) => { +function jsonTest (t, { assertionMsg, expected, stdout }) { + let parsed = stdout + + t.doesNotThrow(function () { + parsed = JSON.parse(stdout) + }, 'valid JSON') + + t.deepEqual(parsed, expected, assertionMsg) +} + +function snapshotTest (t, { stdout, assertionMsg }) { + t.matchSnapshot(stdout, assertionMsg) +} + +function testFundCmd ({ title, assertionMsg, args = [], opts = {}, output = checkOutput, assertion = snapshotTest, expected }) { + const validate = (t) => (err, code, stdout, stderr) => { if (err) throw err output(t, { code, stdout, stderr }) + assertion(t, { assertionMsg, expected, stdout }) - let parsed = stdout - if (json) { - t.doesNotThrow(function () { - parsed = JSON.parse(stdout) - }, 'valid JSON') - } - - t.matchSnapshot(parsed, expect) t.end() } test(title, (t) => { - common.npm(['fund'].concat(args), opts, validate(t)) - }) - - test(`${title}, using --json option`, (t) => { - common.npm(['fund', '--json'].concat(args), opts, validate(t, true)) + common.npm(['fund', '--unicode=false'].concat(args), opts, validate(t)) }) } @@ -148,25 +151,81 @@ test('setup', function (t) { testFundCmd({ title: 'fund with no package containing funding', - expect: 'should print empty funding info', + assertionMsg: 'should print empty funding info', opts: { cwd: noFunding } }) testFundCmd({ title: 'fund in which same maintainer owns all its deps', - expect: 'should print stack packages together', + assertionMsg: 'should print stack packages together', opts: { cwd: maintainerOwnsAllDeps } }) +testFundCmd({ + title: 'fund in which same maintainer owns all its deps, using --json option', + assertionMsg: 'should print stack packages together', + args: ['--json'], + opts: { cwd: maintainerOwnsAllDeps }, + assertion: jsonTest, + expected: { + length: 3, + name: 'maintainer-owns-all-deps', + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' }, + dependencies: { + 'dep-bar': { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' } + }, + 'dep-foo': { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' }, + dependencies: { + 'dep-sub-foo': { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' } + } + } + } + } + } +}) + testFundCmd({ title: 'fund containing multi-level nested deps with no funding', - expect: 'should omit dependencies with no funding declared', + assertionMsg: 'should omit dependencies with no funding declared', opts: { cwd: nestedNoFundingPackages } }) +testFundCmd({ + title: 'fund containing multi-level nested deps with no funding, using --json option', + assertionMsg: 'should omit dependencies with no funding declared', + args: ['--json'], + opts: { cwd: nestedNoFundingPackages }, + assertion: jsonTest, + expected: { + length: 3, + name: 'nested-no-funding-packages', + version: '1.0.0', + dependencies: { + lorem: { version: '1.0.0', funding: { url: 'https://example.com/lorem' } }, + bar: { + version: '1.0.0', + funding: { type: 'individual', url: 'http://example.com/donate' }, + dependencies: { + 'sub-bar': { + version: '1.0.0', + funding: { url: 'https://example.com/sponsor' } + } + } + } + } + } +}) + testFundCmd({ title: 'fund does not support global', - expect: 'should throw EFUNDGLOBAL error', + assertionMsg: 'should throw EFUNDGLOBAL error', args: ['--global'], output: (t, { code, stdout, stderr }) => { t.is(code, 1, `exited code 0`) @@ -175,13 +234,44 @@ testFundCmd({ } }) +testFundCmd({ + title: 'fund does not support global, using --json option', + assertionMsg: 'should throw EFUNDGLOBAL error', + args: ['--global', '--json'], + output: (t, { code, stdout, stderr }) => { + t.is(code, 1, `exited code 0`) + const [ errCode, errCmd ] = stderr.split('\n') + t.matchSnapshot(`${errCode}\n${errCmd}`, 'should write error msgs to stderr') + }, + assertion: jsonTest, + expected: { + error: { + code: 'EFUNDGLOBAL', + summary: '\`npm fund\` does not support globals', + detail: '' + } + } +}) + testFundCmd({ title: 'fund using package argument with no browser', - expect: 'should open funding url', + assertionMsg: 'should open funding url', args: ['--browser', 'undefined', '.'], opts: { cwd: maintainerOwnsAllDeps } }) +testFundCmd({ + title: 'fund using package argument with no browser, using --json option', + assertionMsg: 'should open funding url', + args: ['--json', '--browser', 'undefined', '.'], + opts: { cwd: maintainerOwnsAllDeps }, + assertion: jsonTest, + expected: { + title: 'individual funding available at the following URL', + url: 'http://example.com/donate' + } +}) + test('fund using package argument', function (t) { const fakeBrowser = path.join(common.pkg, '_script.sh') const outFile = path.join(common.pkg, '_output') From 2f9f22cf4673f94f4021186e50101a11e5834935 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Thu, 31 Oct 2019 14:12:33 -0400 Subject: [PATCH 4/5] fix planned tap tests --- test/tap/fund.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/test/tap/fund.js b/test/tap/fund.js index 97fa31ce29d82..e7e1fe4c504bb 100644 --- a/test/tap/fund.js +++ b/test/tap/fund.js @@ -115,12 +115,7 @@ function checkOutput (t, { code, stdout, stderr }) { } function jsonTest (t, { assertionMsg, expected, stdout }) { - let parsed = stdout - - t.doesNotThrow(function () { - parsed = JSON.parse(stdout) - }, 'valid JSON') - + let parsed = JSON.parse(stdout) t.deepEqual(parsed, expected, assertionMsg) } @@ -134,11 +129,10 @@ function testFundCmd ({ title, assertionMsg, args = [], opts = {}, output = chec output(t, { code, stdout, stderr }) assertion(t, { assertionMsg, expected, stdout }) - - t.end() } - test(title, (t) => { + return test(title, (t) => { + t.plan(3) common.npm(['fund', '--unicode=false'].concat(args), opts, validate(t)) }) } From fd1856db6afd37db6356e2b0685c0231cf8d2379 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Thu, 31 Oct 2019 19:21:33 -0400 Subject: [PATCH 5/5] alternative solution to --no-browser arg --- lib/utils/funding.js | 2 +- lib/utils/open-url.js | 30 ++++++++++----- tap-snapshots/test-tap-repo.js-TAP.test.js | 7 ++++ test/tap/fund.js | 45 ++++++++++++---------- test/tap/repo.js | 19 ++++++++- 5 files changed, 71 insertions(+), 32 deletions(-) diff --git a/lib/utils/funding.js b/lib/utils/funding.js index 5ea690164e802..2c994e0b6b426 100644 --- a/lib/utils/funding.js +++ b/lib/utils/funding.js @@ -8,7 +8,7 @@ exports.validFundingUrl = validFundingUrl // Is the value of a `funding` property of a `package.json` // a valid type+url for `npm fund` to display? function validFundingUrl (funding) { - if (!funding.url) { + if (!funding || !funding.url) { return false } diff --git a/lib/utils/open-url.js b/lib/utils/open-url.js index b8f21b6cdbaf6..e1ed2b3fab76d 100644 --- a/lib/utils/open-url.js +++ b/lib/utils/open-url.js @@ -5,16 +5,28 @@ const opener = require('opener') // attempt to open URL in web-browser, print address otherwise: module.exports = function open (url, errMsg, cb, browser = npm.config.get('browser')) { - opener(url, { command: npm.config.get('browser') }, (er) => { - if (er && er.code === 'ENOENT') { - const alternateMsg = npm.config.get('json') - ? JSON.stringify({ - title: errMsg, - url - }, null, 2) - : `${errMsg}:\n\n${url}` + function printAlternateMsg () { + const json = npm.config.get('json') + const alternateMsg = json + ? JSON.stringify({ + title: errMsg, + url + }, null, 2) + : `${errMsg}:\n\n${url}` + + output(alternateMsg) + } + + const skipBrowser = process.argv.indexOf('--no-browser') > -1 - output(alternateMsg) + if (skipBrowser) { + printAlternateMsg() + return cb() + } + + opener(url, { command: browser }, (er) => { + if (er && er.code === 'ENOENT') { + printAlternateMsg() return cb() } else { return cb(er) diff --git a/tap-snapshots/test-tap-repo.js-TAP.test.js b/tap-snapshots/test-tap-repo.js-TAP.test.js index 408119c37b137..3fba79edb8d42 100644 --- a/tap-snapshots/test-tap-repo.js-TAP.test.js +++ b/tap-snapshots/test-tap-repo.js-TAP.test.js @@ -12,3 +12,10 @@ exports[`test/tap/repo.js TAP npm repo underscore --json > should print json res } ` + +exports[`test/tap/repo.js TAP npm repo underscore --no-browser > should print alternative msg 1`] = ` +repository available at the following URL: + +https://github.com/jashkenas/underscore + +` diff --git a/test/tap/fund.js b/test/tap/fund.js index e7e1fe4c504bb..cc66bea51a056 100644 --- a/test/tap/fund.js +++ b/test/tap/fund.js @@ -8,6 +8,7 @@ const Tacks = require('tacks') const Dir = Tacks.Dir const File = Tacks.File const common = require('../common-tap.js') +const isWindows = require('../../lib/utils/is-windows.js') const base = common.pkg const noFunding = path.join(base, 'no-funding-package') @@ -241,7 +242,7 @@ testFundCmd({ expected: { error: { code: 'EFUNDGLOBAL', - summary: '\`npm fund\` does not support globals', + summary: '`npm fund` does not support globals', detail: '' } } @@ -250,14 +251,14 @@ testFundCmd({ testFundCmd({ title: 'fund using package argument with no browser', assertionMsg: 'should open funding url', - args: ['--browser', 'undefined', '.'], + args: ['.', '--no-browser'], opts: { cwd: maintainerOwnsAllDeps } }) testFundCmd({ title: 'fund using package argument with no browser, using --json option', assertionMsg: 'should open funding url', - args: ['--json', '--browser', 'undefined', '.'], + args: ['.', '--json', '--no-browser'], opts: { cwd: maintainerOwnsAllDeps }, assertion: jsonTest, expected: { @@ -266,27 +267,29 @@ testFundCmd({ } }) -test('fund using package argument', function (t) { - const fakeBrowser = path.join(common.pkg, '_script.sh') - const outFile = path.join(common.pkg, '_output') +if (!isWindows) { + test('fund using package argument', function (t) { + const fakeBrowser = path.join(common.pkg, '_script.sh') + const outFile = path.join(common.pkg, '_output') - const s = '#!/usr/bin/env bash\n' + - 'echo "$@" > ' + JSON.stringify(common.pkg) + '/_output\n' - fs.writeFileSync(fakeBrowser, s) - fs.chmodSync(fakeBrowser, '0755') + const s = '#!/usr/bin/env bash\n' + + 'echo "$@" > ' + JSON.stringify(common.pkg) + '/_output\n' + fs.writeFileSync(fakeBrowser, s) + fs.chmodSync(fakeBrowser, '0755') - common.npm([ - 'fund', '.', - '--loglevel=silent', - '--browser=' + fakeBrowser - ], { cwd: maintainerOwnsAllDeps }, function (err, code, stdout, stderr) { - t.ifError(err, 'repo command ran without error') - t.equal(code, 0, 'exit ok') - var res = fs.readFileSync(outFile, 'utf8') - t.equal(res, 'http://example.com/donate\n') - t.end() + common.npm([ + 'fund', '.', + '--loglevel=silent', + '--browser=' + fakeBrowser + ], { cwd: maintainerOwnsAllDeps }, function (err, code, stdout, stderr) { + t.ifError(err, 'repo command ran without error') + t.equal(code, 0, 'exit ok') + var res = fs.readFileSync(outFile, 'utf8') + t.equal(res, 'http://example.com/donate\n') + t.end() + }) }) -}) +} test('cleanup', function (t) { t.pass(base) diff --git a/test/tap/repo.js b/test/tap/repo.js index e68acb5b979ca..3e97fdeaed228 100644 --- a/test/tap/repo.js +++ b/test/tap/repo.js @@ -48,7 +48,7 @@ test('npm repo underscore --json', function (t) { '--json', '--registry=' + common.registry, '--loglevel=silent', - '--browser=undefined' + '--no-browser' ], opts, function (err, code, stdout, stderr) { t.ifError(err, 'repo command ran without error') t.equal(code, 0, 'exit ok') @@ -59,6 +59,23 @@ test('npm repo underscore --json', function (t) { }) }) +test('npm repo underscore --no-browser', function (t) { + mr({ port: common.port }, function (er, s) { + common.npm([ + 'repo', 'underscore', + '--no-browser', + '--registry=' + common.registry, + '--loglevel=silent' + ], opts, function (err, code, stdout, stderr) { + t.ifError(err, 'repo command ran without error') + t.equal(code, 0, 'exit ok') + t.matchSnapshot(stdout, 'should print alternative msg') + s.close() + t.end() + }) + }) +}) + test('npm repo optimist - github (https://)', function (t) { mr({ port: common.port }, function (er, s) { common.npm([