Skip to content

Commit

Permalink
fix(unpublish): properly apply publishConfig
Browse files Browse the repository at this point in the history
The tests for unpublish were mocked so heavily they weren't actually
asserting anything.  In rewriting them several bugs were found.

 - `write=true` was not being consistenly used when fetching packument
   data, it is now.
 - The decision on when to load the local package.json file was not
   working at all, that has been fixed.  If the cwd contains a
   package.json whose name matches the package you are uninstalling, the
   local package.json will be read and its publishConfig applied to your
   request.
 - dead code inside the `npm unpublish` path was removed.  There is no
   need to check if you are unpublishing the last version here, you're
   already unpublishing the entire project.
 - publishConfig is now being applied through the config flatten method,
   not a raw object assignment.
  • Loading branch information
wraithgar committed Mar 22, 2022
1 parent 362831c commit a64f0c2
Show file tree
Hide file tree
Showing 4 changed files with 389 additions and 441 deletions.
90 changes: 46 additions & 44 deletions lib/commands/unpublish.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
const path = require('path')
const util = require('util')
const npa = require('npm-package-arg')
const libaccess = require('libnpmaccess')
const npmFetch = require('npm-registry-fetch')
const libunpub = require('libnpmpublish').unpublish
const npa = require('npm-package-arg')
const npmFetch = require('npm-registry-fetch')
const path = require('path')
const util = require('util')
const readJson = util.promisify(require('read-package-json'))

const flatten = require('../utils/config/flatten.js')
const getIdentity = require('../utils/get-identity.js')
const log = require('../utils/log-shim')
const otplease = require('../utils/otplease.js')
const getIdentity = require('../utils/get-identity.js')

const LAST_REMAINING_VERSION_ERROR = 'Refusing to delete the last version of the package. ' +
'It will block from republishing a new version for 24 hours.\n' +
Expand All @@ -22,7 +24,8 @@ class Unpublish extends BaseCommand {
static ignoreImplicitWorkspace = false

async getKeysOfVersions (name, opts) {
const json = await npmFetch.json(npa(name).escapedName, opts)
const pkgUri = npa(name).escapedName
const json = await npmFetch.json(`${pkgUri}?write=true`, opts)
return Object.keys(json.versions)
}

Expand Down Expand Up @@ -67,12 +70,10 @@ class Unpublish extends BaseCommand {
throw this.usageError()
}

const spec = args.length && npa(args[0])
let spec = args.length && npa(args[0])
const force = this.npm.config.get('force')
const { silent } = this.npm
const dryRun = this.npm.config.get('dry-run')
let pkgName
let pkgVersion

log.silly('unpublish', 'args[0]', args[0])
log.silly('unpublish', 'spec', spec)
Expand All @@ -85,53 +86,54 @@ class Unpublish extends BaseCommand {
}

const opts = { ...this.npm.flatOptions }
if (!spec || path.resolve(spec.name) === this.npm.localPrefix) {

let pkgName
let pkgVersion
let manifest
let manifestErr
try {
const pkgJson = path.join(this.npm.localPrefix, 'package.json')
manifest = await readJson(pkgJson)
} 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 this.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 there's a package.json in the current folder, then
// read the package name and version out of that.
const pkgJson = path.join(this.npm.localPrefix, 'package.json')
let manifest
try {
manifest = await readJson(pkgJson)
} catch (err) {
if (err && err.code !== 'ENOENT' && err.code !== 'ENOTDIR') {
throw err
} else {
if (manifestErr) {
if (manifestErr.code === 'ENOENT' || manifestErr.code === 'ENOTDIR') {
throw this.usageError()
} else {
throw manifestErr
}
}

log.verbose('unpublish', manifest)

const { name, version, publishConfig } = manifest
const pkgJsonSpec = npa.resolve(name, version)
const optsWithPub = { ...opts, publishConfig }

const versions = await this.getKeysOfVersions(name, optsWithPub)
if (versions.length === 1 && !force) {
throw this.usageError(
LAST_REMAINING_VERSION_ERROR
)
spec = npa.resolve(manifest.name, manifest.version)
if (manifest.publishConfig) {
flatten(manifest.publishConfig, opts)
}

if (!dryRun) {
await otplease(opts, opts => libunpub(pkgJsonSpec, optsWithPub))
}
pkgName = name
pkgVersion = version ? `@${version}` : ''
} else {
const versions = await this.getKeysOfVersions(spec.name, opts)
if (versions.length === 1 && !force) {
throw this.usageError(
LAST_REMAINING_VERSION_ERROR
)
}
if (!dryRun) {
await otplease(opts, opts => libunpub(spec, opts))
}
pkgName = spec.name
pkgVersion = spec.type === 'version' ? `@${spec.rawSpec}` : ''
pkgName = manifest.name
pkgVersion = manifest.version ? `@${manifest.version}` : ''
}

if (!dryRun) {
await otplease(opts, opts => libunpub(spec, opts))
}
if (!silent) {
this.npm.output(`- ${pkgName}${pkgVersion}`)
}
Expand Down
14 changes: 0 additions & 14 deletions tap-snapshots/test/lib/commands/unpublish.js.test.cjs

This file was deleted.

10 changes: 6 additions & 4 deletions test/fixtures/tnock.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

const nock = require('nock')

// TODO (other tests actually make network calls today, which is bad)
// nock.disableNetConnect()
// Uncomment this to find requests that aren't matching
// nock.emitter.on('no match', req => console.log(req.options))

module.exports = tnock
function tnock (t, host) {
const server = nock(host)
function tnock (t, host, opts) {
nock.disableNetConnect()
const server = nock(host, opts)
t.teardown(function () {
nock.enableNetConnect()
server.done()
})
return server
Expand Down
Loading

0 comments on commit a64f0c2

Please sign in to comment.