From d7d6c13717068e74a0a607236010ff098ac64fed Mon Sep 17 00:00:00 2001 From: roni25536 Date: Wed, 27 Mar 2024 19:18:09 +0000 Subject: [PATCH 1/6] fix: Prioritize CLI flags over publishConfig settings in publish command --- lib/commands/publish.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 63abc50b4745f..6ebf76737a212 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -220,7 +220,17 @@ class Publish extends BaseCommand { }) } if (manifest.publishConfig) { + const cliFlags = this.npm.config.data.get('cli').raw + if(cliFlags){ + // Filter out properties set in CLI flags to prioritize them over + // corresponding `publishConfig` settings + const filteredPublishConfig = Object.fromEntries( + Object.entries(manifest.publishConfig).filter(([key]) => !cliFlags.hasOwnProperty(key)) + ) + flatten(filteredPublishConfig, opts) + }else{ flatten(manifest.publishConfig, opts) + } } return manifest } From f53bc4cf0c5981de3ad0cda88c351c3d37e4f103 Mon Sep 17 00:00:00 2001 From: roni25536 Date: Wed, 27 Mar 2024 19:18:19 +0000 Subject: [PATCH 2/6] fix: Prioritize CLI flags over publishConfig settings in unpublish command --- lib/commands/unpublish.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index a9c20900534c3..b8d8ce2a1c342 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -141,7 +141,17 @@ 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) { + const cliFlags = this.npm.config.data.get('cli').raw + if(cliFlags){ + // Filter out properties set in CLI flags to prioritize them over + // corresponding `publishConfig` settings + const filteredPublishConfig = Object.fromEntries( + Object.entries(manifest.publishConfig).filter(([key]) => !cliFlags.hasOwnProperty(key)) + ) + flatten(filteredPublishConfig, opts) + }else{ flatten(manifest.publishConfig, opts) + } } const versions = await Unpublish.getKeysOfVersions(spec.name, opts) From aa08f38941726ff458094abe5c9dcb3f15f986b8 Mon Sep 17 00:00:00 2001 From: roni25536 Date: Wed, 27 Mar 2024 19:46:47 +0000 Subject: [PATCH 3/6] fix: lint --- lib/commands/publish.js | 17 ++++++++--------- lib/commands/unpublish.js | 17 ++++++++--------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 6ebf76737a212..6f62057c42197 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -221,15 +221,14 @@ class Publish extends BaseCommand { } if (manifest.publishConfig) { const cliFlags = this.npm.config.data.get('cli').raw - if(cliFlags){ - // Filter out properties set in CLI flags to prioritize them over - // corresponding `publishConfig` settings - const filteredPublishConfig = Object.fromEntries( - Object.entries(manifest.publishConfig).filter(([key]) => !cliFlags.hasOwnProperty(key)) - ) - flatten(filteredPublishConfig, opts) - }else{ - flatten(manifest.publishConfig, opts) + if (cliFlags) { + // 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) + } else { + flatten(manifest.publishConfig, opts) } } return manifest diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index b8d8ce2a1c342..ccf8374892af2 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -142,15 +142,14 @@ class Unpublish extends BaseCommand { // being unpublished, load up the publishConfig if (manifest?.name === spec.name && manifest.publishConfig) { const cliFlags = this.npm.config.data.get('cli').raw - if(cliFlags){ - // Filter out properties set in CLI flags to prioritize them over - // corresponding `publishConfig` settings - const filteredPublishConfig = Object.fromEntries( - Object.entries(manifest.publishConfig).filter(([key]) => !cliFlags.hasOwnProperty(key)) - ) - flatten(filteredPublishConfig, opts) - }else{ - flatten(manifest.publishConfig, opts) + if (cliFlags) { + // 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) + } else { + flatten(manifest.publishConfig, opts) } } From 7cf10307a69e7c2353d8a4ed8b0a23eba108b5fe Mon Sep 17 00:00:00 2001 From: roni25536 Date: Wed, 3 Apr 2024 17:02:58 +0000 Subject: [PATCH 4/6] remove condition --- lib/commands/publish.js | 14 +++++--------- lib/commands/unpublish.js | 14 +++++--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 6f62057c42197..e201c232a78cd 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -221,15 +221,11 @@ class Publish extends BaseCommand { } if (manifest.publishConfig) { const cliFlags = this.npm.config.data.get('cli').raw - if (cliFlags) { - // 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) - } else { - flatten(manifest.publishConfig, opts) - } + // 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 ccf8374892af2..a4d445a035b62 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -142,15 +142,11 @@ class Unpublish extends BaseCommand { // being unpublished, load up the publishConfig if (manifest?.name === spec.name && manifest.publishConfig) { const cliFlags = this.npm.config.data.get('cli').raw - if (cliFlags) { - // 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) - } else { - flatten(manifest.publishConfig, opts) - } + // 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) From badf0f4840f6dde2abf6e00e92009ce757b0aeff Mon Sep 17 00:00:00 2001 From: roni25536 Date: Wed, 3 Apr 2024 17:03:56 +0000 Subject: [PATCH 5/6] add prioritize CLI flags over publishConfig test for publish command --- .../test/lib/commands/publish.js.test.cjs | 4 ++ test/lib/commands/publish.js | 52 +++++++++++++++++++ 2 files changed, 56 insertions(+) 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: { From 3a00ffd44d00edc010550647e8e203d99d30d9dc Mon Sep 17 00:00:00 2001 From: roni25536 Date: Wed, 3 Apr 2024 17:04:30 +0000 Subject: [PATCH 6/6] add prioritize CLI flags over publishConfig no spec test for unpublish command --- test/lib/commands/unpublish.js | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) 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, {