From 865ee0aa0b6e825487c5957a49e71609ab02e306 Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 25 Nov 2024 17:09:12 -0500 Subject: [PATCH 01/20] fix: standardize npm-registry-fetch import declaration --- lib/commands/deprecate.js | 6 +++--- lib/commands/dist-tag.js | 8 ++++---- lib/commands/doctor.js | 4 ++-- lib/commands/star.js | 6 +++--- lib/commands/stars.js | 4 ++-- lib/utils/ping.js | 4 ++-- lib/utils/verify-signatures.js | 6 +++--- workspaces/arborist/lib/audit-report.js | 4 ++-- workspaces/arborist/lib/query-selector-all.js | 4 ++-- workspaces/libnpmorg/lib/index.js | 8 ++++---- 10 files changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/commands/deprecate.js b/lib/commands/deprecate.js index 977fd9fce11da..95eaf429120fa 100644 --- a/lib/commands/deprecate.js +++ b/lib/commands/deprecate.js @@ -1,4 +1,4 @@ -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') const { otplease } = require('../utils/auth.js') const npa = require('npm-package-arg') const { log } = require('proc-log') @@ -47,7 +47,7 @@ class Deprecate extends BaseCommand { } const uri = '/' + p.escapedName - const packument = await fetch.json(uri, { + const packument = await npmFetch.json(uri, { ...this.npm.flatOptions, spec: p, query: { write: true }, @@ -60,7 +60,7 @@ class Deprecate extends BaseCommand { for (const v of versions) { packument.versions[v].deprecated = msg } - return otplease(this.npm, this.npm.flatOptions, opts => fetch(uri, { + return otplease(this.npm, this.npm.flatOptions, opts => npmFetch(uri, { ...opts, spec: p, method: 'PUT', diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index ba9d6446c869f..3fdecd926a564 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -1,5 +1,5 @@ const npa = require('npm-package-arg') -const regFetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') const semver = require('semver') const { log, output } = require('proc-log') const { otplease } = require('../utils/auth.js') @@ -119,7 +119,7 @@ class DistTag extends BaseCommand { }, spec, } - await otplease(this.npm, reqOpts, o => regFetch(url, o)) + await otplease(this.npm, reqOpts, o => npmFetch(url, o)) output.standard(`+${t}: ${spec.name}@${version}`) } @@ -145,7 +145,7 @@ class DistTag extends BaseCommand { method: 'DELETE', spec, } - await otplease(this.npm, reqOpts, o => regFetch(url, o)) + await otplease(this.npm, reqOpts, o => npmFetch(url, o)) output.standard(`-${tag}: ${spec.name}@${version}`) } @@ -191,7 +191,7 @@ class DistTag extends BaseCommand { } async fetchTags (spec, opts) { - const data = await regFetch.json( + const data = await npmFetch.json( `/-/package/${spec.escapedName}/dist-tags`, { ...opts, 'prefer-online': true, spec } ) diff --git a/lib/commands/doctor.js b/lib/commands/doctor.js index 64a8423a32fa3..8f87fdc17891c 100644 --- a/lib/commands/doctor.js +++ b/lib/commands/doctor.js @@ -1,6 +1,6 @@ const cacache = require('cacache') const { access, lstat, readdir, constants: { R_OK, W_OK, X_OK } } = require('node:fs/promises') -const fetch = require('make-fetch-happen') +const npmFetch = require('make-fetch-happen') const which = require('which') const pacote = require('pacote') const { resolve } = require('node:path') @@ -166,7 +166,7 @@ class Doctor extends BaseCommand { const currentRange = `^${current}` const url = 'https://nodejs.org/dist/index.json' log.info('doctor', 'Getting Node.js release information') - const res = await fetch(url, { method: 'GET', ...this.npm.flatOptions }) + const res = await npmFetch(url, { method: 'GET', ...this.npm.flatOptions }) const data = await res.json() let maxCurrent = '0.0.0' let maxLTS = '0.0.0' diff --git a/lib/commands/star.js b/lib/commands/star.js index 1b76955810c72..7d1be1d389730 100644 --- a/lib/commands/star.js +++ b/lib/commands/star.js @@ -1,4 +1,4 @@ -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') const npa = require('npm-package-arg') const { log, output } = require('proc-log') const getIdentity = require('../utils/get-identity') @@ -32,7 +32,7 @@ class Star extends BaseCommand { const username = await getIdentity(this.npm, this.npm.flatOptions) for (const pkg of pkgs) { - const fullData = await fetch.json(pkg.escapedName, { + const fullData = await npmFetch.json(pkg.escapedName, { ...this.npm.flatOptions, spec: pkg, query: { write: true }, @@ -55,7 +55,7 @@ class Star extends BaseCommand { log.verbose('unstar', 'unstarring', body) } - const data = await fetch.json(pkg.escapedName, { + const data = await npmFetch.json(pkg.escapedName, { ...this.npm.flatOptions, spec: pkg, method: 'PUT', diff --git a/lib/commands/stars.js b/lib/commands/stars.js index 1059569979daf..d059d01250900 100644 --- a/lib/commands/stars.js +++ b/lib/commands/stars.js @@ -1,4 +1,4 @@ -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') const { log, output } = require('proc-log') const getIdentity = require('../utils/get-identity.js') const BaseCommand = require('../base-cmd.js') @@ -16,7 +16,7 @@ class Stars extends BaseCommand { user = await getIdentity(this.npm, this.npm.flatOptions) } - const { rows } = await fetch.json('/-/_view/starredByUser', { + const { rows } = await npmFetch.json('/-/_view/starredByUser', { ...this.npm.flatOptions, query: { key: `"${user}"` }, }) diff --git a/lib/utils/ping.js b/lib/utils/ping.js index 1c8c9e827a4ea..3d47ca1ecaf54 100644 --- a/lib/utils/ping.js +++ b/lib/utils/ping.js @@ -1,7 +1,7 @@ // ping the npm registry // used by the ping and doctor commands -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') module.exports = async (flatOptions) => { - const res = await fetch('/-/ping', { ...flatOptions, cache: false }) + const res = await npmFetch('/-/ping', { ...flatOptions, cache: false }) return res.json().catch(() => ({})) } diff --git a/lib/utils/verify-signatures.js b/lib/utils/verify-signatures.js index 09711581d11dd..5671738315284 100644 --- a/lib/utils/verify-signatures.js +++ b/lib/utils/verify-signatures.js @@ -1,4 +1,4 @@ -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') const localeCompare = require('@isaacs/string-locale-compare')('en') const npa = require('npm-package-arg') const pacote = require('pacote') @@ -202,7 +202,7 @@ class VerifySignatures { // If keys not found in Sigstore TUF repo, fallback to registry keys API if (!keys) { - keys = await fetch.json('/-/npm/v1/keys', { + keys = await npmFetch.json('/-/npm/v1/keys', { ...this.npm.flatOptions, registry, }).then(({ keys: ks }) => ks.map((key) => ({ @@ -253,7 +253,7 @@ class VerifySignatures { } getSpecRegistry (spec) { - return fetch.pickRegistry(spec, this.npm.flatOptions) + return npmFetch.pickRegistry(spec, this.npm.flatOptions) } getValidPackageInfo (edge) { diff --git a/workspaces/arborist/lib/audit-report.js b/workspaces/arborist/lib/audit-report.js index 836c2ed5a20ea..dbd9be8bd3865 100644 --- a/workspaces/arborist/lib/audit-report.js +++ b/workspaces/arborist/lib/audit-report.js @@ -15,7 +15,7 @@ const _init = Symbol('init') const _omit = Symbol('omit') const { log, time } = require('proc-log') -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') class AuditReport extends Map { static load (tree, opts) { @@ -291,7 +291,7 @@ class AuditReport extends Map { return null } - const res = await fetch('/-/npm/v1/security/advisories/bulk', { + const res = await npmFetch('/-/npm/v1/security/advisories/bulk', { ...this.options, registry: this.options.auditRegistry || this.options.registry, method: 'POST', diff --git a/workspaces/arborist/lib/query-selector-all.js b/workspaces/arborist/lib/query-selector-all.js index fa48d5f84b556..c2cd00d0a2e2e 100644 --- a/workspaces/arborist/lib/query-selector-all.js +++ b/workspaces/arborist/lib/query-selector-all.js @@ -8,7 +8,7 @@ const { minimatch } = require('minimatch') const npa = require('npm-package-arg') const pacote = require('pacote') const semver = require('semver') -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') // handle results for parsed query asts, results are stored in a map that has a // key that points to each ast selector node and stores the resulting array of @@ -461,7 +461,7 @@ class Results { packages[node.name].push(node.version) } }) - const res = await fetch('/-/npm/v1/security/advisories/bulk', { + const res = await npmFetch('/-/npm/v1/security/advisories/bulk', { ...this.flatOptions, registry: this.flatOptions.auditRegistry || this.flatOptions.registry, method: 'POST', diff --git a/workspaces/libnpmorg/lib/index.js b/workspaces/libnpmorg/lib/index.js index 4684b516d2b4a..f3d361b8be6d7 100644 --- a/workspaces/libnpmorg/lib/index.js +++ b/workspaces/libnpmorg/lib/index.js @@ -1,7 +1,7 @@ 'use strict' const eu = encodeURIComponent -const fetch = require('npm-registry-fetch') +const npmFetch = require('npm-registry-fetch') const validate = require('aproba') // From https://github.com/npm/registry/blob/master/docs/orgs/memberships.md @@ -19,7 +19,7 @@ cmd.set = (org, user, role, opts = {}) => { validate('SSSO|SSZO', [org, user, role, opts]) user = user.replace(/^@?/, '') org = org.replace(/^@?/, '') - return fetch.json(`/-/org/${eu(org)}/user`, { + return npmFetch.json(`/-/org/${eu(org)}/user`, { ...opts, method: 'PUT', body: { user, role }, @@ -30,7 +30,7 @@ cmd.rm = (org, user, opts = {}) => { validate('SSO', [org, user, opts]) user = user.replace(/^@?/, '') org = org.replace(/^@?/, '') - return fetch(`/-/org/${eu(org)}/user`, { + return npmFetch(`/-/org/${eu(org)}/user`, { ...opts, method: 'DELETE', body: { user }, @@ -55,7 +55,7 @@ cmd.ls = (org, opts = {}) => { cmd.ls.stream = (org, opts = {}) => { validate('SO', [org, opts]) org = org.replace(/^@?/, '') - return fetch.json.stream(`/-/org/${eu(org)}/user`, '*', { + return npmFetch.json.stream(`/-/org/${eu(org)}/user`, '*', { ...opts, mapJSON: (value, [key]) => { return [key, value] From 2bccf91f253f41717958785a8ba2b145e88d76ae Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 25 Nov 2024 17:09:40 -0500 Subject: [PATCH 02/20] feat: adds mock for dist-tags --- test/fixtures/mock-npm.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 9e9113972d6a3..6b47079b5da8a 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -2,6 +2,7 @@ const os = require('node:os') const fs = require('node:fs').promises const fsSync = require('node:fs') const path = require('node:path') +const npmFetch = require('npm-registry-fetch') const tap = require('tap') const mockLogs = require('./mock-logs.js') const mockGlobals = require('@npmcli/mock-globals') @@ -449,8 +450,34 @@ function workspaceMock (t, opts) { } } +const mockNpmRegistryFetch = (tags) => { + const fetchOpts = {} + const getRequest = async (url, opts) => { + if (fetchOpts[url]) { + fetchOpts[url].push(opts) + } else { + fetchOpts[url] = [opts] + } + const find = ({ ...tags })[url] + if (typeof find === 'function') { + return find() + } + return find + } + const nrf = async (url, opts) => { + return { + json: getRequest(url, opts), + } + } + const mock = Object.assign(nrf, npmFetch, { json: getRequest }) + const mocks = { 'npm-registry-fetch': mock } + const getOpts = (url) => fetchOpts[url] + return { mocks, mock, fetchOpts, getOpts } +} + module.exports = setupMockNpm module.exports.load = setupMockNpm module.exports.setGlobalNodeModules = setGlobalNodeModules module.exports.loadNpmWithRegistry = loadNpmWithRegistry module.exports.workspaceMock = workspaceMock +module.exports.mockNpmRegistryFetch = mockNpmRegistryFetch From 26d4c2bb56cfe759290b2ea41b3d30c510d0ee91 Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 25 Nov 2024 23:20:04 -0500 Subject: [PATCH 03/20] feat!: no implicit latest tag on publish when latest > version --- lib/commands/dist-tag.js | 8 +- lib/commands/publish.js | 18 ++++ smoke-tests/test/npm-replace-global.js | 1 + test/fixtures/mock-npm.js | 30 +++++- test/lib/commands/publish.js | 121 ++++++++++++++++++++++++- 5 files changed, 172 insertions(+), 6 deletions(-) diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index 3fdecd926a564..4274b982d32fb 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -102,7 +102,7 @@ class DistTag extends BaseCommand { throw new Error('Tag name must not be a valid SemVer range: ' + t) } - const tags = await this.fetchTags(spec, opts) + const tags = await DistTag.fetchTags(spec, opts) if (tags[t] === version) { log.warn('dist-tag add', t, 'is already set to version', version) return @@ -131,7 +131,7 @@ class DistTag extends BaseCommand { throw this.usageError() } - const tags = await this.fetchTags(spec, opts) + const tags = await DistTag.fetchTags(spec, opts) if (!tags[tag]) { log.info('dist-tag del', tag, 'is not a dist-tag on', spec.name) throw new Error(tag + ' is not a dist-tag on ' + spec.name) @@ -164,7 +164,7 @@ class DistTag extends BaseCommand { spec = npa(spec) try { - const tags = await this.fetchTags(spec, opts) + const tags = await DistTag.fetchTags(spec, opts) const msg = Object.keys(tags).map(k => `${k}: ${tags[k]}`).sort().join('\n') output.standard(msg) @@ -190,7 +190,7 @@ class DistTag extends BaseCommand { } } - async fetchTags (spec, opts) { + static async fetchTags (spec, opts) { const data = await npmFetch.json( `/-/package/${spec.escapedName}/dist-tags`, { ...opts, 'prefer-online': true, spec } diff --git a/lib/commands/publish.js b/lib/commands/publish.js index d15bb5a2eb272..7be3cfe1d0f0d 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -16,6 +16,7 @@ const { getContents, logTar } = require('../utils/tar.js') const { flatten } = require('@npmcli/config/lib/definitions') const pkgJson = require('@npmcli/package-json') const BaseCommand = require('../base-cmd.js') +const DistTag = require('./dist-tag.js') class Publish extends BaseCommand { static description = 'Publish a package' @@ -131,6 +132,13 @@ class Publish extends BaseCommand { const resolved = npa.resolve(manifest.name, manifest.version) + const latestDistTag = await this.#latestDistTag(resolved) + const latestTagIsGreater = latestDistTag ? semver.gte(latestDistTag, manifest.version) : false + + if (latestTagIsGreater && isDefaultTag) { + throw new Error('Cannot publish a lower version without an explicit dist tag.') + } + // make sure tag is valid, this will throw if invalid npa(`${manifest.name}@${defaultTag}`) @@ -196,6 +204,16 @@ class Publish extends BaseCommand { } } + async #latestDistTag (spec) { + try { + const tags = await DistTag.fetchTags(spec, this.npm.flatOptions) + return tags.latest + } catch (_e) { + // this will fail if the package is new, so just return null + return null + } + } + // if it's a directory, read it from the file system // otherwise, get the full metadata from whatever it is // XXX can't pacote read the manifest from a directory? diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index d3638158e2ca6..687c540375b86 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -136,6 +136,7 @@ t.test('publish and replace global self', async t => { if (setup.SMOKE_PUBLISH) { await npmPackage() } + registry.nock.get('/-/package/npm/dist-tags').reply(404, 'not found') registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 6b47079b5da8a..486f8e75104b6 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -459,6 +459,9 @@ const mockNpmRegistryFetch = (tags) => { fetchOpts[url] = [opts] } const find = ({ ...tags })[url] + if (!find) { + throw new Error(`no npm-registry-fetch mock for ${url}`) + } if (typeof find === 'function') { return find() } @@ -466,7 +469,7 @@ const mockNpmRegistryFetch = (tags) => { } const nrf = async (url, opts) => { return { - json: getRequest(url, opts), + json: () => getRequest(url, opts), } } const mock = Object.assign(nrf, npmFetch, { json: getRequest }) @@ -475,9 +478,34 @@ const mockNpmRegistryFetch = (tags) => { return { mocks, mock, fetchOpts, getOpts } } +const putPackagePayload = ({ pkg, alternateRegistry, version }) => ({ + _id: pkg, + name: pkg, + 'dist-tags': { latest: version }, + access: null, + versions: { + [version]: { + name: pkg, + version: version, + _id: `${pkg}@${version}`, + dist: { + shasum: /\.*/, + tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-${version}.tgz`, + }, + publishConfig: { + registry: alternateRegistry, + }, + }, + }, + _attachments: { + [`${pkg}-${version}.tgz`]: {}, + }, +}) + module.exports = setupMockNpm module.exports.load = setupMockNpm module.exports.setGlobalNodeModules = setGlobalNodeModules module.exports.loadNpmWithRegistry = loadNpmWithRegistry module.exports.workspaceMock = workspaceMock module.exports.mockNpmRegistryFetch = mockNpmRegistryFetch +module.exports.putPackagePayload = putPackagePayload diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 5462b30cb7a79..40d436b1a48ae 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1,5 +1,8 @@ const t = require('tap') -const { load: loadMockNpm } = require('../../fixtures/mock-npm') +const { + load: originalLoadMockNpm, + mockNpmRegistryFetch, + putPackagePayload } = require('../../fixtures/mock-npm') const { cleanZlib } = require('../../fixtures/clean-snapshot') const MockRegistry = require('@npmcli/mock-registry') const pacote = require('pacote') @@ -22,6 +25,20 @@ const pkgJson = { t.cleanSnapshot = data => cleanZlib(data) +function loadMockNpm (test, args) { + return originalLoadMockNpm(test, { + ...args, + mocks: { + ...mockNpmRegistryFetch({ + [`/-/package/${pkg}/dist-tags`]: () => { + throw new Error('not found') + }, + }).mocks, + ...args.mocks, + }, + }) +} + t.test('respects publishConfig.registry, runs appropriate scripts', async t => { const { npm, joinedOutput, prefix } = await loadMockNpm(t, { config: { @@ -1069,3 +1086,105 @@ t.test('does not abort when prerelease and authored tag latest', async t => { }).reply(200, {}) await npm.exec('publish', []) }) + +t.test('PREVENTS publish when latest dist-tag is HIGHER than publishing version', async t => { + const latest = '100.0.0' + const version = '50.0.0' + + const { npm } = await loadMockNpm(t, { + config: { + loglevel: 'silent', + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + ...pkgJson, + version, + scripts: { + prepublishOnly: 'touch scripts-prepublishonly', + prepublish: 'touch scripts-prepublish', // should NOT run this one + publish: 'touch scripts-publish', + postpublish: 'touch scripts-postpublish', + }, + publishConfig: { registry: alternateRegistry }, + }, null, 2), + }, + mocks: { + ...mockNpmRegistryFetch({ + [`/-/package/${pkg}/dist-tags`]: { latest }, + }).mocks, + }, + }) + await t.rejects(async () => { + await npm.exec('publish', []) + }, new Error('Cannot publish a lower version without an explicit dist tag.')) +}) + +t.test('ALLOWS publish when latest dist-tag is LOWER than publishing version', async t => { + const version = '100.0.0' + const latest = '50.0.0' + + const { npm } = await loadMockNpm(t, { + config: { + loglevel: 'silent', + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + ...pkgJson, + version, + publishConfig: { registry: alternateRegistry }, + }, null, 2), + }, + mocks: { + ...mockNpmRegistryFetch({ + [`/-/package/${pkg}/dist-tags`]: { latest }, + }).mocks, + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + authorization: 'test-other-token', + }) + registry.nock.put(`/${pkg}`, body => { + return t.match(body, putPackagePayload({ + pkg, alternateRegistry, version, + })) + }).reply(200, {}) + await npm.exec('publish', []) +}) + +t.test('ALLOWS publish when latest dist-tag is missing from response', async t => { + const version = '100.0.0' + + const { npm } = await loadMockNpm(t, { + config: { + loglevel: 'silent', + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + ...pkgJson, + version, + publishConfig: { registry: alternateRegistry }, + }, null, 2), + }, + mocks: { + ...mockNpmRegistryFetch({ + [`/-/package/${pkg}/dist-tags`]: { }, + }).mocks, + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + authorization: 'test-other-token', + }) + registry.nock.put(`/${pkg}`, body => { + return t.match(body, putPackagePayload({ + pkg, alternateRegistry, version, + })) + }).reply(200, {}) + await npm.exec('publish', []) +}) From e2f44550c66733fa5a6114d9920eadf12fbb6489 Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 2 Dec 2024 12:05:41 -0500 Subject: [PATCH 04/20] feat: uses packument to determine semver latest --- lib/commands/publish.js | 22 +++++++++++++++++++++- test/lib/commands/publish.js | 14 ++++++-------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 7be3cfe1d0f0d..08547b2163729 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -132,7 +132,7 @@ class Publish extends BaseCommand { const resolved = npa.resolve(manifest.name, manifest.version) - const latestDistTag = await this.#latestDistTag(resolved) + const latestDistTag = await this.#latestPublishedVersion(resolved) const latestTagIsGreater = latestDistTag ? semver.gte(latestDistTag, manifest.version) : false if (latestTagIsGreater && isDefaultTag) { @@ -204,6 +204,26 @@ class Publish extends BaseCommand { } } + async #latestPublishedVersion (spec) { + try { + const uri = '/' + spec.escapedName + const packument = await npmFetch.json(uri, { + ...this.npm.flatOptions, + spec, + }) + if (typeof packument?.versions === 'undefined') { + return null + } + const ordered = Object.keys(packument?.versions) + .map(v => new semver.SemVer(v)) + .filter(v => !v.prerelease.length) + .sort((a, b) => b.compare(a)) + return ordered.length >= 1 ? ordered[0].version : null + } catch (e) { + return null + } + } + async #latestDistTag (spec) { try { const tags = await DistTag.fetchTags(spec, this.npm.flatOptions) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 40d436b1a48ae..a6bae596101d9 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -30,7 +30,7 @@ function loadMockNpm (test, args) { ...args, mocks: { ...mockNpmRegistryFetch({ - [`/-/package/${pkg}/dist-tags`]: () => { + [`/${pkg}`]: () => { throw new Error('not found') }, }).mocks, @@ -1088,7 +1088,6 @@ t.test('does not abort when prerelease and authored tag latest', async t => { }) t.test('PREVENTS publish when latest dist-tag is HIGHER than publishing version', async t => { - const latest = '100.0.0' const version = '50.0.0' const { npm } = await loadMockNpm(t, { @@ -1111,7 +1110,7 @@ t.test('PREVENTS publish when latest dist-tag is HIGHER than publishing version' }, mocks: { ...mockNpmRegistryFetch({ - [`/-/package/${pkg}/dist-tags`]: { latest }, + [`/${pkg}`]: { versions: { '50.0.0': {}, '99.0.0': {}, '100.0.0': {}, '101.0.0-pre': {} } }, }).mocks, }, }) @@ -1120,9 +1119,8 @@ t.test('PREVENTS publish when latest dist-tag is HIGHER than publishing version' }, new Error('Cannot publish a lower version without an explicit dist tag.')) }) -t.test('ALLOWS publish when latest dist-tag is LOWER than publishing version', async t => { +t.test('ALLOWS publish when latest versions are LOWER than publishing version', async t => { const version = '100.0.0' - const latest = '50.0.0' const { npm } = await loadMockNpm(t, { config: { @@ -1138,7 +1136,7 @@ t.test('ALLOWS publish when latest dist-tag is LOWER than publishing version', a }, mocks: { ...mockNpmRegistryFetch({ - [`/-/package/${pkg}/dist-tags`]: { latest }, + [`/${pkg}`]: { versions: { '50.0.0': {}, '99.0.0': {}, '101.0.0-pre': {} } }, }).mocks, }, }) @@ -1155,7 +1153,7 @@ t.test('ALLOWS publish when latest dist-tag is LOWER than publishing version', a await npm.exec('publish', []) }) -t.test('ALLOWS publish when latest dist-tag is missing from response', async t => { +t.test('ALLOWS publish when not published yet', async t => { const version = '100.0.0' const { npm } = await loadMockNpm(t, { @@ -1172,7 +1170,7 @@ t.test('ALLOWS publish when latest dist-tag is missing from response', async t = }, mocks: { ...mockNpmRegistryFetch({ - [`/-/package/${pkg}/dist-tags`]: { }, + [`/${pkg}`]: { }, }).mocks, }, }) From 51651ef683e56caeeade91832e14529e099aea03 Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 2 Dec 2024 12:10:30 -0500 Subject: [PATCH 05/20] chore: remove unused function --- lib/commands/publish.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 08547b2163729..12614c335c4b7 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -224,16 +224,6 @@ class Publish extends BaseCommand { } } - async #latestDistTag (spec) { - try { - const tags = await DistTag.fetchTags(spec, this.npm.flatOptions) - return tags.latest - } catch (_e) { - // this will fail if the package is new, so just return null - return null - } - } - // if it's a directory, read it from the file system // otherwise, get the full metadata from whatever it is // XXX can't pacote read the manifest from a directory? From f47a514c38af5184ab076ab4ebfc29ba9ce93968 Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 2 Dec 2024 12:13:14 -0500 Subject: [PATCH 06/20] fix: cover empty versions --- test/lib/commands/publish.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index a6bae596101d9..1428260a97a5b 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1186,3 +1186,37 @@ t.test('ALLOWS publish when not published yet', async t => { }).reply(200, {}) await npm.exec('publish', []) }) + +t.test('ALLOWS publish when not published yet (no versions)', async t => { + const version = '100.0.0' + + const { npm } = await loadMockNpm(t, { + config: { + loglevel: 'silent', + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + ...pkgJson, + version, + publishConfig: { registry: alternateRegistry }, + }, null, 2), + }, + mocks: { + ...mockNpmRegistryFetch({ + [`/${pkg}`]: { versions: {} }, + }).mocks, + }, + }) + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + authorization: 'test-other-token', + }) + registry.nock.put(`/${pkg}`, body => { + return t.match(body, putPackagePayload({ + pkg, alternateRegistry, version, + })) + }).reply(200, {}) + await npm.exec('publish', []) +}) From 6f2a724e1605103be665f6c8f200ba079e94dd9d Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 2 Dec 2024 12:13:30 -0500 Subject: [PATCH 07/20] fix: rm unused import --- lib/commands/publish.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 12614c335c4b7..c1426231397a1 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -16,7 +16,6 @@ const { getContents, logTar } = require('../utils/tar.js') const { flatten } = require('@npmcli/config/lib/definitions') const pkgJson = require('@npmcli/package-json') const BaseCommand = require('../base-cmd.js') -const DistTag = require('./dist-tag.js') class Publish extends BaseCommand { static description = 'Publish a package' From 93fcf15c9ca1e0128067d54e1765440103eb3a2b Mon Sep 17 00:00:00 2001 From: reggi Date: Mon, 2 Dec 2024 12:26:56 -0500 Subject: [PATCH 08/20] fix: smoke --- smoke-tests/test/npm-replace-global.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 687c540375b86..563aba4d39170 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -136,7 +136,7 @@ t.test('publish and replace global self', async t => { if (setup.SMOKE_PUBLISH) { await npmPackage() } - registry.nock.get('/-/package/npm/dist-tags').reply(404, 'not found') + registry.nock.get('/npm').reply(404, 'not found') registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] From 0f7f1afc731753b4e08268049b8447905fd8516c Mon Sep 17 00:00:00 2001 From: Reggi Date: Mon, 2 Dec 2024 13:22:01 -0500 Subject: [PATCH 09/20] fix: dist tag variable name Co-authored-by: Jordan Harband --- lib/commands/publish.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index c1426231397a1..d3a43fc07a784 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -131,8 +131,8 @@ class Publish extends BaseCommand { const resolved = npa.resolve(manifest.name, manifest.version) - const latestDistTag = await this.#latestPublishedVersion(resolved) - const latestTagIsGreater = latestDistTag ? semver.gte(latestDistTag, manifest.version) : false + const latestVersion = await this.#latestPublishedVersion(resolved) + const latestTagIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) if (latestTagIsGreater && isDefaultTag) { throw new Error('Cannot publish a lower version without an explicit dist tag.') From 97abe180ef14efc385cb4c887f9f8d0b9470c93a Mon Sep 17 00:00:00 2001 From: Reggi Date: Mon, 2 Dec 2024 13:26:05 -0500 Subject: [PATCH 10/20] fix: optimize .map.filter with .flatMap Co-authored-by: Jordan Harband --- lib/commands/publish.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index d3a43fc07a784..bf936bea5a086 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -214,8 +214,10 @@ class Publish extends BaseCommand { return null } const ordered = Object.keys(packument?.versions) - .map(v => new semver.SemVer(v)) - .filter(v => !v.prerelease.length) + .flatMap(v => { + const s = new semver.SemVer(v) + return s.prerelease.length > 0 ? [] : s; + }) .sort((a, b) => b.compare(a)) return ordered.length >= 1 ? ordered[0].version : null } catch (e) { From eae9974559e0199f252fd2d20add502f07de4f60 Mon Sep 17 00:00:00 2001 From: Reggi Date: Mon, 2 Dec 2024 13:34:57 -0500 Subject: [PATCH 11/20] fix: rm semicolin --- lib/commands/publish.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index bf936bea5a086..62ec2cc255c4f 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -216,7 +216,7 @@ class Publish extends BaseCommand { const ordered = Object.keys(packument?.versions) .flatMap(v => { const s = new semver.SemVer(v) - return s.prerelease.length > 0 ? [] : s; + return s.prerelease.length > 0 ? [] : s }) .sort((a, b) => b.compare(a)) return ordered.length >= 1 ? ordered[0].version : null From 2340e234c4ceef693bf2cf66bc929f24a96a4eeb Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 15:22:49 -0500 Subject: [PATCH 12/20] fix: tests, strict nock, add packument calls --- lib/commands/dist-tag.js | 8 +- lib/commands/publish.js | 12 +- test/fixtures/mock-npm.js | 74 ++-- test/lib/commands/publish.js | 721 +++++++++++------------------------ 4 files changed, 288 insertions(+), 527 deletions(-) diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index 4274b982d32fb..3fdecd926a564 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -102,7 +102,7 @@ class DistTag extends BaseCommand { throw new Error('Tag name must not be a valid SemVer range: ' + t) } - const tags = await DistTag.fetchTags(spec, opts) + const tags = await this.fetchTags(spec, opts) if (tags[t] === version) { log.warn('dist-tag add', t, 'is already set to version', version) return @@ -131,7 +131,7 @@ class DistTag extends BaseCommand { throw this.usageError() } - const tags = await DistTag.fetchTags(spec, opts) + const tags = await this.fetchTags(spec, opts) if (!tags[tag]) { log.info('dist-tag del', tag, 'is not a dist-tag on', spec.name) throw new Error(tag + ' is not a dist-tag on ' + spec.name) @@ -164,7 +164,7 @@ class DistTag extends BaseCommand { spec = npa(spec) try { - const tags = await DistTag.fetchTags(spec, opts) + const tags = await this.fetchTags(spec, opts) const msg = Object.keys(tags).map(k => `${k}: ${tags[k]}`).sort().join('\n') output.standard(msg) @@ -190,7 +190,7 @@ class DistTag extends BaseCommand { } } - static async fetchTags (spec, opts) { + async fetchTags (spec, opts) { const data = await npmFetch.json( `/-/package/${spec.escapedName}/dist-tags`, { ...opts, 'prefer-online': true, spec } diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 62ec2cc255c4f..c02535f089e3c 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -130,8 +130,9 @@ class Publish extends BaseCommand { } const resolved = npa.resolve(manifest.name, manifest.version) + const registry = npmFetch.pickRegistry(resolved, opts) - const latestVersion = await this.#latestPublishedVersion(resolved) + const latestVersion = await this.#latestPublishedVersion(resolved, registry) const latestTagIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) if (latestTagIsGreater && isDefaultTag) { @@ -141,7 +142,6 @@ class Publish extends BaseCommand { // make sure tag is valid, this will throw if invalid npa(`${manifest.name}@${defaultTag}`) - const registry = npmFetch.pickRegistry(resolved, opts) const creds = this.npm.config.getCredentialsByURI(registry) const noCreds = !(creds.token || creds.username || creds.certfile && creds.keyfile) const outputRegistry = replaceInfo(registry) @@ -203,12 +203,12 @@ class Publish extends BaseCommand { } } - async #latestPublishedVersion (spec) { + async #latestPublishedVersion (spec, registry) { try { - const uri = '/' + spec.escapedName - const packument = await npmFetch.json(uri, { + const packument = await pacote.packument(spec, { ...this.npm.flatOptions, - spec, + preferOnline: true, + registry, }) if (typeof packument?.versions === 'undefined') { return null diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 486f8e75104b6..e385f9053335d 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -293,12 +293,26 @@ const setupMockNpm = async (t, { const loadNpmWithRegistry = async (t, opts) => { const mock = await setupMockNpm(t, opts) + return { + ...mock, + ...loadRegistry(t, mock, opts), + ...loadFsAssertions(t, mock), + } +} + +const loadRegistry = (t, mock, opts) => { const registry = new MockRegistry({ tap: t, - registry: mock.npm.config.get('registry'), - strict: true, + registry: opts.registry ?? mock.npm.config.get('registry'), + authorization: opts.authorization, + basic: opts.basic, + debug: opts.debugRegistry ?? false, + strict: opts.strictRegistryNock ?? true, }) + return { registry } +} +const loadFsAssertions = (t, mock) => { const fileShouldExist = (filePath) => { t.equal( fsSync.existsSync(path.join(mock.npm.prefix, filePath)), true, `${filePath} should exist` @@ -353,7 +367,7 @@ const loadNpmWithRegistry = async (t, opts) => { packageDirty, } - return { registry, assert, ...mock } + return { assert } } /** breaks down a spec "abbrev@1.1.1" into different parts for mocking */ @@ -478,29 +492,41 @@ const mockNpmRegistryFetch = (tags) => { return { mocks, mock, fetchOpts, getOpts } } -const putPackagePayload = ({ pkg, alternateRegistry, version }) => ({ - _id: pkg, - name: pkg, - 'dist-tags': { latest: version }, - access: null, - versions: { - [version]: { - name: pkg, - version: version, - _id: `${pkg}@${version}`, - dist: { - shasum: /\.*/, - tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-${version}.tgz`, - }, - publishConfig: { - registry: alternateRegistry, +const putPackagePayload = (opts) => { + const package = opts.packageJson + const name = opts.name || package?.name + const registry = opts.registry || package?.publishConfig?.registry || 'https://registry.npmjs.org' + const access = opts.access || null + + const nameProperties = !name ? {} : { + _id: name, + name: name, + } + + const packageProperties = !package ? {} : { + 'dist-tags': { latest: package.version }, + versions: { + [package.version]: { + _id: `${package.name}@${package.version}`, + dist: { + shasum: /\.*/, + tarball: + `http://${new URL(registry).host}/${package.name}/-/${package.name}-${package.version}.tgz`, + }, + ...package, }, }, - }, - _attachments: { - [`${pkg}-${version}.tgz`]: {}, - }, -}) + _attachments: { + [`${package.name}-${package.version}.tgz`]: {}, + }, + } + + return { + access, + ...nameProperties, + ...packageProperties, + } +} module.exports = setupMockNpm module.exports.load = setupMockNpm diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 1428260a97a5b..a3e1b66be5273 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1,10 +1,6 @@ const t = require('tap') -const { - load: originalLoadMockNpm, - mockNpmRegistryFetch, - putPackagePayload } = require('../../fixtures/mock-npm') +const { loadNpmWithRegistry, putPackagePayload } = require('../../fixtures/mock-npm') const { cleanZlib } = require('../../fixtures/clean-snapshot') -const MockRegistry = require('@npmcli/mock-registry') const pacote = require('pacote') const Arborist = require('@npmcli/arborist') const path = require('node:path') @@ -23,71 +19,51 @@ const pkgJson = { version: '1.0.0', } -t.cleanSnapshot = data => cleanZlib(data) - -function loadMockNpm (test, args) { - return originalLoadMockNpm(test, { - ...args, - mocks: { - ...mockNpmRegistryFetch({ - [`/${pkg}`]: () => { - throw new Error('not found') - }, - }).mocks, - ...args.mocks, - }, - }) +const packageNock = (t, registry, name, { + packageJson, access, noPut, putStatus, manifest, +} = {}) => { + const spec = npa(name) + if (!manifest) { + registry.nock.get(`/${spec.escapedName}`).reply(404, 'not found') + registry.nock.get(`/${spec.escapedName}`).reply(404, 'not found') + } else { + registry.nock.get(`/${spec.escapedName}`).reply(200, manifest) + } + if (!noPut) { + registry.nock.put(`/${spec.escapedName}`, body => { + return t.match(body, putPackagePayload({ name, packageJson, access })) + }).reply(putStatus ?? 200, {}) + } + return registry.nock } +const assertPublishNock = packageNock + +t.cleanSnapshot = data => cleanZlib(data) + t.test('respects publishConfig.registry, runs appropriate scripts', async t => { - const { npm, joinedOutput, prefix } = await loadMockNpm(t, { + const packageJson = { + ...pkgJson, + scripts: { + prepublishOnly: 'touch scripts-prepublishonly', + prepublish: 'touch scripts-prepublish', // should NOT run this one + publish: 'touch scripts-publish', + postpublish: 'touch scripts-postpublish', + }, + publishConfig: { registry: alternateRegistry }, + } + const { npm, joinedOutput, prefix, registry } = await loadNpmWithRegistry(t, { config: { loglevel: 'silent', [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', }, prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - scripts: { - prepublishOnly: 'touch scripts-prepublishonly', - prepublish: 'touch scripts-prepublish', // should NOT run this one - publish: 'touch scripts-publish', - postpublish: 'touch scripts-postpublish', - }, - publishConfig: { registry: alternateRegistry }, - }, null, 2), + 'package.json': JSON.stringify(packageJson, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, registry: alternateRegistry, authorization: 'test-other-token', }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, { - _id: pkg, - name: pkg, - 'dist-tags': { latest: '1.0.0' }, - access: null, - versions: { - '1.0.0': { - name: pkg, - version: '1.0.0', - _id: `${pkg}@1.0.0`, - dist: { - shasum: /\.*/, - tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-1.0.0.tgz`, - }, - publishConfig: { - registry: alternateRegistry, - }, - }, - }, - _attachments: { - [`${pkg}-1.0.0.tgz`]: {}, - }, - }) - }).reply(200, {}) + assertPublishNock(t, registry, pkg, { packageJson }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.equal(fs.existsSync(path.join(prefix, 'scripts-prepublishonly')), true, 'ran prepublishOnly') @@ -97,115 +73,66 @@ t.test('respects publishConfig.registry, runs appropriate scripts', async t => { }) t.test('re-loads publishConfig.registry if added during script process', async t => { - const { joinedOutput, npm } = await loadMockNpm(t, { + const initPackageJson = { + ...pkgJson, + scripts: { + prepare: 'cp new.json package.json', + }, + } + const packageJson = { + ...initPackageJson, + publishConfig: { registry: alternateRegistry }, + } + const { joinedOutput, npm, registry } = await loadNpmWithRegistry(t, { config: { [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', // Keep output from leaking into tap logs for readability 'foreground-scripts': false, }, prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - scripts: { - prepare: 'cp new.json package.json', - }, - }, null, 2), - 'new.json': JSON.stringify({ - ...pkgJson, - publishConfig: { registry: alternateRegistry }, - }), + 'package.json': JSON.stringify(initPackageJson, null, 2), + 'new.json': JSON.stringify(packageJson, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, registry: alternateRegistry, authorization: 'test-other-token', }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, { - _id: pkg, - name: pkg, - 'dist-tags': { latest: '1.0.0' }, - access: null, - versions: { - '1.0.0': { - name: pkg, - version: '1.0.0', - _id: `${pkg}@1.0.0`, - dist: { - shasum: /\.*/, - tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-1.0.0.tgz`, - }, - publishConfig: { - registry: alternateRegistry, - }, - }, - }, - _attachments: { - [`${pkg}-1.0.0.tgz`]: {}, - }, - }) - }).reply(200, {}) + assertPublishNock(t, registry, pkg, { packageJson }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) t.test('prioritize CLI flags over publishConfig', async t => { - const publishConfig = { registry: 'http://publishconfig' } - const { joinedOutput, npm } = await loadMockNpm(t, { + const initPackageJson = { + ...pkgJson, + scripts: { + prepare: 'cp new.json package.json', + }, + } + const packageJson = { + ...initPackageJson, + publishConfig: { registry: alternateRegistry }, + } + const { joinedOutput, npm, registry } = await loadNpmWithRegistry(t, { config: { [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', // Keep output from leaking into tap logs for readability 'foreground-scripts': false, }, prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - scripts: { - prepare: 'cp new.json package.json', - }, - }, null, 2), - 'new.json': JSON.stringify({ - ...pkgJson, - publishConfig, - }), + 'package.json': JSON.stringify(initPackageJson, null, 2), + 'new.json': JSON.stringify(packageJson, null, 2), }, argv: ['--registry', alternateRegistry], - }) - const registry = new MockRegistry({ - tap: t, - registry: alternateRegistry, + registryUrl: alternateRegistry, authorization: 'test-other-token', }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, { - _id: pkg, - name: pkg, - 'dist-tags': { latest: '1.0.0' }, - access: null, - versions: { - '1.0.0': { - name: pkg, - version: '1.0.0', - _id: `${pkg}@1.0.0`, - dist: { - shasum: /\.*/, - tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-1.0.0.tgz`, - }, - publishConfig, - }, - }, - _attachments: { - [`${pkg}-1.0.0.tgz`]: {}, - }, - }) - }).reply(200, {}) + assertPublishNock(t, registry, pkg, { packageJson }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) t.test('json', async t => { - const { joinedOutput, npm, logs } = await loadMockNpm(t, { + const { joinedOutput, npm, logs, registry } = await loadNpmWithRegistry(t, { config: { json: true, ...auth, @@ -213,20 +140,16 @@ t.test('json', async t => { prefixDir: { 'package.json': JSON.stringify(pkgJson, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock.put(`/${pkg}`).reply(200, {}) + assertPublishNock(t, registry, pkg) await npm.exec('publish', []) t.matchSnapshot(logs.notice) t.matchSnapshot(joinedOutput(), 'new package json') }) t.test('dry-run', async t => { - const { joinedOutput, npm, logs } = await loadMockNpm(t, { + const { joinedOutput, npm, logs, registry } = await loadNpmWithRegistry(t, { config: { 'dry-run': true, ...auth, @@ -234,14 +157,16 @@ t.test('dry-run', async t => { prefixDir: { 'package.json': JSON.stringify(pkgJson, null, 2), }, + authorization: token, }) + assertPublishNock(t, registry, pkg, { noPut: true }) await npm.exec('publish', []) t.equal(joinedOutput(), `+ ${pkg}@1.0.0`) t.matchSnapshot(logs.notice) }) t.test('foreground-scripts defaults to true', async t => { - const { outputs, npm, logs } = await loadMockNpm(t, { + const { outputs, npm, logs, registry } = await loadNpmWithRegistry(t, { config: { 'dry-run': true, ...auth, @@ -258,11 +183,9 @@ t.test('foreground-scripts defaults to true', async t => { ), }, }) - + assertPublishNock(t, registry, 'test-fg-scripts', { noPut: true }) await npm.exec('publish', []) - t.matchSnapshot(logs.notice) - t.strictSame( outputs, [ @@ -274,7 +197,7 @@ t.test('foreground-scripts defaults to true', async t => { }) t.test('foreground-scripts can still be set to false', async t => { - const { outputs, npm, logs } = await loadMockNpm(t, { + const { outputs, npm, logs, registry } = await loadNpmWithRegistry(t, { config: { 'dry-run': true, 'foreground-scripts': false, @@ -293,6 +216,7 @@ t.test('foreground-scripts can still be set to false', async t => { }, }) + assertPublishNock(t, registry, 'test-fg-scripts', { noPut: true }) await npm.exec('publish', []) t.matchSnapshot(logs.notice) @@ -304,12 +228,12 @@ t.test('foreground-scripts can still be set to false', async t => { }) t.test('shows usage with wrong set of arguments', async t => { - const { publish } = await loadMockNpm(t, { command: 'publish' }) + const { publish } = await loadNpmWithRegistry(t, { command: 'publish' }) await t.rejects(publish.exec(['a', 'b', 'c']), publish.usage) }) t.test('throws when invalid tag is semver', async t => { - const { npm } = await loadMockNpm(t, { + const { npm } = await loadNpmWithRegistry(t, { config: { tag: '0.0.13', }, @@ -324,7 +248,7 @@ t.test('throws when invalid tag is semver', async t => { }) t.test('throws when invalid tag when not url encodable', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { config: { tag: '@test', }, @@ -332,6 +256,8 @@ t.test('throws when invalid tag when not url encodable', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) + assertPublishNock(t, registry, pkg, { noPut: true }) + await t.rejects( npm.exec('publish', []), { @@ -342,7 +268,7 @@ t.test('throws when invalid tag when not url encodable', async t => { }) t.test('tarball', async t => { - const { npm, joinedOutput, logs, home } = await loadMockNpm(t, { + const { npm, joinedOutput, logs, home, registry } = await loadNpmWithRegistry(t, { config: { 'fetch-retries': 0, ...auth, @@ -355,31 +281,24 @@ t.test('tarball', async t => { }, null, 2), 'index.js': 'console.log("hello world"}', }, + authorization: token, }) const tarball = await pacote.tarball(home, { Arborist }) const tarFilename = path.join(home, 'tarball.tgz') fs.writeFileSync(tarFilename, tarball) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), - authorization: token, - }) - registry.nock.put('/test-tar-package', body => { - return t.match(body, { - name: 'test-tar-package', - }) - }).reply(200, {}) + assertPublishNock(t, registry, 'test-tar-package') await npm.exec('publish', [tarFilename]) t.matchSnapshot(logs.notice) t.matchSnapshot(joinedOutput(), 'new package json') }) t.test('no auth default registry', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { prefixDir: { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) + assertPublishNock(t, registry, pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -390,7 +309,7 @@ t.test('no auth default registry', async t => { }) t.test('no auth dry-run', async t => { - const { npm, joinedOutput, logs } = await loadMockNpm(t, { + const { npm, joinedOutput, logs, registry } = await loadNpmWithRegistry(t, { config: { 'dry-run': true, }, @@ -398,13 +317,14 @@ t.test('no auth dry-run', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) + assertPublishNock(t, registry, pkg, { noPut: true }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput()) t.matchSnapshot(logs.warn, 'warns about auth being needed') }) t.test('no auth for configured registry', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { config: { registry: alternateRegistry, ...auth, @@ -413,6 +333,7 @@ t.test('no auth for configured registry', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) + assertPublishNock(t, registry, pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -423,7 +344,7 @@ t.test('no auth for configured registry', async t => { }) t.test('no auth for scope configured registry', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -436,6 +357,7 @@ t.test('no auth for scope configured registry', async t => { }, null, 2), }, }) + assertPublishNock(t, registry, '@npm/test-package', { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -446,8 +368,7 @@ t.test('no auth for scope configured registry', async t => { }) t.test('has token auth for scope configured registry', async t => { - const spec = npa('@npm/test-package') - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -459,22 +380,16 @@ t.test('has token auth for scope configured registry', async t => { version: '1.0.0', }, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, registry: alternateRegistry, authorization: 'test-scope-token', }) - registry.nock.put(`/${spec.escapedName}`, body => { - return t.match(body, { name: '@npm/test-package' }) - }).reply(200, {}) + assertPublishNock(t, registry, '@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) t.test('has mTLS auth for scope configured registry', async t => { - const spec = npa('@npm/test-package') - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -487,14 +402,9 @@ t.test('has mTLS auth for scope configured registry', async t => { version: '1.0.0', }, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, registry: alternateRegistry, }) - registry.nock.put(`/${spec.escapedName}`, body => { - return t.match(body, { name: '@npm/test-package' }) - }).reply(200, {}) + assertPublishNock(t, registry, '@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -536,7 +446,7 @@ t.test('workspaces', t => { } t.test('all workspaces - no color', async t => { - const { npm, joinedOutput, logs } = await loadMockNpm(t, { + const { npm, joinedOutput, logs, registry } = await loadNpmWithRegistry(t, { config: { tag: 'latest', color: false, @@ -544,29 +454,19 @@ t.test('workspaces', t => { workspaces: true, }, prefixDir: dir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/workspace-a', body => { - return t.match(body, { name: 'workspace-a' }) - }).reply(200, {}) - .put('/workspace-b', body => { - return t.match(body, { name: 'workspace-b' }) - }).reply(200, {}) - .put('/workspace-n', body => { - return t.match(body, { name: 'workspace-n' }) - }).reply(200, {}) + assertPublishNock(t, registry, 'workspace-p', { noPut: true }) + ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { + assertPublishNock(t, registry, name) + }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'all public workspaces') t.matchSnapshot(logs.warn, 'warns about skipped private workspace') }) t.test('all workspaces - color', async t => { - const { npm, joinedOutput, logs } = await loadMockNpm(t, { + const { npm, joinedOutput, logs, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', @@ -574,67 +474,47 @@ t.test('workspaces', t => { workspaces: true, }, prefixDir: dir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/workspace-a', body => { - return t.match(body, { name: 'workspace-a' }) - }).reply(200, {}) - .put('/workspace-b', body => { - return t.match(body, { name: 'workspace-b' }) - }).reply(200, {}) - .put('/workspace-n', body => { - return t.match(body, { name: 'workspace-n' }) - }).reply(200, {}) + assertPublishNock(t, registry, 'workspace-p', { noPut: true }) + ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { + assertPublishNock(t, registry, name) + }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'all public workspaces') t.matchSnapshot(logs.warn, 'warns about skipped private workspace in color') }) t.test('one workspace - success', async t => { - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', workspace: ['workspace-a'], }, prefixDir: dir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/workspace-a', body => { - return t.match(body, { name: 'workspace-a' }) - }).reply(200, {}) + ;['workspace-a'].forEach(name => { + assertPublishNock(t, registry, name) + }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'single workspace') }) t.test('one workspace - failure', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', workspace: ['workspace-a'], }, prefixDir: dir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/workspace-a', body => { - return t.match(body, { name: 'workspace-a' }) - }).reply(404, {}) + + assertPublishNock(t, registry, 'workspace-a', { putStatus: 404 }) + await t.rejects(npm.exec('publish', []), { code: 'E404' }) }) @@ -660,29 +540,23 @@ t.test('workspaces', t => { }, } - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', workspaces: true, }, prefixDir: testDir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/workspace-a', body => { - return t.match(body, { name: 'workspace-a' }) - }).reply(200, {}) + assertPublishNock(t, registry, '@scoped/workspace-p', { noPut: true }) + assertPublishNock(t, registry, 'workspace-a') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'one marked private') }) t.test('invalid workspace', async t => { - const { npm } = await loadMockNpm(t, { + const { npm } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', @@ -697,7 +571,7 @@ t.test('workspaces', t => { }) t.test('json', async t => { - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', @@ -705,22 +579,12 @@ t.test('workspaces', t => { json: true, }, prefixDir: dir, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/workspace-a', body => { - return t.match(body, { name: 'workspace-a' }) - }).reply(200, {}) - .put('/workspace-b', body => { - return t.match(body, { name: 'workspace-b' }) - }).reply(200, {}) - .put('/workspace-n', body => { - return t.match(body, { name: 'workspace-n' }) - }).reply(200, {}) + assertPublishNock(t, registry, 'workspace-p', { noPut: true }) + ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { + assertPublishNock(t, registry, name) + }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'all workspaces in json') }) @@ -746,23 +610,16 @@ t.test('workspaces', t => { }, } - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', }, prefixDir: testDir, chdir: ({ prefix }) => path.resolve(prefix, './workspace-a'), - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock - .put('/pkg', body => { - return t.match(body, { name: 'pkg' }) - }).reply(200, {}) + assertPublishNock(t, registry, 'pkg') await npm.exec('publish', ['../dir/pkg']) t.matchSnapshot(joinedOutput(), 'publish different package spec') }) @@ -771,7 +628,7 @@ t.test('workspaces', t => { }) t.test('ignore-scripts', async t => { - const { npm, joinedOutput, prefix } = await loadMockNpm(t, { + const { npm, joinedOutput, prefix, registry } = await loadNpmWithRegistry(t, { config: { ...auth, 'ignore-scripts': true, @@ -787,13 +644,9 @@ t.test('ignore-scripts', async t => { }, }, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock.put(`/${pkg}`).reply(200, {}) + assertPublishNock(t, registry, pkg) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.equal( @@ -819,27 +672,22 @@ t.test('ignore-scripts', async t => { }) t.test('_auth config default registry', async t => { - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { '//registry.npmjs.org/:_auth': basic, }, prefixDir: { 'package.json': JSON.stringify(pkgJson), }, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), basic, }) - registry.nock.put(`/${pkg}`).reply(200, {}) + assertPublishNock(t, registry, pkg) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) t.test('bare _auth and registry config', async t => { - const spec = npa('@npm/test-package') - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { registry: alternateRegistry, '//other.registry.npmjs.org/:_auth': basic, @@ -850,19 +698,16 @@ t.test('bare _auth and registry config', async t => { version: '1.0.0', }, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, registry: alternateRegistry, basic, }) - registry.nock.put(`/${spec.escapedName}`).reply(200, {}) + assertPublishNock(t, registry, '@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) t.test('bare _auth config scoped registry', async t => { - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -875,6 +720,7 @@ t.test('bare _auth config scoped registry', async t => { }, null, 2), }, }) + assertPublishNock(t, registry, '@npm/test-package', { noPut: true }) await t.rejects( npm.exec('publish', []), { message: `This command requires you to be logged in to ${alternateRegistry}` } @@ -882,8 +728,7 @@ t.test('bare _auth config scoped registry', async t => { }) t.test('scoped _auth config scoped registry', async t => { - const spec = npa('@npm/test-package') - const { npm, joinedOutput } = await loadMockNpm(t, { + const { npm, joinedOutput, registry } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -895,48 +740,37 @@ t.test('scoped _auth config scoped registry', async t => { version: '1.0.0', }, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, registry: alternateRegistry, basic, }) - registry.nock.put(`/${spec.escapedName}`).reply(200, {}) + assertPublishNock(t, registry, '@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) t.test('restricted access', async t => { - const spec = npa('@npm/test-package') - const { npm, joinedOutput, logs } = await loadMockNpm(t, { + const packageJson = { + name: '@npm/test-package', + version: '1.0.0', + } + const { npm, joinedOutput, logs, registry } = await loadNpmWithRegistry(t, { config: { ...auth, access: 'restricted', }, prefixDir: { - 'package.json': JSON.stringify({ - name: '@npm/test-package', - version: '1.0.0', - }, null, 2), + 'package.json': JSON.stringify(packageJson, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock.put(`/${spec.escapedName}`, body => { - t.equal(body.access, 'restricted', 'access is explicitly set to restricted') - return true - }).reply(200, {}) + assertPublishNock(t, registry, '@npm/test-package', { packageJson, access: 'restricted' }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.matchSnapshot(logs.notice) }) t.test('public access', async t => { - const spec = npa('@npm/test-package') - const { npm, joinedOutput, logs } = await loadMockNpm(t, { + const { npm, joinedOutput, logs, registry } = await loadNpmWithRegistry(t, { config: { ...auth, access: 'public', @@ -947,16 +781,9 @@ t.test('public access', async t => { version: '1.0.0', }, null, 2), }, - }) - const registry = new MockRegistry({ - tap: t, - registry: npm.config.get('registry'), authorization: token, }) - registry.nock.put(`/${spec.escapedName}`, body => { - t.equal(body.access, 'public', 'access is explicitly set to public') - return true - }).reply(200, {}) + assertPublishNock(t, registry, '@npm/test-package', { access: 'public' }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.matchSnapshot(logs.notice) @@ -976,7 +803,7 @@ t.test('manifest', async t => { t.cleanSnapshot = (s) => s.replace(new RegExp(npmPkg.version, 'g'), '{VERSION}') let manifest = null - const { npm } = await loadMockNpm(t, { + const { npm, registry } = await loadNpmWithRegistry(t, { config: { ...auth, tag: 'latest', @@ -989,6 +816,9 @@ t.test('manifest', async t => { }, }, }) + + assertPublishNock(t, registry, 'npm', { noPut: true }) + await npm.exec('publish', []) const okKeys = [ @@ -1015,208 +845,113 @@ t.test('manifest', async t => { t.matchSnapshot(manifest, 'manifest') }) -t.test('aborts when prerelease and no tag', async t => { - const { npm } = await loadMockNpm(t, { - config: { - loglevel: 'silent', - [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', - }, - prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - version: '1.0.0-0', - publishConfig: { registry: alternateRegistry }, - }, null, 2), - }, +t.test('prerelease dist tag', (t) => { + t.test('aborts when prerelease and no tag', async t => { + const { npm } = await loadNpmWithRegistry(t, { + config: { + loglevel: 'silent', + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + ...pkgJson, + version: '1.0.0-0', + publishConfig: { registry: alternateRegistry }, + }, null, 2), + }, + }) + await t.rejects(async () => { + await npm.exec('publish', []) + }, new Error('You must specify a tag using --tag when publishing a prerelease version')) }) - await t.rejects(async () => { - await npm.exec('publish', []) - }, new Error('You must specify a tag using --tag when publishing a prerelease version')) -}) - -t.test('does not abort when prerelease and authored tag latest', async t => { - const prereleasePkg = { - ...pkgJson, - version: '1.0.0-0', - } - const { npm } = await loadMockNpm(t, { - config: { - loglevel: 'silent', - tag: 'latest', - [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', - }, - prefixDir: { - 'package.json': JSON.stringify({ - ...prereleasePkg, - publishConfig: { registry: alternateRegistry }, - }, null, 2), - }, - }) - const registry = new MockRegistry({ - tap: t, - registry: alternateRegistry, - authorization: 'test-other-token', - }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, { - _id: pkg, - name: pkg, - 'dist-tags': { latest: prereleasePkg.version }, - access: null, - versions: { - [prereleasePkg.version]: { - name: pkg, - version: prereleasePkg.version, - _id: `${pkg}@${prereleasePkg.version}`, - dist: { - shasum: /\.*/, - // eslint-disable-next-line max-len - tarball: `http:${alternateRegistry.slice(6)}/test-package/-/test-package-${prereleasePkg.version}.tgz`, - }, - publishConfig: { - registry: alternateRegistry, - }, - }, + t.test('does not abort when prerelease and authored tag latest', async t => { + const packageJson = { + ...pkgJson, + version: '1.0.0-0', + publishConfig: { registry: alternateRegistry }, + } + const { npm, registry } = await loadNpmWithRegistry(t, { + config: { + loglevel: 'silent', + tag: 'latest', + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', }, - _attachments: { - [`${pkg}-${prereleasePkg.version}.tgz`]: {}, + prefixDir: { + 'package.json': JSON.stringify(packageJson, null, 2), }, + registry: alternateRegistry, + authorization: 'test-other-token', }) - }).reply(200, {}) - await npm.exec('publish', []) -}) - -t.test('PREVENTS publish when latest dist-tag is HIGHER than publishing version', async t => { - const version = '50.0.0' - - const { npm } = await loadMockNpm(t, { - config: { - loglevel: 'silent', - [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', - }, - prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - version, - scripts: { - prepublishOnly: 'touch scripts-prepublishonly', - prepublish: 'touch scripts-prepublish', // should NOT run this one - publish: 'touch scripts-publish', - postpublish: 'touch scripts-postpublish', - }, - publishConfig: { registry: alternateRegistry }, - }, null, 2), - }, - mocks: { - ...mockNpmRegistryFetch({ - [`/${pkg}`]: { versions: { '50.0.0': {}, '99.0.0': {}, '100.0.0': {}, '101.0.0-pre': {} } }, - }).mocks, - }, - }) - await t.rejects(async () => { + assertPublishNock(t, registry, pkg, { packageJson }) await npm.exec('publish', []) - }, new Error('Cannot publish a lower version without an explicit dist tag.')) -}) + }) -t.test('ALLOWS publish when latest versions are LOWER than publishing version', async t => { - const version = '100.0.0' + t.end() +}) - const { npm } = await loadMockNpm(t, { +t.test('latest dist tag', (t) => { + const init = (version) => ({ config: { loglevel: 'silent', - [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + ...auth, }, prefixDir: { 'package.json': JSON.stringify({ ...pkgJson, version, - publishConfig: { registry: alternateRegistry }, }, null, 2), }, - mocks: { - ...mockNpmRegistryFetch({ - [`/${pkg}`]: { versions: { '50.0.0': {}, '99.0.0': {}, '101.0.0-pre': {} } }, - }).mocks, - }, - }) - const registry = new MockRegistry({ - tap: t, - registry: alternateRegistry, - authorization: 'test-other-token', + authorization: token, }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, putPackagePayload({ - pkg, alternateRegistry, version, - })) - }).reply(200, {}) - await npm.exec('publish', []) -}) -t.test('ALLOWS publish when not published yet', async t => { - const version = '100.0.0' + const packuments = [ + { version: '100.0.0' }, + { version: '105.0.0-pre' }, + ] - const { npm } = await loadMockNpm(t, { - config: { - loglevel: 'silent', - [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', - }, - prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - version, - publishConfig: { registry: alternateRegistry }, - }, null, 2), - }, - mocks: { - ...mockNpmRegistryFetch({ - [`/${pkg}`]: { }, - }).mocks, - }, - }) - const registry = new MockRegistry({ - tap: t, - registry: alternateRegistry, - authorization: 'test-other-token', + t.test('PREVENTS publish when latest version is HIGHER than publishing version', async t => { + const version = '99.0.0' + const { npm, registry } = await loadNpmWithRegistry(t, init(version)) + const manifest = registry.manifest({ name: pkg, packuments }) + assertPublishNock(t, registry, pkg, { noPut: true, manifest }) + await t.rejects(async () => { + await npm.exec('publish', []) + }, new Error('Cannot publish a lower version without an explicit dist tag.')) + }) + + t.test('ALLOWS publish when latest is HIGHER than publishing version and flag', async t => { + const version = '99.0.0' + const { npm, registry } = await loadNpmWithRegistry(t, { + ...init(version), + argv: ['--tag', 'latest'], + }) + const manifest = registry.manifest({ name: pkg, packuments }) + assertPublishNock(t, registry, pkg, { manifest }) + await npm.exec('publish', []) }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, putPackagePayload({ - pkg, alternateRegistry, version, - })) - }).reply(200, {}) - await npm.exec('publish', []) -}) -t.test('ALLOWS publish when not published yet (no versions)', async t => { - const version = '100.0.0' + t.test('ALLOWS publish when latest versions are LOWER than publishing version', async t => { + const version = '101.0.0' + const { npm, registry } = await loadNpmWithRegistry(t, init(version)) + const manifest = registry.manifest({ name: pkg, packuments }) + assertPublishNock(t, registry, pkg, { manifest }) + await npm.exec('publish', []) + }) - const { npm } = await loadMockNpm(t, { - config: { - loglevel: 'silent', - [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', - }, - prefixDir: { - 'package.json': JSON.stringify({ - ...pkgJson, - version, - publishConfig: { registry: alternateRegistry }, - }, null, 2), - }, - mocks: { - ...mockNpmRegistryFetch({ - [`/${pkg}`]: { versions: {} }, - }).mocks, - }, + t.test('ALLOWS publish when packument has empty versions (for coverage)', async t => { + const version = '1.0.0' + const { npm, registry } = await loadNpmWithRegistry(t, init(version)) + assertPublishNock(t, registry, pkg, { manifest: { versions: { } } }) + await npm.exec('publish', []) }) - const registry = new MockRegistry({ - tap: t, - registry: alternateRegistry, - authorization: 'test-other-token', + + t.test('ALLOWS publish when packument has empty manifest (for coverage)', async t => { + const version = '1.0.0' + const { npm, registry } = await loadNpmWithRegistry(t, init(version)) + assertPublishNock(t, registry, pkg, { manifest: { } }) + await npm.exec('publish', []) }) - registry.nock.put(`/${pkg}`, body => { - return t.match(body, putPackagePayload({ - pkg, alternateRegistry, version, - })) - }).reply(200, {}) - await npm.exec('publish', []) + + t.end() }) From d9271ade4e41e3ebf8a0d13703239a4b4f15bd66 Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 15:26:08 -0500 Subject: [PATCH 13/20] fix: assertPublishNock --- test/lib/commands/publish.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index a3e1b66be5273..412f51f8def78 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -19,7 +19,7 @@ const pkgJson = { version: '1.0.0', } -const packageNock = (t, registry, name, { +const assertPublishNock = (t, registry, name, { packageJson, access, noPut, putStatus, manifest, } = {}) => { const spec = npa(name) @@ -37,8 +37,6 @@ const packageNock = (t, registry, name, { return registry.nock } -const assertPublishNock = packageNock - t.cleanSnapshot = data => cleanZlib(data) t.test('respects publishConfig.registry, runs appropriate scripts', async t => { From 777184dcebc25cad94200746f1d81ce577484413 Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 15:28:39 -0500 Subject: [PATCH 14/20] fix: cover sort --- test/lib/commands/publish.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 412f51f8def78..942e01672984a 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -904,6 +904,8 @@ t.test('latest dist tag', (t) => { }) const packuments = [ + // this needs more than one item in it to cover the sort logic + { version: '50.0.0' }, { version: '100.0.0' }, { version: '105.0.0-pre' }, ] From 5149b5941d48a102e8dff30aa720f70066b4c7ce Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 16:27:43 -0500 Subject: [PATCH 15/20] fix: smoke --- smoke-tests/test/npm-replace-global.js | 1 + 1 file changed, 1 insertion(+) diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index 563aba4d39170..b567ddcab2483 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -137,6 +137,7 @@ t.test('publish and replace global self', async t => { await npmPackage() } registry.nock.get('/npm').reply(404, 'not found') + registry.nock.get('/npm').reply(404, 'not found') registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] From 0f4be49b0318b97e0e03585dabe4d3d7b60ac2a4 Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 22:53:38 -0500 Subject: [PATCH 16/20] fix: adds registry.publish --- mock-registry/lib/index.js | 61 +++++++++++++++++++- test/fixtures/mock-npm.js | 67 ---------------------- test/lib/commands/publish.js | 106 ++++++++++++++--------------------- 3 files changed, 101 insertions(+), 133 deletions(-) diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 8fdb46902a373..3b06681b7ed31 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -351,13 +351,30 @@ class MockRegistry { } // full unpublish of an entire package - async unpublish ({ manifest }) { + unpublish ({ manifest }) { let nock = this.nock const spec = npa(manifest.name) nock = nock.delete(this.fullPath(`/${spec.escapedName}/-rev/${manifest._rev}`)).reply(201) return nock } + publish (name, { + packageJson, access, noPut, putCode, manifest, packuments, + } = {}) { + // this getPackage call is used to get the latest semver version before publish + if (manifest) { + this.getPackage(name, { code: 200, resp: manifest }) + } else if (packuments) { + this.getPackage(name, { code: 200, resp: this.manifest({ name, packuments }) }) + } else { + // assumes the package does not exist yet and will 404 x2 from pacote.manifest + this.getPackage(name, { times: 2, code: 404 }) + } + if (!noPut) { + this.putPackage(name, { code: putCode, packageJson, access }) + } + } + getPackage (name, { times = 1, code = 200, query, resp = {} }) { let nock = this.nock nock = nock.get(`/${npa(name).escapedName}`).times(times) @@ -372,6 +389,48 @@ class MockRegistry { this.nock = nock } + putPackage (name, { code = 200, resp = {}, ...putPackagePayload }) { + this.nock.put(`/${npa(name).escapedName}`, body => { + return this.#tap.match(body, this.putPackagePayload({ name, ...putPackagePayload })) + }).reply(code, resp) + } + + putPackagePayload (opts) { + const pkg = opts.packageJson + const name = opts.name || pkg?.name + const registry = opts.registry || pkg?.publishConfig?.registry || 'https://registry.npmjs.org' + const access = opts.access || null + + const nameProperties = !name ? {} : { + _id: name, + name: name, + } + + const packageProperties = !pkg ? {} : { + 'dist-tags': { latest: pkg.version }, + versions: { + [pkg.version]: { + _id: `${pkg.name}@${pkg.version}`, + dist: { + shasum: /\.*/, + tarball: + `http://${new URL(registry).host}/${pkg.name}/-/${pkg.name}-${pkg.version}.tgz`, + }, + ...pkg, + }, + }, + _attachments: { + [`${pkg.name}-${pkg.version}.tgz`]: {}, + }, + } + + return { + access, + ...nameProperties, + ...packageProperties, + } + } + getTokens (tokens) { return this.nock.get('/-/npm/v1/tokens') .reply(200, { diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index e385f9053335d..bfeb9a05615a2 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -2,7 +2,6 @@ const os = require('node:os') const fs = require('node:fs').promises const fsSync = require('node:fs') const path = require('node:path') -const npmFetch = require('npm-registry-fetch') const tap = require('tap') const mockLogs = require('./mock-logs.js') const mockGlobals = require('@npmcli/mock-globals') @@ -464,74 +463,8 @@ function workspaceMock (t, opts) { } } -const mockNpmRegistryFetch = (tags) => { - const fetchOpts = {} - const getRequest = async (url, opts) => { - if (fetchOpts[url]) { - fetchOpts[url].push(opts) - } else { - fetchOpts[url] = [opts] - } - const find = ({ ...tags })[url] - if (!find) { - throw new Error(`no npm-registry-fetch mock for ${url}`) - } - if (typeof find === 'function') { - return find() - } - return find - } - const nrf = async (url, opts) => { - return { - json: () => getRequest(url, opts), - } - } - const mock = Object.assign(nrf, npmFetch, { json: getRequest }) - const mocks = { 'npm-registry-fetch': mock } - const getOpts = (url) => fetchOpts[url] - return { mocks, mock, fetchOpts, getOpts } -} - -const putPackagePayload = (opts) => { - const package = opts.packageJson - const name = opts.name || package?.name - const registry = opts.registry || package?.publishConfig?.registry || 'https://registry.npmjs.org' - const access = opts.access || null - - const nameProperties = !name ? {} : { - _id: name, - name: name, - } - - const packageProperties = !package ? {} : { - 'dist-tags': { latest: package.version }, - versions: { - [package.version]: { - _id: `${package.name}@${package.version}`, - dist: { - shasum: /\.*/, - tarball: - `http://${new URL(registry).host}/${package.name}/-/${package.name}-${package.version}.tgz`, - }, - ...package, - }, - }, - _attachments: { - [`${package.name}-${package.version}.tgz`]: {}, - }, - } - - return { - access, - ...nameProperties, - ...packageProperties, - } -} - module.exports = setupMockNpm module.exports.load = setupMockNpm module.exports.setGlobalNodeModules = setGlobalNodeModules module.exports.loadNpmWithRegistry = loadNpmWithRegistry module.exports.workspaceMock = workspaceMock -module.exports.mockNpmRegistryFetch = mockNpmRegistryFetch -module.exports.putPackagePayload = putPackagePayload diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 942e01672984a..3fc535456e18d 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -1,11 +1,10 @@ const t = require('tap') -const { loadNpmWithRegistry, putPackagePayload } = require('../../fixtures/mock-npm') +const { loadNpmWithRegistry } = require('../../fixtures/mock-npm') const { cleanZlib } = require('../../fixtures/clean-snapshot') const pacote = require('pacote') const Arborist = require('@npmcli/arborist') const path = require('node:path') const fs = require('node:fs') -const npa = require('npm-package-arg') const pkg = 'test-package' const token = 'test-auth-token' @@ -19,24 +18,6 @@ const pkgJson = { version: '1.0.0', } -const assertPublishNock = (t, registry, name, { - packageJson, access, noPut, putStatus, manifest, -} = {}) => { - const spec = npa(name) - if (!manifest) { - registry.nock.get(`/${spec.escapedName}`).reply(404, 'not found') - registry.nock.get(`/${spec.escapedName}`).reply(404, 'not found') - } else { - registry.nock.get(`/${spec.escapedName}`).reply(200, manifest) - } - if (!noPut) { - registry.nock.put(`/${spec.escapedName}`, body => { - return t.match(body, putPackagePayload({ name, packageJson, access })) - }).reply(putStatus ?? 200, {}) - } - return registry.nock -} - t.cleanSnapshot = data => cleanZlib(data) t.test('respects publishConfig.registry, runs appropriate scripts', async t => { @@ -61,7 +42,7 @@ t.test('respects publishConfig.registry, runs appropriate scripts', async t => { registry: alternateRegistry, authorization: 'test-other-token', }) - assertPublishNock(t, registry, pkg, { packageJson }) + registry.publish(pkg, { packageJson }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.equal(fs.existsSync(path.join(prefix, 'scripts-prepublishonly')), true, 'ran prepublishOnly') @@ -94,7 +75,7 @@ t.test('re-loads publishConfig.registry if added during script process', async t registry: alternateRegistry, authorization: 'test-other-token', }) - assertPublishNock(t, registry, pkg, { packageJson }) + registry.publish(pkg, { packageJson }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -124,7 +105,7 @@ t.test('prioritize CLI flags over publishConfig', async t => { registryUrl: alternateRegistry, authorization: 'test-other-token', }) - assertPublishNock(t, registry, pkg, { packageJson }) + registry.publish(pkg, { packageJson }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -140,7 +121,7 @@ t.test('json', async t => { }, authorization: token, }) - assertPublishNock(t, registry, pkg) + registry.publish(pkg) await npm.exec('publish', []) t.matchSnapshot(logs.notice) t.matchSnapshot(joinedOutput(), 'new package json') @@ -157,7 +138,7 @@ t.test('dry-run', async t => { }, authorization: token, }) - assertPublishNock(t, registry, pkg, { noPut: true }) + registry.publish(pkg, { noPut: true }) await npm.exec('publish', []) t.equal(joinedOutput(), `+ ${pkg}@1.0.0`) t.matchSnapshot(logs.notice) @@ -181,7 +162,7 @@ t.test('foreground-scripts defaults to true', async t => { ), }, }) - assertPublishNock(t, registry, 'test-fg-scripts', { noPut: true }) + registry.publish('test-fg-scripts', { noPut: true }) await npm.exec('publish', []) t.matchSnapshot(logs.notice) t.strictSame( @@ -214,7 +195,7 @@ t.test('foreground-scripts can still be set to false', async t => { }, }) - assertPublishNock(t, registry, 'test-fg-scripts', { noPut: true }) + registry.publish('test-fg-scripts', { noPut: true }) await npm.exec('publish', []) t.matchSnapshot(logs.notice) @@ -254,7 +235,7 @@ t.test('throws when invalid tag when not url encodable', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - assertPublishNock(t, registry, pkg, { noPut: true }) + registry.publish(pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), @@ -284,7 +265,7 @@ t.test('tarball', async t => { const tarball = await pacote.tarball(home, { Arborist }) const tarFilename = path.join(home, 'tarball.tgz') fs.writeFileSync(tarFilename, tarball) - assertPublishNock(t, registry, 'test-tar-package') + registry.publish('test-tar-package') await npm.exec('publish', [tarFilename]) t.matchSnapshot(logs.notice) t.matchSnapshot(joinedOutput(), 'new package json') @@ -296,7 +277,7 @@ t.test('no auth default registry', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - assertPublishNock(t, registry, pkg, { noPut: true }) + registry.publish(pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -315,7 +296,7 @@ t.test('no auth dry-run', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - assertPublishNock(t, registry, pkg, { noPut: true }) + registry.publish(pkg, { noPut: true }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput()) t.matchSnapshot(logs.warn, 'warns about auth being needed') @@ -331,7 +312,7 @@ t.test('no auth for configured registry', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - assertPublishNock(t, registry, pkg, { noPut: true }) + registry.publish(pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -355,7 +336,7 @@ t.test('no auth for scope configured registry', async t => { }, null, 2), }, }) - assertPublishNock(t, registry, '@npm/test-package', { noPut: true }) + registry.publish('@npm/test-package', { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -381,7 +362,7 @@ t.test('has token auth for scope configured registry', async t => { registry: alternateRegistry, authorization: 'test-scope-token', }) - assertPublishNock(t, registry, '@npm/test-package') + registry.publish('@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -402,7 +383,7 @@ t.test('has mTLS auth for scope configured registry', async t => { }, registry: alternateRegistry, }) - assertPublishNock(t, registry, '@npm/test-package') + registry.publish('@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -454,9 +435,9 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - assertPublishNock(t, registry, 'workspace-p', { noPut: true }) + registry.publish('workspace-p', { noPut: true }) ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { - assertPublishNock(t, registry, name) + registry.publish(name) }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'all public workspaces') @@ -474,9 +455,9 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - assertPublishNock(t, registry, 'workspace-p', { noPut: true }) + registry.publish('workspace-p', { noPut: true }) ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { - assertPublishNock(t, registry, name) + registry.publish(name) }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'all public workspaces') @@ -494,7 +475,7 @@ t.test('workspaces', t => { authorization: token, }) ;['workspace-a'].forEach(name => { - assertPublishNock(t, registry, name) + registry.publish(name) }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'single workspace') @@ -510,9 +491,7 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - - assertPublishNock(t, registry, 'workspace-a', { putStatus: 404 }) - + registry.publish('workspace-a', { putCode: 404 }) await t.rejects(npm.exec('publish', []), { code: 'E404' }) }) @@ -547,8 +526,8 @@ t.test('workspaces', t => { prefixDir: testDir, authorization: token, }) - assertPublishNock(t, registry, '@scoped/workspace-p', { noPut: true }) - assertPublishNock(t, registry, 'workspace-a') + registry.publish('@scoped/workspace-p', { noPut: true }) + registry.publish('workspace-a') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'one marked private') }) @@ -579,9 +558,9 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - assertPublishNock(t, registry, 'workspace-p', { noPut: true }) + registry.publish('workspace-p', { noPut: true }) ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { - assertPublishNock(t, registry, name) + registry.publish(name) }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'all workspaces in json') @@ -617,7 +596,7 @@ t.test('workspaces', t => { chdir: ({ prefix }) => path.resolve(prefix, './workspace-a'), authorization: token, }) - assertPublishNock(t, registry, 'pkg') + registry.publish('pkg') await npm.exec('publish', ['../dir/pkg']) t.matchSnapshot(joinedOutput(), 'publish different package spec') }) @@ -644,7 +623,7 @@ t.test('ignore-scripts', async t => { }, authorization: token, }) - assertPublishNock(t, registry, pkg) + registry.publish(pkg) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.equal( @@ -679,7 +658,7 @@ t.test('_auth config default registry', async t => { }, basic, }) - assertPublishNock(t, registry, pkg) + registry.publish(pkg) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -699,7 +678,7 @@ t.test('bare _auth and registry config', async t => { registry: alternateRegistry, basic, }) - assertPublishNock(t, registry, '@npm/test-package') + registry.publish('@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -718,7 +697,7 @@ t.test('bare _auth config scoped registry', async t => { }, null, 2), }, }) - assertPublishNock(t, registry, '@npm/test-package', { noPut: true }) + registry.publish('@npm/test-package', { noPut: true }) await t.rejects( npm.exec('publish', []), { message: `This command requires you to be logged in to ${alternateRegistry}` } @@ -741,7 +720,7 @@ t.test('scoped _auth config scoped registry', async t => { registry: alternateRegistry, basic, }) - assertPublishNock(t, registry, '@npm/test-package') + registry.publish('@npm/test-package') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') }) @@ -761,7 +740,7 @@ t.test('restricted access', async t => { }, authorization: token, }) - assertPublishNock(t, registry, '@npm/test-package', { packageJson, access: 'restricted' }) + registry.publish('@npm/test-package', { packageJson, access: 'restricted' }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.matchSnapshot(logs.notice) @@ -781,7 +760,7 @@ t.test('public access', async t => { }, authorization: token, }) - assertPublishNock(t, registry, '@npm/test-package', { access: 'public' }) + registry.publish('@npm/test-package', { access: 'public' }) await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'new package version') t.matchSnapshot(logs.notice) @@ -815,7 +794,7 @@ t.test('manifest', async t => { }, }) - assertPublishNock(t, registry, 'npm', { noPut: true }) + registry.publish('npm', { noPut: true }) await npm.exec('publish', []) @@ -881,7 +860,7 @@ t.test('prerelease dist tag', (t) => { registry: alternateRegistry, authorization: 'test-other-token', }) - assertPublishNock(t, registry, pkg, { packageJson }) + registry.publish(pkg, { packageJson }) await npm.exec('publish', []) }) @@ -913,8 +892,7 @@ t.test('latest dist tag', (t) => { t.test('PREVENTS publish when latest version is HIGHER than publishing version', async t => { const version = '99.0.0' const { npm, registry } = await loadNpmWithRegistry(t, init(version)) - const manifest = registry.manifest({ name: pkg, packuments }) - assertPublishNock(t, registry, pkg, { noPut: true, manifest }) + registry.publish(pkg, { noPut: true, packuments }) await t.rejects(async () => { await npm.exec('publish', []) }, new Error('Cannot publish a lower version without an explicit dist tag.')) @@ -926,30 +904,28 @@ t.test('latest dist tag', (t) => { ...init(version), argv: ['--tag', 'latest'], }) - const manifest = registry.manifest({ name: pkg, packuments }) - assertPublishNock(t, registry, pkg, { manifest }) + registry.publish(pkg, { packuments }) await npm.exec('publish', []) }) t.test('ALLOWS publish when latest versions are LOWER than publishing version', async t => { const version = '101.0.0' const { npm, registry } = await loadNpmWithRegistry(t, init(version)) - const manifest = registry.manifest({ name: pkg, packuments }) - assertPublishNock(t, registry, pkg, { manifest }) + registry.publish(pkg, { packuments }) await npm.exec('publish', []) }) t.test('ALLOWS publish when packument has empty versions (for coverage)', async t => { const version = '1.0.0' const { npm, registry } = await loadNpmWithRegistry(t, init(version)) - assertPublishNock(t, registry, pkg, { manifest: { versions: { } } }) + registry.publish(pkg, { manifest: { versions: { } } }) await npm.exec('publish', []) }) t.test('ALLOWS publish when packument has empty manifest (for coverage)', async t => { const version = '1.0.0' const { npm, registry } = await loadNpmWithRegistry(t, init(version)) - assertPublishNock(t, registry, pkg, { manifest: { } }) + registry.publish(pkg, { manifest: {} }) await npm.exec('publish', []) }) From 03ec495268765ddb4f57dead9a04391b7acc0769 Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 22:55:52 -0500 Subject: [PATCH 17/20] fix: tag termanology --- lib/commands/publish.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index c02535f089e3c..a77daa8112262 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -133,10 +133,10 @@ class Publish extends BaseCommand { const registry = npmFetch.pickRegistry(resolved, opts) const latestVersion = await this.#latestPublishedVersion(resolved, registry) - const latestTagIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) + const latestSemverIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) - if (latestTagIsGreater && isDefaultTag) { - throw new Error('Cannot publish a lower version without an explicit dist tag.') + if (latestSemverIsGreater && isDefaultTag) { + throw new Error('Cannot publish a lower version without an explicit tag.') } // make sure tag is valid, this will throw if invalid From e30b8df3b79b162e6cadc89c06927c11f93411f1 Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 23:05:06 -0500 Subject: [PATCH 18/20] fix: moved throw after auth check --- lib/commands/publish.js | 16 ++++++++-------- test/lib/commands/publish.js | 22 ++++++---------------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index a77daa8112262..5bec5f85ec3e0 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -130,18 +130,11 @@ class Publish extends BaseCommand { } const resolved = npa.resolve(manifest.name, manifest.version) - const registry = npmFetch.pickRegistry(resolved, opts) - - const latestVersion = await this.#latestPublishedVersion(resolved, registry) - const latestSemverIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) - - if (latestSemverIsGreater && isDefaultTag) { - throw new Error('Cannot publish a lower version without an explicit tag.') - } // make sure tag is valid, this will throw if invalid npa(`${manifest.name}@${defaultTag}`) + const registry = npmFetch.pickRegistry(resolved, opts) const creds = this.npm.config.getCredentialsByURI(registry) const noCreds = !(creds.token || creds.username || creds.certfile && creds.keyfile) const outputRegistry = replaceInfo(registry) @@ -164,6 +157,13 @@ class Publish extends BaseCommand { } } + const latestVersion = await this.#latestPublishedVersion(resolved, registry) + const latestSemverIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) + + if (latestSemverIsGreater && isDefaultTag) { + throw new Error('Cannot publish a lower version without an explicit tag.') + } + const access = opts.access === null ? 'default' : opts.access let msg = `Publishing to ${outputRegistry} with tag ${defaultTag} and ${access} access` if (dryRun) { diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index 3fc535456e18d..aa5a556251f57 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -227,7 +227,7 @@ t.test('throws when invalid tag is semver', async t => { }) t.test('throws when invalid tag when not url encodable', async t => { - const { npm, registry } = await loadNpmWithRegistry(t, { + const { npm } = await loadNpmWithRegistry(t, { config: { tag: '@test', }, @@ -235,8 +235,6 @@ t.test('throws when invalid tag when not url encodable', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - registry.publish(pkg, { noPut: true }) - await t.rejects( npm.exec('publish', []), { @@ -272,12 +270,11 @@ t.test('tarball', async t => { }) t.test('no auth default registry', async t => { - const { npm, registry } = await loadNpmWithRegistry(t, { + const { npm } = await loadNpmWithRegistry(t, { prefixDir: { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - registry.publish(pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -303,7 +300,7 @@ t.test('no auth dry-run', async t => { }) t.test('no auth for configured registry', async t => { - const { npm, registry } = await loadNpmWithRegistry(t, { + const { npm } = await loadNpmWithRegistry(t, { config: { registry: alternateRegistry, ...auth, @@ -312,7 +309,6 @@ t.test('no auth for configured registry', async t => { 'package.json': JSON.stringify(pkgJson, null, 2), }, }) - registry.publish(pkg, { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -323,7 +319,7 @@ t.test('no auth for configured registry', async t => { }) t.test('no auth for scope configured registry', async t => { - const { npm, registry } = await loadNpmWithRegistry(t, { + const { npm } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -336,7 +332,6 @@ t.test('no auth for scope configured registry', async t => { }, null, 2), }, }) - registry.publish('@npm/test-package', { noPut: true }) await t.rejects( npm.exec('publish', []), { @@ -435,7 +430,6 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - registry.publish('workspace-p', { noPut: true }) ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { registry.publish(name) }) @@ -455,7 +449,6 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - registry.publish('workspace-p', { noPut: true }) ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { registry.publish(name) }) @@ -526,7 +519,6 @@ t.test('workspaces', t => { prefixDir: testDir, authorization: token, }) - registry.publish('@scoped/workspace-p', { noPut: true }) registry.publish('workspace-a') await npm.exec('publish', []) t.matchSnapshot(joinedOutput(), 'one marked private') @@ -558,7 +550,6 @@ t.test('workspaces', t => { prefixDir: dir, authorization: token, }) - registry.publish('workspace-p', { noPut: true }) ;['workspace-a', 'workspace-b', 'workspace-n'].forEach(name => { registry.publish(name) }) @@ -684,7 +675,7 @@ t.test('bare _auth and registry config', async t => { }) t.test('bare _auth config scoped registry', async t => { - const { npm, registry } = await loadNpmWithRegistry(t, { + const { npm } = await loadNpmWithRegistry(t, { config: { scope: '@npm', registry: alternateRegistry, @@ -697,7 +688,6 @@ t.test('bare _auth config scoped registry', async t => { }, null, 2), }, }) - registry.publish('@npm/test-package', { noPut: true }) await t.rejects( npm.exec('publish', []), { message: `This command requires you to be logged in to ${alternateRegistry}` } @@ -895,7 +885,7 @@ t.test('latest dist tag', (t) => { registry.publish(pkg, { noPut: true, packuments }) await t.rejects(async () => { await npm.exec('publish', []) - }, new Error('Cannot publish a lower version without an explicit dist tag.')) + }, new Error('Cannot publish a lower version without an explicit tag.')) }) t.test('ALLOWS publish when latest is HIGHER than publishing version and flag', async t => { From 80c87f1bed0c3215168fd267864dc8d1a6481e8d Mon Sep 17 00:00:00 2001 From: reggi Date: Wed, 4 Dec 2024 23:12:37 -0500 Subject: [PATCH 19/20] fix: make npm smoke test not strict nock --- smoke-tests/test/fixtures/setup.js | 6 ++++-- smoke-tests/test/npm-replace-global.js | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/smoke-tests/test/fixtures/setup.js b/smoke-tests/test/fixtures/setup.js index 18492d0d52f8f..2d0c54c984243 100644 --- a/smoke-tests/test/fixtures/setup.js +++ b/smoke-tests/test/fixtures/setup.js @@ -73,7 +73,9 @@ const getCleanPaths = async () => { }) } -module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy = false } = {}) => { +module.exports = async (t, { + testdir = {}, debug, mockRegistry = true, strictRegistryNock = true, useProxy = false, +} = {}) => { const debugLog = debug || CI ? (...a) => t.comment(...a) : () => {} debugLog({ SMOKE_PUBLISH_TARBALL, CI }) @@ -103,7 +105,7 @@ module.exports = async (t, { testdir = {}, debug, mockRegistry = true, useProxy tap: t, registry: MOCK_REGISTRY, debug, - strict: true, + strict: strictRegistryNock, }) const proxyEnv = {} diff --git a/smoke-tests/test/npm-replace-global.js b/smoke-tests/test/npm-replace-global.js index b567ddcab2483..c53beb10875af 100644 --- a/smoke-tests/test/npm-replace-global.js +++ b/smoke-tests/test/npm-replace-global.js @@ -103,6 +103,7 @@ t.test('publish and replace global self', async t => { getPaths, paths: { globalBin, globalNodeModules, cache }, } = await setupNpmGlobal(t, { + strictRegistryNock: false, testdir: { home: { '.npmrc': `//${setup.MOCK_REGISTRY.host}/:_authToken = test-token`, @@ -136,8 +137,6 @@ t.test('publish and replace global self', async t => { if (setup.SMOKE_PUBLISH) { await npmPackage() } - registry.nock.get('/npm').reply(404, 'not found') - registry.nock.get('/npm').reply(404, 'not found') registry.nock.put('/npm', body => { if (body._id === 'npm' && body.versions[version]) { publishedPackument = body.versions[version] From ceb143ff69d9e9571b1b5d8a80094851eb626a41 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 6 Dec 2024 11:26:46 -0500 Subject: [PATCH 20/20] fix: error message updates --- lib/commands/publish.js | 5 +++-- test/lib/commands/publish.js | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 5bec5f85ec3e0..c59588fefb241 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -120,7 +120,7 @@ class Publish extends BaseCommand { const isDefaultTag = this.npm.config.isDefault('tag') if (isPreRelease && isDefaultTag) { - throw new Error('You must specify a tag using --tag when publishing a prerelease version') + throw new Error('You must specify a tag using --tag when publishing a prerelease version.') } // If we are not in JSON mode then we show the user the contents of the tarball @@ -161,7 +161,8 @@ class Publish extends BaseCommand { const latestSemverIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version) if (latestSemverIsGreater && isDefaultTag) { - throw new Error('Cannot publish a lower version without an explicit tag.') + /* eslint-disable-next-line max-len */ + throw new Error(`Cannot implicitly apply the "latest" tag because published version ${latestVersion} is higher than the new version ${manifest.version}. You must specify a tag using --tag.`) } const access = opts.access === null ? 'default' : opts.access diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index aa5a556251f57..10dc9b33deda4 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -885,7 +885,8 @@ t.test('latest dist tag', (t) => { registry.publish(pkg, { noPut: true, packuments }) await t.rejects(async () => { await npm.exec('publish', []) - }, new Error('Cannot publish a lower version without an explicit tag.')) + /* eslint-disable-next-line max-len */ + }, new Error('Cannot implicitly apply the "latest" tag because published version 100.0.0 is higher than the new version 99.0.0. You must specify a tag using --tag.')) }) t.test('ALLOWS publish when latest is HIGHER than publishing version and flag', async t => {