diff --git a/docs/lib/content/commands/npm-unpublish.md b/docs/lib/content/commands/npm-unpublish.md index 82c4424c90d38..741fc83cee9aa 100644 --- a/docs/lib/content/commands/npm-unpublish.md +++ b/docs/lib/content/commands/npm-unpublish.md @@ -25,8 +25,12 @@ removing the tarball. The npm registry will return an error if you are not [logged in](/commands/npm-adduser). -If you do not specify a version or if you remove all of a package's -versions then the registry will remove the root package entry entirely. +If you do not specify a package name at all, the name and version to be +unpublished will be pulled from the project in the current directory. + +If you specify a package name but do not specify a version or if you +remove all of a package's versions then the registry will remove the +root package entry entirely. Even if you unpublish a package version, that specific name and version combination can never be reused. In order to publish the package again, diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index 402f8f30efff8..a9c20900534c3 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -1,7 +1,7 @@ const libaccess = require('libnpmaccess') const libunpub = require('libnpmpublish').unpublish const npa = require('npm-package-arg') -const npmFetch = require('npm-registry-fetch') +const pacote = require('pacote') const pkgJson = require('@npmcli/package-json') const { flatten } = require('@npmcli/config/lib/definitions') @@ -23,12 +23,12 @@ class Unpublish extends BaseCommand { static ignoreImplicitWorkspace = false static async getKeysOfVersions (name, opts) { - const pkgUri = npa(name).escapedName - const json = await npmFetch.json(`${pkgUri}?write=true`, { + const packument = await pacote.packument(name, { ...opts, spec: name, + query: { write: true }, }) - return Object.keys(json.versions) + return Object.keys(packument.versions) } static async completion (args, npm) { @@ -59,7 +59,7 @@ class Unpublish extends BaseCommand { return pkgs } - const versions = await this.getKeysOfVersions(pkgs[0], opts) + const versions = await Unpublish.getKeysOfVersions(pkgs[0], opts) if (!versions.length) { return pkgs } else { @@ -67,20 +67,35 @@ class Unpublish extends BaseCommand { } } - async exec (args) { + async exec (args, { localPrefix } = {}) { if (args.length > 1) { throw this.usageError() } - let spec = args.length && npa(args[0]) + // workspace mode + if (!localPrefix) { + localPrefix = this.npm.localPrefix + } + const force = this.npm.config.get('force') const { silent } = this.npm const dryRun = this.npm.config.get('dry-run') + let spec + if (args.length) { + spec = npa(args[0]) + if (spec.type !== 'version' && spec.rawSpec !== '*') { + throw this.usageError( + 'Can only unpublish a single version, or the entire project.\n' + + 'Tags and ranges are not supported.' + ) + } + } + log.silly('unpublish', 'args[0]', args[0]) log.silly('unpublish', 'spec', spec) - if ((!spec || !spec.rawSpec) && !force) { + if (spec?.rawSpec === '*' && !force) { throw this.usageError( 'Refusing to delete entire project.\n' + 'Run with --force to do this.' @@ -89,69 +104,67 @@ class Unpublish extends BaseCommand { const opts = { ...this.npm.flatOptions } - let pkgName - let pkgVersion let manifest - let manifestErr try { - const { content } = await pkgJson.prepare(this.npm.localPrefix) + const { content } = await pkgJson.prepare(localPrefix) manifest = content } catch (err) { - manifestErr = err - } - if (spec) { - // If cwd has a package.json with a name that matches the package being - // unpublished, load up the publishConfig - if (manifest && manifest.name === spec.name && manifest.publishConfig) { - flatten(manifest.publishConfig, opts) - } - const versions = await Unpublish.getKeysOfVersions(spec.name, opts) - if (versions.length === 1 && !force) { - throw this.usageError(LAST_REMAINING_VERSION_ERROR) - } - pkgName = spec.name - pkgVersion = spec.type === 'version' ? `@${spec.rawSpec}` : '' - } else { - if (manifestErr) { - if (manifestErr.code === 'ENOENT' || manifestErr.code === 'ENOTDIR') { + if (err.code === 'ENOENT' || err.code === 'ENOTDIR') { + if (!spec) { + // We needed a local package.json to figure out what package to + // unpublish throw this.usageError() - } else { - throw manifestErr } + } else { + // folks should know if ANY local package.json had a parsing error. + // They may be relying on `publishConfig` to be loading and we don't + // want to ignore errors in that case. + throw err } + } - log.verbose('unpublish', manifest) - + let pkgVersion // for cli output + if (spec) { + pkgVersion = spec.type === 'version' ? `@${spec.rawSpec}` : '' + } else { spec = npa.resolve(manifest.name, manifest.version) - if (manifest.publishConfig) { - flatten(manifest.publishConfig, opts) + log.verbose('unpublish', manifest) + pkgVersion = manifest.version ? `@${manifest.version}` : '' + if (!manifest.version && !force) { + throw this.usageError( + 'Refusing to delete entire project.\n' + + 'Run with --force to do this.' + ) } + } - pkgName = manifest.name - pkgVersion = manifest.version ? `@${manifest.version}` : '' + // If localPrefix has a package.json with a name that matches the package + // being unpublished, load up the publishConfig + if (manifest?.name === spec.name && manifest.publishConfig) { + flatten(manifest.publishConfig, opts) + } + + const versions = await Unpublish.getKeysOfVersions(spec.name, opts) + if (versions.length === 1 && spec.rawSpec === versions[0] && !force) { + throw this.usageError(LAST_REMAINING_VERSION_ERROR) + } + if (versions.length === 1) { + pkgVersion = '' } if (!dryRun) { await otplease(this.npm, opts, o => libunpub(spec, o)) } if (!silent) { - this.npm.output(`- ${pkgName}${pkgVersion}`) + this.npm.output(`- ${spec.name}${pkgVersion}`) } } async execWorkspaces (args) { await this.setWorkspaces() - const force = this.npm.config.get('force') - if (!force) { - throw this.usageError( - 'Refusing to delete entire project(s).\n' + - 'Run with --force to do this.' - ) - } - - for (const name of this.workspaceNames) { - await this.exec([name]) + for (const path of this.workspacePaths) { + await this.exec(args, { localPrefix: path }) } } } diff --git a/test/lib/commands/unpublish.js b/test/lib/commands/unpublish.js index 6e898bd3d07e4..097309393a258 100644 --- a/test/lib/commands/unpublish.js +++ b/test/lib/commands/unpublish.js @@ -26,10 +26,10 @@ t.test('no args --force success', async t => { authorization: 'test-auth-token', }) const manifest = registry.manifest({ name: pkg }) - await registry.package({ manifest, query: { write: true } }) + await registry.package({ manifest, query: { write: true }, times: 2 }) registry.unpublish({ manifest }) await npm.exec('unpublish', []) - t.equal(joinedOutput(), '- test-package@1.0.0') + t.equal(joinedOutput(), '- test-package') }) t.test('no args --force missing package.json', async t => { @@ -63,11 +63,28 @@ t.test('no args --force error reading package.json', async t => { ) }) -t.test('no args entire project', async t => { +t.test('with args --force error reading package.json', async t => { + const { npm } = await loadMockNpm(t, { + config: { + force: true, + }, + prefixDir: { + 'package.json': '{ not valid json ]', + }, + }) + + await t.rejects( + npm.exec('unpublish', [pkg]), + /Invalid package.json/, + 'should throw error from reading package.json' + ) +}) + +t.test('no force entire project', async t => { const { npm } = await loadMockNpm(t) await t.rejects( - npm.exec('unpublish', []), + npm.exec('unpublish', ['@npmcli/unpublish-test']), /Refusing to delete entire project/ ) }) @@ -82,6 +99,26 @@ t.test('too many args', async t => { ) }) +t.test('range', async t => { + const { npm } = await loadMockNpm(t) + + await t.rejects( + npm.exec('unpublish', ['a@>1.0.0']), + { code: 'EUSAGE' }, + /single version/ + ) +}) + +t.test('tag', async t => { + const { npm } = await loadMockNpm(t) + + await t.rejects( + npm.exec('unpublish', ['a@>1.0.0']), + { code: 'EUSAGE' }, + /single version/ + ) +}) + t.test('unpublish @version not the last version', async t => { const { joinedOutput, npm } = await loadMockNpm(t, { config: { @@ -129,7 +166,24 @@ t.test('unpublish @version last version', async t => { ) }) -t.test('no version found in package.json', async t => { +t.test('no version found in package.json no force', async t => { + const { npm } = await loadMockNpm(t, { + config: { + ...auth, + }, + prefixDir: { + 'package.json': JSON.stringify({ + name: pkg, + }, null, 2), + }, + }) + await t.rejects( + npm.exec('unpublish', []), + /Refusing to delete entire project/ + ) +}) + +t.test('no version found in package.json with force', async t => { const { joinedOutput, npm } = await loadMockNpm(t, { config: { force: true, @@ -147,7 +201,7 @@ t.test('no version found in package.json', async t => { authorization: 'test-auth-token', }) const manifest = registry.manifest({ name: pkg }) - await registry.package({ manifest, query: { write: true } }) + await registry.package({ manifest, query: { write: true }, times: 2 }) registry.unpublish({ manifest }) await npm.exec('unpublish', []) @@ -219,7 +273,7 @@ t.test('workspaces', async t => { 'workspace-b': { 'package.json': JSON.stringify({ name: 'workspace-b', - version: '1.2.3-n', + version: '1.2.3-b', repository: 'https://github.com/npm/workspace-b', }), }, @@ -231,20 +285,20 @@ t.test('workspaces', async t => { }, } - t.test('no force', async t => { + t.test('with package name no force', async t => { const { npm } = await loadMockNpm(t, { config: { - workspaces: true, + workspace: ['workspace-a'], }, prefixDir, }) await t.rejects( - npm.exec('unpublish', []), + npm.exec('unpublish', ['workspace-a']), /Refusing to delete entire project/ ) }) - t.test('all workspaces --force', async t => { + t.test('all workspaces last version --force', async t => { const { joinedOutput, npm } = await loadMockNpm(t, { config: { workspaces: true, @@ -258,9 +312,9 @@ t.test('workspaces', async t => { registry: npm.config.get('registry'), authorization: 'test-auth-token', }) - const manifestA = registry.manifest({ name: 'workspace-a' }) - const manifestB = registry.manifest({ name: 'workspace-b' }) - const manifestN = registry.manifest({ name: 'workspace-n' }) + const manifestA = registry.manifest({ name: 'workspace-a', versions: ['1.2.3-a'] }) + const manifestB = registry.manifest({ name: 'workspace-b', versions: ['1.2.3-b'] }) + const manifestN = registry.manifest({ name: 'workspace-n', versions: ['1.2.3-n'] }) await registry.package({ manifest: manifestA, query: { write: true }, times: 2 }) await registry.package({ manifest: manifestB, query: { write: true }, times: 2 }) await registry.package({ manifest: manifestN, query: { write: true }, times: 2 }) @@ -271,28 +325,6 @@ t.test('workspaces', async t => { await npm.exec('unpublish', []) t.equal(joinedOutput(), '- workspace-a\n- workspace-b\n- workspace-n') }) - - t.test('one workspace --force', async t => { - const { joinedOutput, npm } = await loadMockNpm(t, { - config: { - workspace: ['workspace-a'], - force: true, - ...auth, - }, - prefixDir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - authorization: 'test-auth-token', - }) - const manifestA = registry.manifest({ name: 'workspace-a' }) - await registry.package({ manifest: manifestA, query: { write: true }, times: 2 }) - registry.nock.delete(`/workspace-a/-rev/${manifestA._rev}`).reply(201) - - await npm.exec('unpublish', []) - t.equal(joinedOutput(), '- workspace-a') - }) }) t.test('dryRun with spec', async t => { @@ -331,6 +363,16 @@ t.test('dryRun with no args', async t => { }, null, 2), }, }) + const registry = new MockRegistry({ + tap: t, + registry: npm.config.get('registry'), + authorization: 'test-auth-token', + }) + const manifest = registry.manifest({ + name: pkg, + packuments: ['1.0.0', '1.0.1'], + }) + await registry.package({ manifest, query: { write: true } }) await npm.exec('unpublish', []) t.equal(joinedOutput(), '- test-package@1.0.0') @@ -360,10 +402,10 @@ t.test('publishConfig no spec', async t => { authorization: 'test-other-token', }) const manifest = registry.manifest({ name: pkg }) - await registry.package({ manifest, query: { write: true } }) + await registry.package({ manifest, query: { write: true }, times: 2 }) registry.unpublish({ manifest }) await npm.exec('unpublish', []) - t.equal(joinedOutput(), '- test-package@1.0.0') + t.equal(joinedOutput(), '- test-package') }) t.test('publishConfig with spec', async t => { @@ -421,7 +463,7 @@ t.test('scoped registry config', async t => { authorization: 'test-other-token', }) const manifest = registry.manifest({ name: scopedPkg }) - await registry.package({ manifest, query: { write: true } }) + await registry.package({ manifest, query: { write: true }, times: 2 }) registry.unpublish({ manifest }) await npm.exec('unpublish', [scopedPkg]) })