From 1901d25473ba376a42eaaa16abe9811c50beae19 Mon Sep 17 00:00:00 2001 From: "Kyle E. Mitchell" Date: Tue, 10 Sep 2019 00:52:21 -0700 Subject: [PATCH] support: simplify to just collecting and showing URLs --- doc/files/package.json.md | 19 +-- doc/files/support.json | 57 -------- lib/install.js | 5 +- lib/support.js | 232 +++++++-------------------------- lib/utils/valid-support-url.js | 19 +++ package-lock.json | 46 +------ package.json | 3 - test/tap/support.js | 105 ++++++--------- 8 files changed, 116 insertions(+), 370 deletions(-) delete mode 100644 doc/files/support.json create mode 100644 lib/utils/valid-support-url.js diff --git a/doc/files/package.json.md b/doc/files/package.json.md index 9234f04cc8df5..76d2cf57436c9 100644 --- a/doc/files/package.json.md +++ b/doc/files/package.json.md @@ -172,22 +172,13 @@ npm also sets a top-level "maintainers" field with your npm user info. ## support -You can specify an HTTP endpoint for up-to-date information about ways -to support development of your package: +You can specify a URL for up-to-date information about ways to support +development of your package: - { "support": "https://example.com/support.json" } + { "support": "https://example.com/project/support" } -For example, you might like to develop your support data file in your -source code repository: - - { "support": "https://raw.githubusercontent.com/{user}/{repo}/master/support.json" } - -The URL you specify should respond to unauthenticated GET requests -with a JSON object. If the JSON object contains a `contributors` -array, `npm support` will interpret it as a `support.json` file. -If the JSON object contains a `versions` array, `npm support` -will interpret it as [Node.js Package Maintenance Working -Group](https://github.com/nodejs/package-maintenance) metadata. +Users can use the `npm support` subcommand to all the dependencies of +their project with `support` URLs. ## files diff --git a/doc/files/support.json b/doc/files/support.json deleted file mode 100644 index 672d933a75333..0000000000000 --- a/doc/files/support.json +++ /dev/null @@ -1,57 +0,0 @@ -spuport.json(5) -- Specifics of npm's support.json handling -=========================================================== - -## DESCRIPTION - -This document describes the format of `support.json` files, which you -can use to share information about how to support your work and projects -through `npm support`. - -`support.json` data must be actual JSON, not just a JavaScript object -literal. - -## contributors - -Each `support.json` file must contain a `contributors` property whose -value is an array. That array can contain two types of objects. - -Contributor objects provide information about people and organizations -that produce a package, and how to support them. For example: - -```json -{ - "name": "Ana Exemplar", - "homepage": "http://example.com/anaexemplar", - "links": [ - "http://patreon.com/anaexemplar" - ] -} -``` - -```json -{ - "name": "Ana Exemplar", - "homepage": "http://example.com/anaexemplar", - "links": [ - "http://patreon.com/anaexemplar" - ] -} - -```json -{ - "name": "JS Foundation", - "type": "organization", - "homepage": "https://js.foundation", - "links": [ - "https://js.foundation/about/donate" - ] -} -``` - -`contributors` array items may also include URLs for contributor objects: - -```json -{ - "url": "http://example.com/support-anaexemplar.json" -} -``` diff --git a/lib/install.js b/lib/install.js index deece78212fff..e80adc9e3a58c 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') @@ -811,7 +812,9 @@ Installer.prototype.printInstalledForHuman = function (diffs, auditResult) { var mutation = action[0] var pkg = action[1] if (pkg.failed) return - if (mutation !== 'remove' && pkg.package.support) { + if ( + mutation !== 'remove' && validSupportURL(pkg.package.support) + ) { haveSupportable = true } if (mutation === 'remove') { diff --git a/lib/support.js b/lib/support.js index 418d37fab5ffb..5813df93ff2a6 100644 --- a/lib/support.js +++ b/lib/support.js @@ -1,12 +1,11 @@ 'use strict' -var npm = require('./npm.js') -var output = require('./utils/output.js') -var readPackageTree = require('read-package-tree') -var runParallelLimit = require('run-parallel-limit') -var simpleGet = require('simple-get') -var semver = require('semver') -var hasANSI = require('has-ansi') +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 @@ -18,7 +17,6 @@ support.usage = usage( support.completion = function (opts, cb) { const argv = opts.conf.argv.remain - switch (argv[2]) { case 'support': return cb(null, []) @@ -27,206 +25,64 @@ support.completion = function (opts, cb) { } } +// Compare lib/ls.js. function support (args, silent, cb) { - readPackageTree(npm.dir, function (error, tree) { - if (error) return cb(error) - var supportablePackages = Array.from(findSupportablePackages(tree)) - downloadSupportData(supportablePackages, function (error, data) { - if (error) return cb(error) - - if (typeof cb !== 'function') { - cb = silent - silent = false - } - if (silent) return cb(null, data) - - var out - var json = npm.config.get('json') - if (json) { - out = JSON.stringify(data, null, 2) - } else { - out = data - .sort(function (a, b) { - var comparison = a.name.localeCompare(b.name) - return comparison === 0 - ? semver.compare(a.version, b.version) - : comparison - }) - .map(displaySupportData) - .join('\n\n') - } - output(out) - if (error) process.exitCode = 1 - cb(error, data) - }) + 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 findSupportablePackages (root) { - var set = new Set() +function findPackages (root) { + const set = new Set() iterate(root) - return set + 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) { - var metadata = node.package - if (metadata.support) { + 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, - parent: node.parent, - path: node.path + support: metadata.support }) } if (node.children) iterate(node) } } -function downloadSupportData (supportablePackages, cb) { - var cache = new Map() - var headers = { 'user-agent': npm.config.get('user-agent') } - runParallelLimit(supportablePackages.map(function (entry) { - return function task (done) { - var url = entry.support - get(url, function (error, response, projectData) { - if (error) { - return done(null, { - url: url, - error: 'could not download data' - }) - } - if (typeof projectData !== 'object' || Array.isArray(projectData)) { - return done(null, { - url: url, - error: 'not an object' - }) - } - var contributors = projectData.contributors - if (!Array.isArray(contributors)) { - return done(null, projectData) - } - runParallelLimit(contributors.map(function (contributor) { - return function (done) { - if ( - typeof contributor !== 'object' || - typeof contributor.url !== 'string' - ) { - return setImmediate(function () { - done(null, contributor) - }) - } - get(contributor.url, function (error, response, contributorData) { - if (error) { - return done(null, { - url: contributor.url, - error: error - }) - } - contributorData.url = contributor.url - var result = { - name: contributorData.name, - type: contributorData.type, - url: contributor.url - } - if (looksLikeURL(contributorData.homepage)) { - result.homepage = contributorData.homepage - } - if ( - Array.isArray(contributorData.links) && - contributorData.links.every(function (element) { - return looksLikeURL(element) - }) - ) { - result.links = contributorData.links - } - done(null, result) - }) - } - }), 5, function (error, resolvedContributors) { - if (error) return done(error) - done(null, { - name: entry.name, - version: entry.version, - url: entry.support, - homepage: entry.homepage, - contributors: resolvedContributors - }) - }) - }) - } - }), 5, cb) - - function get (url, cb) { - var cached = cache.get(url) - if (cached) { - return setImmediate(function () { - cb(null, { cached: true }, cached) - }) - } - simpleGet.concat({ - url: url, - json: true, - headers: headers - }, function (err, response, data) { - if (err) return cb(err) - cache.set(url, data) - cb(null, response, data) - }) - } -} - -function displaySupportData (entry) { - var returned = [entry.name + '@' + entry.version] - if (looksLikeURL(entry.homepage)) { - returned[0] += ' (' + entry.homepage + ')' - } - if (Array.isArray(entry.contributors)) { - entry.contributors.forEach(function (contributor) { - var name = contributor.name - if (looksLikeSafeString(name)) { - var item = ['- ' + name] - var email = contributor.email - if (looksLikeSafeString(email)) { - item[0] += ' <' + email + '>' - } - var homepage = contributor.homepage - if (looksLikeURL(homepage)) { - item[0] += ' (' + homepage + ')' - } - var links = contributor.links - if (Array.isArray(links)) { - links.forEach(function (link) { - if (looksLikeURL(link)) item.push(' ' + link) - }) - } - returned.push(item.join('\n')) - } - }) - } - return returned.join('\n') -} - -function looksLikeSafeString (argument) { - return ( - typeof argument === 'string' && - argument.length > 0 && - argument.length < 80 && - !hasANSI(argument) - ) -} - -function looksLikeURL (argument) { - return ( - looksLikeSafeString(argument) && - ( - argument.indexOf('https://') === 0 || - argument.indexOf('http://') === 0 - ) - ) +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 afdf2089319f6..58daf760c4245 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1182,14 +1182,6 @@ "resolved": "https://registry.npmjs.org/decode-uri-component/-/decode-uri-component-0.2.0.tgz", "integrity": "sha1-6zkTMzRYd1y4TNGh+uBiEGu4dUU=" }, - "decompress-response": { - "version": "3.3.0", - "resolved": "https://registry.npmjs.org/decompress-response/-/decompress-response-3.3.0.tgz", - "integrity": "sha1-gKTdMjdIOEv6JICDYirt7Jgq3/M=", - "requires": { - "mimic-response": "^1.0.0" - } - }, "deep-equal": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/deep-equal/-/deep-equal-1.0.1.tgz", @@ -2434,21 +2426,6 @@ "function-bind": "^1.1.1" } }, - "has-ansi": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/has-ansi/-/has-ansi-3.0.0.tgz", - "integrity": "sha1-Ngd+8dFfMzSEqn+neihgbxxlWzc=", - "requires": { - "ansi-regex": "^3.0.0" - }, - "dependencies": { - "ansi-regex": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-3.0.0.tgz", - "integrity": "sha1-7QMXwyIGT3lGbAKWa922Bas32Zg=" - } - } - }, "has-flag": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", @@ -3558,11 +3535,6 @@ "resolved": "https://registry.npmjs.org/mimic-fn/-/mimic-fn-1.2.0.tgz", "integrity": "sha512-jf84uxzwiuiIVKiOLpfYk7N46TSy8ubTonmneY9vrpHNAnp0QBt2BxWV9dO3/j+BoVAb+a5G6YDPW3M5HOdMWQ==" }, - "mimic-response": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/mimic-response/-/mimic-response-1.0.1.tgz", - "integrity": "sha512-j5EctnkH7amfV/q5Hgmoal1g2QHFJRraOtmx0JpIqkxhBhI/lJSl1nMpQ45hVarwNETOoWEimndZ4QK0RHxuxQ==" - }, "minimatch": { "version": "3.0.4", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", @@ -5073,11 +5045,6 @@ "integrity": "sha512-DEqnSRTDw/Tc3FXf49zedI638Z9onwUotBMiUFKmrO2sdFKIbXamXGQ3Axd4qgphxKB4kw/qP1w5kTxnfU1B9Q==", "dev": true }, - "run-parallel-limit": { - "version": "1.0.5", - "resolved": "https://registry.npmjs.org/run-parallel-limit/-/run-parallel-limit-1.0.5.tgz", - "integrity": "sha512-NsY+oDngvrvMxKB3G8ijBzIema6aYbQMD2bHOamvN52BysbIGTnEY2xsNyfrcr9GhY995/t/0nQN3R3oZvaDlg==" - }, "run-queue": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/run-queue/-/run-queue-1.0.3.tgz", @@ -5165,17 +5132,8 @@ "simple-concat": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/simple-concat/-/simple-concat-1.0.0.tgz", - "integrity": "sha1-c0TLuLbib7J9ZrL8hvn21Zl1IcY=" - }, - "simple-get": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/simple-get/-/simple-get-3.0.3.tgz", - "integrity": "sha512-Wvre/Jq5vgoz31Z9stYWPLn0PqRqmBDpFSdypAnHu5AvRVCYPRYGnvryNLiXu8GOBNDH82J2FRHUGMjjHUpXFw==", - "requires": { - "decompress-response": "^3.3.0", - "once": "^1.3.1", - "simple-concat": "^1.0.0" - } + "integrity": "sha1-c0TLuLbib7J9ZrL8hvn21Zl1IcY=", + "dev": true }, "slash": { "version": "1.0.0", diff --git a/package.json b/package.json index 1a61ccb539100..9b64923c418bd 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,6 @@ "gentle-fs": "^2.2.1", "glob": "^7.1.4", "graceful-fs": "^4.2.2", - "has-ansi": "^3.0.0", "has-unicode": "~2.0.1", "hosted-git-info": "^2.8.2", "iferr": "^1.0.2", @@ -127,11 +126,9 @@ "request": "^2.88.0", "retry": "^0.12.0", "rimraf": "^2.6.3", - "run-parallel-limit": "^1.0.5", "safe-buffer": "^5.1.2", "semver": "^5.7.1", "sha": "^3.0.0", - "simple-get": "^3.0.3", "slide": "~1.1.6", "sorted-object": "~2.0.1", "sorted-union-stream": "~2.1.3", diff --git a/test/tap/support.js b/test/tap/support.js index 8fbf2b131a5d1..93d4887423a13 100644 --- a/test/tap/support.js +++ b/test/tap/support.js @@ -1,98 +1,77 @@ 'use strict' var test = require('tap').test -var http = require('http') var Tacks = require('tacks') +var path = require('path') var Dir = Tacks.Dir var File = Tacks.File var common = require('../common-tap.js') -var server -var PORT = 8989 - -var CONTRIBUTOR = 'Test Contributor' -var EMAIL = 'contributor@example.com' -var HOMEPAGE = 'http://example.com/contributor' -var CONTRIBUTOR_LINK = 'http://example.com/donate' - -var testdir = common.pkg +var fixturepath = common.pkg var fixture = new Tacks(Dir({ - node_modules: Dir({ - 'package.json': File({ - name: 'a', - version: '0.0.0', - dependencies: { 'has-support': '7.7.7' } - }), - 'node_modules': Dir({ - b: Dir({ - 'package.json': File({ - name: 'has-support', - homepage: 'http://example.com/project', - version: '7.7.7', - support: 'http://localhost:' + PORT + '/project.json' - }) + '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(testdir) - fixture.create(testdir) - server = http.createServer() - .on('request', function (request, response) { - if (request.url === '/project.json') { - response.end(JSON.stringify({ - contributors: [ - { url: 'http://localhost:' + PORT + '/contributor.json' } - ] - })) - } else if (request.url === '/contributor.json') { - response.end(JSON.stringify({ - name: CONTRIBUTOR, - email: EMAIL, - homepage: HOMEPAGE, - links: [CONTRIBUTOR_LINK] - })) - } else { - response.statusCode = 404 - response.end() - } - }) - .listen(PORT, function () { - t.end() - }) + fixture.remove(fixturepath) + fixture.create(fixturepath) + t.end() }) test('support --json', function (t) { - common.npm(['support', '--json'], {cwd: testdir}, function (err, code, stdout, stderr) { + 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') - t.includes(stdout, 'has-support', 'metions project name') - t.includes(stdout, '7.7.7', 'metions project version') - t.includes(stdout, CONTRIBUTOR, 'metions contributor name') - t.includes(stdout, HOMEPAGE, 'metions contributor homepage') - t.includes(stdout, CONTRIBUTOR_LINK, 'metions contributor link') + 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: testdir}, function (err, code, stdout, stderr) { + 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, 'has-support', 'metions project name') - t.includes(stdout, '7.7.7', 'metions project version') - t.includes(stdout, CONTRIBUTOR, 'metions contributor name') - t.includes(stdout, HOMEPAGE, 'metions contributor homepage') - t.includes(stdout, CONTRIBUTOR_LINK, 'metions contributor link') + 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) { - server.close() - fixture.remove(testdir) + t.pass(fixturepath) + fixture.remove(fixturepath) t.end() })