From 1b913226412452662bb11fda1dbc7557de16f08a Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 26 Oct 2020 15:49:19 -0700 Subject: [PATCH 1/2] pack: do not show individual files of bundled deps We show a list of bundled dependencies, there's no need to ALSO show the individual files in each one. --- lib/utils/tar.js | 15 +++++++-------- tap-snapshots/test-lib-utils-tar.js-TAP.test.js | 3 +-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/utils/tar.js b/lib/utils/tar.js index 9e3c7ec25cea6..887c40a0f6ebe 100644 --- a/lib/utils/tar.js +++ b/lib/utils/tar.js @@ -1,14 +1,10 @@ -'use strict' - const tar = require('tar') const ssri = require('ssri') const npmlog = require('npmlog') const byteSize = require('byte-size') const columnify = require('columnify') -module.exports = { logTar, getContents } - -function logTar (tarball, opts = {}) { +const logTar = (tarball, opts = {}) => { const { unicode = false, log = npmlog } = opts log.notice('') log.notice('', `${unicode ? '📦 ' : 'package:'} ${tarball.name}@${tarball.version}`) @@ -16,8 +12,9 @@ function logTar (tarball, opts = {}) { if (tarball.files.length) { log.notice('', columnify(tarball.files.map((f) => { const bytes = byteSize(f.size) - return { path: f.path, size: `${bytes.value}${bytes.unit}` } - }), { + return (/^node_modules\//.test(f.path)) ? null + : { path: f.path, size: `${bytes.value}${bytes.unit}` } + }).filter(f => f), { include: ['size', 'path'], showHeaders: false, })) @@ -49,7 +46,7 @@ function logTar (tarball, opts = {}) { log.notice('', '') } -async function getContents (manifest, tarball) { +const getContents = async (manifest, tarball) => { const files = [] const bundled = new Set() let totalEntries = 0 @@ -111,3 +108,5 @@ async function getContents (manifest, tarball) { bundled: Array.from(bundled), } } + +module.exports = { logTar, getContents } diff --git a/tap-snapshots/test-lib-utils-tar.js-TAP.test.js b/tap-snapshots/test-lib-utils-tar.js-TAP.test.js index 72b46004b684c..402a0e735afc4 100644 --- a/tap-snapshots/test-lib-utils-tar.js-TAP.test.js +++ b/tap-snapshots/test-lib-utils-tar.js-TAP.test.js @@ -11,8 +11,7 @@ exports[`test/lib/utils/tar.js TAP should log tarball contents > must match snap package: my-cool-pkg@1.0.0 === Tarball Contents === -4B node_modules/bundle-dep -97B package.json +97B package.json === Bundled Dependencies === bundle-dep From 1aa7c7a7c1356ddbc69cafb2f0008b2a79c20730 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 26 Oct 2020 16:49:52 -0700 Subject: [PATCH 2/2] view: Better errors when package.json is not JSON Previously, `npm view ` was just saying "Invalid package.json" if the package.json file was not JSON, or if it was missing. This errors out with the appropriate error in these cases. Also, no need to read the big clunky 'read-package-json' for this, we're literally just checking for a single field. We can be a bit more efficient here. --- lib/view.js | 10 +++++++--- test/lib/view.js | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/lib/view.js b/lib/view.js index 1415147767de8..42ba4b1c1b68e 100644 --- a/lib/view.js +++ b/lib/view.js @@ -9,12 +9,16 @@ const npm = require('./npm.js') const { packument } = require('pacote') const path = require('path') const { inspect, promisify } = require('util') -const readJson = promisify(require('read-package-json')) const relativeDate = require('tiny-relative-date') const semver = require('semver') const style = require('ansistyles') const usageUtil = require('./utils/usage') +const fs = require('fs') +const readFile = promisify(fs.readFile) +const jsonParse = require('json-parse-even-better-errors') +const readJson = async file => jsonParse(await readFile(file, 'utf8')) + const usage = usageUtil( 'view', 'npm view [<@scope>/][@] [[.subfield]...]' @@ -86,8 +90,8 @@ const view = async args => { if (local) { const dir = npm.prefix const manifest = await readJson(path.resolve(dir, 'package.json')) - if (!manifest || !manifest.name) - throw new Error('Invalid package.json') + if (!manifest.name) + throw new Error('Invalid package.json, no "name" field') const p = manifest.name nv = npa(p) if (pkg && ~pkg.indexOf('@')) diff --git a/test/lib/view.js b/test/lib/view.js index 9053d1a6473c2..88b2769a05899 100644 --- a/test/lib/view.js +++ b/test/lib/view.js @@ -482,7 +482,43 @@ t.test('throw error if global mode', (t) => { }) }) -t.test('throw error if invalid package.json', (t) => { +t.test('throw ENOENT error if package.json misisng', (t) => { + const testDir = t.testdir({}) + + const view = requireInject('../../lib/view.js', { + '../../lib/npm.js': { + prefix: testDir, + flatOptions: { + global: false + } + } + }) + view([], (err) => { + t.match(err, { code: 'ENOENT' }) + t.end() + }) +}) + +t.test('throw EJSONPARSE error if package.json not json', (t) => { + const testDir = t.testdir({ + 'package.json': 'not json, nope, not even a little bit!' + }) + + const view = requireInject('../../lib/view.js', { + '../../lib/npm.js': { + prefix: testDir, + flatOptions: { + global: false + } + } + }) + view([], (err) => { + t.match(err, { code: 'EJSONPARSE' }) + t.end() + }) +}) + +t.test('throw error if package.json has no name', (t) => { const testDir = t.testdir({ 'package.json': '{}' }) @@ -496,7 +532,7 @@ t.test('throw error if invalid package.json', (t) => { } }) view([], (err) => { - t.equals(err.message, 'Invalid package.json') + t.equals(err.message, 'Invalid package.json, no "name" field') t.end() }) })