Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: no implicit latest tag on publish when latest > version #7939

Merged
merged 20 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
31 changes: 30 additions & 1 deletion lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,18 @@ 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)
reggi marked this conversation as resolved.
Show resolved Hide 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.')
wraithgar marked this conversation as resolved.
Show resolved Hide resolved
reggi marked this conversation as resolved.
Show resolved Hide resolved
}

// 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)
Expand Down Expand Up @@ -196,6 +203,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
2 changes: 2 additions & 0 deletions smoke-tests/test/npm-replace-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ 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')
reggi marked this conversation as resolved.
Show resolved Hide resolved
registry.nock.put('/npm', body => {
if (body._id === 'npm' && body.versions[version]) {
publishedPackument = body.versions[version]
Expand Down
87 changes: 84 additions & 3 deletions test/fixtures/mock-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -292,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`
Expand Down Expand Up @@ -352,7 +367,7 @@ const loadNpmWithRegistry = async (t, opts) => {
packageDirty,
}

return { registry, assert, ...mock }
return { assert }
}

/** breaks down a spec "[email protected]" into different parts for mocking */
Expand Down Expand Up @@ -449,8 +464,74 @@ 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
Loading
Loading