From c929ed180b3d3361d1541737a4c3c9932cd480c6 Mon Sep 17 00:00:00 2001 From: roni-berlin <72336831+roni-berlin@users.noreply.github.com> Date: Tue, 9 Apr 2024 19:44:40 +0300 Subject: [PATCH] fix: prioritize CLI flags over publishConfig settings (#7321) This PR addresses an issue where CLI flags were not taking precedence over publishConfig settings. To ensure CLI flags have higher priority, properties from the publishConfig object that also exist in CLI flags are filtered out. Related to #6400 --- lib/commands/publish.js | 7 ++- lib/commands/unpublish.js | 7 ++- .../test/lib/commands/publish.js.test.cjs | 4 ++ test/lib/commands/publish.js | 52 +++++++++++++++++++ test/lib/commands/unpublish.js | 30 +++++++++++ 5 files changed, 98 insertions(+), 2 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 0456fd7e8320e..cf6b50cce3c21 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -220,7 +220,12 @@ class Publish extends BaseCommand { }) } if (manifest.publishConfig) { - flatten(manifest.publishConfig, opts) + const cliFlags = this.npm.config.data.get('cli').raw + // Filter out properties set in CLI flags to prioritize them over + // corresponding `publishConfig` settings + const filteredPublishConfig = Object.fromEntries( + Object.entries(manifest.publishConfig).filter(([key]) => !(key in cliFlags))) + flatten(filteredPublishConfig, opts) } return manifest } diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index a9c20900534c3..a4d445a035b62 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -141,7 +141,12 @@ class Unpublish extends BaseCommand { // If localPrefix has a package.json with a name that matches the package // being unpublished, load up the publishConfig if (manifest?.name === spec.name && manifest.publishConfig) { - flatten(manifest.publishConfig, opts) + const cliFlags = this.npm.config.data.get('cli').raw + // Filter out properties set in CLI flags to prioritize them over + // corresponding `publishConfig` settings + const filteredPublishConfig = Object.fromEntries( + Object.entries(manifest.publishConfig).filter(([key]) => !(key in cliFlags))) + flatten(filteredPublishConfig, opts) } const versions = await Unpublish.getKeysOfVersions(spec.name, opts) diff --git a/tap-snapshots/test/lib/commands/publish.js.test.cjs b/tap-snapshots/test/lib/commands/publish.js.test.cjs index 1bbdebb4fd617..eb2b021e5d8c8 100644 --- a/tap-snapshots/test/lib/commands/publish.js.test.cjs +++ b/tap-snapshots/test/lib/commands/publish.js.test.cjs @@ -452,6 +452,10 @@ exports[`test/lib/commands/publish.js TAP restricted access > new package versio + @npm/test-package@1.0.0 ` +exports[`test/lib/commands/publish.js TAP prioritize CLI flags over publishConfig > new package version 1`] = ` ++ test-package@1.0.0 +` + exports[`test/lib/commands/publish.js TAP scoped _auth config scoped registry > new package version 1`] = ` + @npm/test-package@1.0.0 ` diff --git a/test/lib/commands/publish.js b/test/lib/commands/publish.js index ec7299e9eec53..751cd97d8acf6 100644 --- a/test/lib/commands/publish.js +++ b/test/lib/commands/publish.js @@ -131,6 +131,58 @@ t.test('re-loads publishConfig.registry if added during script process', async t 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, { + config: { + [`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + ...pkgJson, + scripts: { + prepare: 'cp new.json package.json', + }, + }, null, 2), + 'new.json': JSON.stringify({ + ...pkgJson, + publishConfig, + }), + }, + argv: ['--registry', alternateRegistry], + }) + 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, + }, + }, + _attachments: { + [`${pkg}-1.0.0.tgz`]: {}, + }, + }) + }).reply(200, {}) + await npm.exec('publish', []) + t.matchSnapshot(joinedOutput(), 'new package version') +}) + t.test('json', async t => { const { joinedOutput, npm, logs } = await loadMockNpm(t, { config: { diff --git a/test/lib/commands/unpublish.js b/test/lib/commands/unpublish.js index 097309393a258..31dc77ea46cd0 100644 --- a/test/lib/commands/unpublish.js +++ b/test/lib/commands/unpublish.js @@ -408,6 +408,36 @@ t.test('publishConfig no spec', async t => { t.equal(joinedOutput(), '- test-package') }) +t.test('prioritize CLI flags over publishConfig no spec', async t => { + const alternateRegistry = 'https://other.registry.npmjs.org' + const publishConfig = { registry: 'http://publishconfig' } + const { joinedOutput, npm } = await loadMockNpm(t, { + config: { + force: true, + '//other.registry.npmjs.org/:_authToken': 'test-other-token', + }, + prefixDir: { + 'package.json': JSON.stringify({ + name: pkg, + version: '1.0.0', + publishConfig, + }, null, 2), + }, + argv: ['--registry', alternateRegistry], + }) + + const registry = new MockRegistry({ + tap: t, + registry: alternateRegistry, + authorization: 'test-other-token', + }) + const manifest = registry.manifest({ name: pkg }) + await registry.package({ manifest, query: { write: true }, times: 2 }) + registry.unpublish({ manifest }) + await npm.exec('unpublish', []) + t.equal(joinedOutput(), '- test-package') +}) + t.test('publishConfig with spec', async t => { const alternateRegistry = 'https://other.registry.npmjs.org' const { joinedOutput, npm } = await loadMockNpm(t, {