From 4a4fbe33c51413adcd558b4af6f1e204b1b87e41 Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Fri, 21 May 2021 13:22:51 -0400 Subject: [PATCH] fix(publish): skip private workspaces Allow users to publish all workspaces with `npm publish --ws` while also skipping any workspace that might have been intentionally marked as private, using `"private": true` in its package.json file. Fixes: https://github.com/npm/cli/issues/3268 PR-URL: https://github.com/npm/cli/pull/3285 Credit: @ruyadorno Close: #3285 Reviewed-by: @wraithgar --- lib/publish.js | 22 +++- tap-snapshots/test/lib/publish.js.test.cjs | 34 +++++ test/lib/publish.js | 145 +++++++++++++++++++++ 3 files changed, 200 insertions(+), 1 deletion(-) diff --git a/lib/publish.js b/lib/publish.js index 1693ea7d97c2..3cb8b0627e97 100644 --- a/lib/publish.js +++ b/lib/publish.js @@ -7,6 +7,7 @@ const runScript = require('@npmcli/run-script') const pacote = require('pacote') const npa = require('npm-package-arg') const npmFetch = require('npm-registry-fetch') +const chalk = require('chalk') const otplease = require('./utils/otplease.js') const { getContents, logTar } = require('./utils/tar.js') @@ -154,10 +155,29 @@ class Publish extends BaseCommand { const results = {} const json = this.npm.config.get('json') const silent = log.level === 'silent' + const noop = a => a + const color = this.npm.color ? chalk : { green: noop, bold: noop } const workspaces = await getWorkspaces(filters, { path: this.npm.localPrefix }) + for (const [name, workspace] of workspaces.entries()) { - const pkgContents = await this.publish([workspace]) + let pkgContents + try { + pkgContents = await this.publish([workspace]) + } catch (err) { + if (err.code === 'EPRIVATE') { + log.warn( + 'publish', + `Skipping workspace ${ + color.green(name) + }, marked as ${ + color.bold('private') + }` + ) + continue + } + throw err + } // This needs to be in-line w/ the rest of the output that non-JSON // publish generates if (!silent && !json) diff --git a/tap-snapshots/test/lib/publish.js.test.cjs b/tap-snapshots/test/lib/publish.js.test.cjs index 05f0e6580ab5..7a7502e02e33 100644 --- a/tap-snapshots/test/lib/publish.js.test.cjs +++ b/tap-snapshots/test/lib/publish.js.test.cjs @@ -5,6 +5,40 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' +exports[`test/lib/publish.js TAP private workspaces colorless > should output all publishes 1`] = ` +Array [ + "+ @npmcli/b@1.0.0", +] +` + +exports[`test/lib/publish.js TAP private workspaces colorless > should publish all non-private workspaces 1`] = ` +Array [ + Object { + "_id": "@npmcli/b@1.0.0", + "name": "@npmcli/b", + "readme": "ERROR: No README data found!", + "version": "1.0.0", + }, +] +` + +exports[`test/lib/publish.js TAP private workspaces with color > should output all publishes 1`] = ` +Array [ + "+ @npmcli/b@1.0.0", +] +` + +exports[`test/lib/publish.js TAP private workspaces with color > should publish all non-private workspaces 1`] = ` +Array [ + Object { + "_id": "@npmcli/b@1.0.0", + "name": "@npmcli/b", + "readme": "ERROR: No README data found!", + "version": "1.0.0", + }, +] +` + exports[`test/lib/publish.js TAP shows usage with wrong set of arguments > should print usage 1`] = ` Error: Usage: npm publish diff --git a/test/lib/publish.js b/test/lib/publish.js index 80f3f7ccc639..e34f00b477ee 100644 --- a/test/lib/publish.js +++ b/test/lib/publish.js @@ -617,3 +617,148 @@ t.test('workspaces', (t) => { }) t.end() }) + +t.test('private workspaces', (t) => { + const testDir = t.testdir({ + 'package.json': JSON.stringify({ + name: 'workspaces-project', + version: '1.0.0', + workspaces: ['packages/*'], + }), + packages: { + a: { + 'package.json': JSON.stringify({ + name: '@npmcli/a', + version: '1.0.0', + private: true, + }), + }, + b: { + 'package.json': JSON.stringify({ + name: '@npmcli/b', + version: '1.0.0', + }), + }, + }, + }) + + const publishes = [] + const outputs = [] + t.beforeEach(() => { + npm.config.set('json', false) + outputs.length = 0 + publishes.length = 0 + }) + const mocks = { + '../../lib/utils/tar.js': { + getContents: (manifest) => ({ + id: manifest._id, + }), + logTar: () => {}, + }, + libnpmpublish: { + publish: (manifest, tarballData, opts) => { + if (manifest.private) { + throw Object.assign( + new Error('private pkg'), + { code: 'EPRIVATE' } + ) + } + publishes.push(manifest) + }, + }, + } + const npm = mockNpm({ + output: (o) => { + outputs.push(o) + }, + }) + npm.localPrefix = testDir + npm.config.getCredentialsByURI = (uri) => { + return { token: 'some.registry.token' } + } + + t.test('with color', t => { + const Publish = t.mock('../../lib/publish.js', { + ...mocks, + npmlog: { + notice () {}, + verbose () {}, + warn (title, msg) { + t.equal(title, 'publish', 'should use publish warn title') + t.match( + msg, + 'Skipping workspace \u001b[32m@npmcli/a\u001b[39m, marked as \u001b[1mprivate\u001b[22m', + 'should display skip private workspace warn msg' + ) + }, + }, + }) + const publish = new Publish(npm) + + npm.color = true + publish.execWorkspaces([], [], (err) => { + t.notOk(err) + t.matchSnapshot(publishes, 'should publish all non-private workspaces') + t.matchSnapshot(outputs, 'should output all publishes') + npm.color = false + t.end() + }) + }) + + t.test('colorless', t => { + const Publish = t.mock('../../lib/publish.js', { + ...mocks, + npmlog: { + notice () {}, + verbose () {}, + warn (title, msg) { + t.equal(title, 'publish', 'should use publish warn title') + t.equal( + msg, + 'Skipping workspace @npmcli/a, marked as private', + 'should display skip private workspace warn msg' + ) + }, + }, + }) + const publish = new Publish(npm) + + publish.execWorkspaces([], [], (err) => { + t.notOk(err) + t.matchSnapshot(publishes, 'should publish all non-private workspaces') + t.matchSnapshot(outputs, 'should output all publishes') + t.end() + }) + }) + + t.test('unexpected error', t => { + const Publish = t.mock('../../lib/publish.js', { + ...mocks, + libnpmpublish: { + publish: (manifest, tarballData, opts) => { + if (manifest.private) + throw new Error('ERR') + + publishes.push(manifest) + }, + }, + npmlog: { + notice () {}, + verbose () {}, + }, + }) + const publish = new Publish(npm) + + publish.execWorkspaces([], [], (err) => { + t.match( + err, + /ERR/, + 'should throw unexpected error' + ) + t.end() + }) + }) + + t.end() +})