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

fix: don't overwrite config keys unless they've really changed #6328

Merged
merged 2 commits into from Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/lib/content/commands/npm-config.md
Expand Up @@ -35,7 +35,7 @@ npm set key=value [key=value...]

Sets each of the config keys to the value provided.

If value is omitted, then it sets it to an empty string.
If value is omitted, the key will be removed from your config file entirely.

Note: for backwards compatibility, `npm config set key value` is supported
as an alias for `npm config set key=value`.
Expand Down
8 changes: 7 additions & 1 deletion lib/commands/config.js
Expand Up @@ -163,7 +163,13 @@ class Config extends BaseCommand {
`The \`${baseKey}\` option is deprecated, and can not be set in this way${deprecated}`
)
}
this.npm.config.set(key, val || '', where)

if (val === '') {
this.npm.config.delete(key, where)
} else {
this.npm.config.set(key, val, where)
}

if (!this.npm.config.validate(where)) {
log.warn('config', 'omitting invalid config values')
}
Expand Down
4 changes: 2 additions & 2 deletions test/lib/commands/config.js
Expand Up @@ -298,13 +298,13 @@ t.test('config set key1 value1 key2=value2 key3', async t => {

t.equal(sandbox.config.get('access'), 'restricted', 'access was set')
t.equal(sandbox.config.get('all'), false, 'all was set')
t.equal(sandbox.config.get('audit'), false, 'audit was set')
t.equal(sandbox.config.get('audit'), true, 'audit was unset and restored to its default')

const contents = await fs.readFile(join(home, '.npmrc'), { encoding: 'utf8' })
const rc = ini.parse(contents)
t.equal(rc.access, 'restricted', 'access is set to restricted')
t.equal(rc.all, false, 'all is set to false')
t.equal(rc.audit, false, 'audit is set to false')
t.not(contents.includes('audit='), 'config file does not set audit')
})

t.test('config set invalid key logs warning', async t => {
Expand Down