diff --git a/lib/pack.js b/lib/pack.js index 8e61efabb36e4..5c0da6be7b6e0 100644 --- a/lib/pack.js +++ b/lib/pack.js @@ -45,22 +45,34 @@ class Pack extends BaseCommand { args = ['.'] const unicode = this.npm.config.get('unicode') + const dryRun = this.npm.config.get('dry-run') - // clone the opts because pacote mutates it with resolved/integrity - const tarballs = await Promise.all(args.map(async (arg) => { + // Get the manifests and filenames first so we can bail early on manifest + // errors before making any tarballs + const manifests = [] + for (const arg of args) { const spec = npa(arg) - const dryRun = this.npm.config.get('dry-run') const manifest = await pacote.manifest(spec, this.npm.flatOptions) + if (!manifest._id) + throw new Error('Invalid package, must have name and version') + const filename = `${manifest.name}-${manifest.version}.tgz` .replace(/^@/, '').replace(/\//, '-') + manifests.push({ arg, filename, manifest }) + } + + // Load tarball names up for printing afterward to isolate from the + // noise generated during packing + const tarballs = [] + for (const { arg, filename, manifest } of manifests) { const tarballData = await libpack(arg, this.npm.flatOptions) const pkgContents = await getContents(manifest, tarballData) if (!dryRun) await writeFile(filename, tarballData) - return pkgContents - })) + tarballs.push(pkgContents) + } for (const tar of tarballs) { logTar(tar, { log, unicode }) diff --git a/test/lib/pack.js b/test/lib/pack.js index 64f4d1258b9ce..6706042b4c82e 100644 --- a/test/lib/pack.js +++ b/test/lib/pack.js @@ -15,10 +15,12 @@ const mockPacote = { manifest: (spec) => { if (spec.type === 'directory') return pacote.manifest(spec) - return { + const m = { name: spec.name || 'test-package', version: spec.version || '1.0.0-test', } + m._id = `${m.name}@${m.version}` + return m }, } @@ -43,9 +45,8 @@ t.test('should pack current directory with no arguments', (t) => { }) const pack = new Pack(npm) - pack.exec([], er => { - if (er) - throw er + pack.exec([], err => { + t.error(err, { bail: true }) const filename = `npm-${require('../../package.json').version}.tgz` t.strictSame(OUTPUT, [[filename]]) @@ -79,9 +80,8 @@ t.test('should pack given directory', (t) => { }) const pack = new Pack(npm) - pack.exec([testDir], er => { - if (er) - throw er + pack.exec([testDir], err => { + t.error(err, { bail: true }) const filename = 'my-cool-pkg-1.0.0.tgz' t.strictSame(OUTPUT, [[filename]]) @@ -115,9 +115,8 @@ t.test('should pack given directory for scoped package', (t) => { }) const pack = new Pack(npm) - return pack.exec([testDir], er => { - if (er) - throw er + return pack.exec([testDir], err => { + t.error(err, { bail: true }) const filename = 'cool-my-pkg-1.0.0.tgz' t.strictSame(OUTPUT, [[filename]]) @@ -150,9 +149,8 @@ t.test('should log pack contents', (t) => { }) const pack = new Pack(npm) - pack.exec([], er => { - if (er) - throw er + pack.exec([], err => { + t.error(err, { bail: true }) const filename = `npm-${require('../../package.json').version}.tgz` t.strictSame(OUTPUT, [[filename]]) @@ -160,6 +158,38 @@ t.test('should log pack contents', (t) => { }) }) +t.test('invalid packument', (t) => { + const mockPacote = { + manifest: () => { + return {} + }, + } + const Pack = t.mock('../../lib/pack.js', { + libnpmpack, + pacote: mockPacote, + npmlog: { + notice: () => {}, + showProgress: () => {}, + clearProgress: () => {}, + }, + }) + const npm = mockNpm({ + config: { + unicode: true, + json: true, + 'dry-run': true, + }, + output, + }) + const pack = new Pack(npm) + pack.exec([], err => { + t.match(err, { message: 'Invalid package, must have name and version' }) + + t.strictSame(OUTPUT, []) + t.end() + }) +}) + t.test('workspaces', (t) => { const testDir = t.testdir({ 'package.json': JSON.stringify({ @@ -201,9 +231,8 @@ t.test('workspaces', (t) => { const pack = new Pack(npm) t.test('all workspaces', (t) => { - pack.execWorkspaces([], [], er => { - if (er) - throw er + pack.execWorkspaces([], [], err => { + t.error(err, { bail: true }) t.strictSame(OUTPUT, [ ['workspace-a-1.0.0.tgz'], @@ -214,9 +243,8 @@ t.test('workspaces', (t) => { }) t.test('all workspaces, `.` first arg', (t) => { - pack.execWorkspaces(['.'], [], er => { - if (er) - throw er + pack.execWorkspaces(['.'], [], err => { + t.error(err, { bail: true }) t.strictSame(OUTPUT, [ ['workspace-a-1.0.0.tgz'], @@ -227,9 +255,8 @@ t.test('workspaces', (t) => { }) t.test('one workspace', (t) => { - pack.execWorkspaces([], ['workspace-a'], er => { - if (er) - throw er + pack.execWorkspaces([], ['workspace-a'], err => { + t.error(err, { bail: true }) t.strictSame(OUTPUT, [ ['workspace-a-1.0.0.tgz'], @@ -239,9 +266,8 @@ t.test('workspaces', (t) => { }) t.test('specific package', (t) => { - pack.execWorkspaces(['abbrev'], [], er => { - if (er) - throw er + pack.execWorkspaces(['abbrev'], [], err => { + t.error(err, { bail: true }) t.strictSame(OUTPUT, [ ['abbrev-1.0.0-test.tgz'],