Skip to content

Commit

Permalink
feat!: no implicit latest tag on publish when latest > version (#7939)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Upon publishing, in order to apply a default "latest"
dist tag, the command now retrieves all prior versions of the package.
It will require that the version you're trying to publish is above the
latest semver version in the registry, not including pre-release tags.

Implements [npm
RFC7](https://github.com/npm/rfcs/blob/main/accepted/0007-publish-without-tag.md).

Related to prerelease dist-tag: #7910
A part of npm 11 roadmap: npm/statusboard#898

---------

Co-authored-by: Jordan Harband <ljharb@gmail.com>
  • Loading branch information
reggi and ljharb authored Dec 6, 2024
1 parent bc9b14d commit f3ac7b7
Show file tree
Hide file tree
Showing 16 changed files with 346 additions and 387 deletions.
6 changes: 3 additions & 3 deletions lib/commands/deprecate.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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 },
Expand All @@ -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',
Expand Down
8 changes: 4 additions & 4 deletions lib/commands/dist-tag.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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}`)
}

Expand All @@ -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}`)
}

Expand Down Expand Up @@ -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 }
)
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/doctor.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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'
Expand Down
32 changes: 31 additions & 1 deletion lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -157,6 +157,14 @@ class Publish extends BaseCommand {
}
}

const latestVersion = await this.#latestPublishedVersion(resolved, registry)
const latestSemverIsGreater = !!latestVersion && semver.gte(latestVersion, manifest.version)

if (latestSemverIsGreater && isDefaultTag) {
/* 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
let msg = `Publishing to ${outputRegistry} with tag ${defaultTag} and ${access} access`
if (dryRun) {
Expand Down Expand Up @@ -196,6 +204,28 @@ class Publish extends BaseCommand {
}
}

async #latestPublishedVersion (spec, registry) {
try {
const packument = await pacote.packument(spec, {
...this.npm.flatOptions,
preferOnline: true,
registry,
})
if (typeof packument?.versions === 'undefined') {
return null
}
const ordered = Object.keys(packument?.versions)
.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) {
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?
Expand Down
6 changes: 3 additions & 3 deletions lib/commands/star.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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 },
Expand All @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions lib/commands/stars.js
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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}"` },
})
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/ping.js
Original file line number Diff line number Diff line change
@@ -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(() => ({}))
}
6 changes: 3 additions & 3 deletions lib/utils/verify-signatures.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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) => ({
Expand Down Expand Up @@ -253,7 +253,7 @@ class VerifySignatures {
}

getSpecRegistry (spec) {
return fetch.pickRegistry(spec, this.npm.flatOptions)
return npmFetch.pickRegistry(spec, this.npm.flatOptions)
}

getValidPackageInfo (edge) {
Expand Down
61 changes: 60 additions & 1 deletion mock-registry/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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, {
Expand Down
6 changes: 4 additions & 2 deletions smoke-tests/test/fixtures/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })

Expand Down Expand Up @@ -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 = {}
Expand Down
1 change: 1 addition & 0 deletions smoke-tests/test/npm-replace-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down
20 changes: 17 additions & 3 deletions test/fixtures/mock-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,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`
Expand Down Expand Up @@ -352,7 +366,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 */
Expand Down
Loading

0 comments on commit f3ac7b7

Please sign in to comment.