Skip to content

Commit 9529b8f

Browse files
committed
fix: warn on invalid publishConfig
1 parent 593c849 commit 9529b8f

File tree

9 files changed

+47
-9
lines changed

9 files changed

+47
-9
lines changed

lib/commands/config.js

+3
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,9 @@ ${defData}
366366
const { content } = await pkgJson.normalize(this.npm.prefix).catch(() => ({ content: {} }))
367367

368368
if (content.publishConfig) {
369+
for (const key in content.publishConfig) {
370+
this.npm.config.checkUnknown('publishConfig', key)
371+
}
369372
const pkgPath = resolve(this.npm.prefix, 'package.json')
370373
msg.push(`; "publishConfig" from ${pkgPath}`)
371374
msg.push('; This set of config values will be used at publish-time.', '')

lib/commands/publish.js

+5
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,11 @@ class Publish extends BaseCommand {
266266
// corresponding `publishConfig` settings
267267
const filteredPublishConfig = Object.fromEntries(
268268
Object.entries(manifest.publishConfig).filter(([key]) => !(key in cliFlags)))
269+
if (logWarnings) {
270+
for (const key in filteredPublishConfig) {
271+
this.npm.config.checkUnknown('publishConfig', key)
272+
}
273+
}
269274
flatten(filteredPublishConfig, opts)
270275
}
271276
return manifest

lib/commands/unpublish.js

+3
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@ class Unpublish extends BaseCommand {
145145
// corresponding `publishConfig` settings
146146
const filteredPublishConfig = Object.fromEntries(
147147
Object.entries(manifest.publishConfig).filter(([key]) => !(key in cliFlags)))
148+
for (const key in filteredPublishConfig) {
149+
this.npm.config.checkUnknown('publishConfig', key)
150+
}
148151
flatten(filteredPublishConfig, opts)
149152
}
150153

tap-snapshots/test/lib/commands/config.js.test.cjs

+8-1
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,13 @@ color = {COLOR}
413413
; "publishConfig" from {CWD}/prefix/package.json
414414
; This set of config values will be used at publish-time.
415415
416-
_authToken = (protected)
416+
//some.registry:_authToken = (protected)
417+
other = "not defined"
417418
registry = "https://some.registry"
418419
`
420+
421+
exports[`test/lib/commands/config.js TAP config list with publishConfig local > warns about unknown config 1`] = `
422+
Array [
423+
"Unknown publishConfig config /"other/". This will stop working in the next major version of npm.",
424+
]
425+
`

tap-snapshots/test/lib/commands/publish.js.test.cjs

+9
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,15 @@ exports[`test/lib/commands/publish.js TAP re-loads publishConfig.registry if add
290290

291291
exports[`test/lib/commands/publish.js TAP respects publishConfig.registry, runs appropriate scripts > new package version 1`] = `
292292
293+
> @npmcli/test-package@1.0.0 prepublishOnly
294+
> touch scripts-prepublishonly
295+
296+
> @npmcli/test-package@1.0.0 publish
297+
> touch scripts-publish
298+
299+
> @npmcli/test-package@1.0.0 postpublish
300+
> touch scripts-postpublish
301+
+ @npmcli/test-package@1.0.0
293302
`
294303

295304
exports[`test/lib/commands/publish.js TAP restricted access > must match snapshot 1`] = `

test/lib/commands/config.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -164,16 +164,17 @@ t.test('config list with publishConfig', async t => {
164164
prefixDir: {
165165
'package.json': JSON.stringify({
166166
publishConfig: {
167+
other: 'not defined',
167168
registry: 'https://some.registry',
168-
_authToken: 'mytoken',
169+
'//some.registry:_authToken': 'mytoken',
169170
},
170171
}),
171172
},
172173
...opts,
173174
})
174175

175176
t.test('local', async t => {
176-
const { npm, joinedOutput } = await loadMockNpmWithPublishConfig(t)
177+
const { npm, logs, joinedOutput } = await loadMockNpmWithPublishConfig(t)
177178

178179
await npm.exec('config', ['list'])
179180

@@ -182,6 +183,7 @@ t.test('config list with publishConfig', async t => {
182183
t.match(output, 'registry = "https://some.registry"')
183184

184185
t.matchSnapshot(output, 'output matches snapshot')
186+
t.matchSnapshot(logs.warn, 'warns about unknown config')
185187
})
186188

187189
t.test('global', async t => {

test/lib/commands/publish.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,14 @@ t.test('respects publishConfig.registry, runs appropriate scripts', async t => {
2929
publish: 'touch scripts-publish',
3030
postpublish: 'touch scripts-postpublish',
3131
},
32-
publishConfig: { registry: alternateRegistry },
32+
publishConfig: {
33+
other: 'not defined',
34+
registry: alternateRegistry,
35+
},
3336
}
34-
const { npm, joinedOutput, prefix, registry } = await loadNpmWithRegistry(t, {
37+
const { npm, joinedOutput, logs, prefix, registry } = await loadNpmWithRegistry(t, {
3538
config: {
36-
loglevel: 'silent',
39+
loglevel: 'warn',
3740
[`${alternateRegistry.slice(6)}/:_authToken`]: 'test-other-token',
3841
},
3942
prefixDir: {
@@ -49,6 +52,7 @@ t.test('respects publishConfig.registry, runs appropriate scripts', async t => {
4952
t.equal(fs.existsSync(path.join(prefix, 'scripts-prepublish')), false, 'did not run prepublish')
5053
t.equal(fs.existsSync(path.join(prefix, 'scripts-publish')), true, 'ran publish')
5154
t.equal(fs.existsSync(path.join(prefix, 'scripts-postpublish')), true, 'ran postpublish')
55+
t.same(logs.warn, ['Unknown publishConfig config "other". This will stop working in the next major version of npm.'])
5256
})
5357

5458
t.test('re-loads publishConfig.registry if added during script process', async t => {

test/lib/commands/unpublish.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ t.test('dryRun with no args', async t => {
380380

381381
t.test('publishConfig no spec', async t => {
382382
const alternateRegistry = 'https://other.registry.npmjs.org'
383-
const { joinedOutput, npm } = await loadMockNpm(t, {
383+
const { logs, joinedOutput, npm } = await loadMockNpm(t, {
384384
config: {
385385
force: true,
386386
'//other.registry.npmjs.org/:_authToken': 'test-other-token',
@@ -390,6 +390,7 @@ t.test('publishConfig no spec', async t => {
390390
name: pkg,
391391
version: '1.0.0',
392392
publishConfig: {
393+
other: 'not defined',
393394
registry: alternateRegistry,
394395
},
395396
}, null, 2),
@@ -406,6 +407,10 @@ t.test('publishConfig no spec', async t => {
406407
registry.unpublish({ manifest })
407408
await npm.exec('unpublish', [])
408409
t.equal(joinedOutput(), '- test-package')
410+
t.same(logs.warn, [
411+
'using --force Recommended protections disabled.',
412+
'Unknown publishConfig config "other". This will stop working in the next major version of npm.',
413+
])
409414
})
410415

411416
t.test('prioritize CLI flags over publishConfig no spec', async t => {

workspaces/config/lib/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -584,14 +584,14 @@ class Config {
584584
}
585585
// Some defaults like npm-version are not user-definable and thus don't have definitions
586586
if (where !== 'default') {
587-
this.#checkUnknown(where, key)
587+
this.checkUnknown(where, key)
588588
}
589589
conf.data[k] = v
590590
}
591591
}
592592
}
593593

594-
#checkUnknown (where, key) {
594+
checkUnknown (where, key) {
595595
if (!this.definitions[key]) {
596596
if (internalEnv.includes(key)) {
597597
return

0 commit comments

Comments
 (0)