diff --git a/docs/lib/content/commands/npm-config.md b/docs/lib/content/commands/npm-config.md index 1874aee418d00..60776f33ad4cf 100644 --- a/docs/lib/content/commands/npm-config.md +++ b/docs/lib/content/commands/npm-config.md @@ -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`. diff --git a/lib/commands/config.js b/lib/commands/config.js index ac5a74d01f7de..dfd44015cd043 100644 --- a/lib/commands/config.js +++ b/lib/commands/config.js @@ -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') } diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js index f2bdcc7231ddf..7d5140bffc45b 100644 --- a/test/lib/commands/config.js +++ b/test/lib/commands/config.js @@ -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 => { diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index a307025d1cc93..7148e126a5db9 100644 --- a/workspaces/config/lib/index.js +++ b/workspaces/config/lib/index.js @@ -169,11 +169,7 @@ class Config { if (!this.loaded) { throw new Error('call config.load() before reading values') } - // TODO single use? - return this.#find(key) - } - #find (key) { // have to look in reverse order const entries = [...this.data.entries()] for (let i = entries.length - 1; i > -1; i--) { @@ -210,8 +206,11 @@ class Config { throw new Error('invalid config location param: ' + where) } this.#checkDeprecated(key) - const { data } = this.data.get(where) + const { data, raw } = this.data.get(where) data[key] = val + if (['global', 'user', 'project'].includes(where)) { + raw[key] = val + } // this is now dirty, the next call to this.valid will have to check it this.data.get(where)[_valid] = null @@ -244,7 +243,11 @@ class Config { if (!confTypes.has(where)) { throw new Error('invalid config location param: ' + where) } - delete this.data.get(where).data[key] + const { data, raw } = this.data.get(where) + delete data[key] + if (['global', 'user', 'project'].includes(where)) { + delete raw[key] + } } async load () { @@ -537,6 +540,7 @@ class Config { } #loadObject (obj, where, source, er = null) { + // obj is the raw data read from the file const conf = this.data.get(where) if (conf.source) { const m = `double-loading "${where}" configs from ${source}, ` + @@ -724,7 +728,9 @@ class Config { } } - const iniData = ini.stringify(conf.data).trim() + '\n' + // We need the actual raw data before we called parseField so that we are + // saving the same content back to the file + const iniData = ini.stringify(conf.raw).trim() + '\n' if (!iniData.trim()) { // ignore the unlink error (eg, if file doesn't exist) await unlink(conf.source).catch(er => {}) @@ -873,7 +879,7 @@ class ConfigData { #raw = null constructor (parent) { this.#data = Object.create(parent && parent.data) - this.#raw = null + this.#raw = {} this[_valid] = true } @@ -897,7 +903,7 @@ class ConfigData { } set loadError (e) { - if (this[_loadError] || this.#raw) { + if (this[_loadError] || (Object.keys(this.#raw).length)) { throw new Error('cannot set ConfigData loadError after load') } this[_loadError] = e @@ -908,7 +914,7 @@ class ConfigData { } set raw (r) { - if (this.#raw || this[_loadError]) { + if (Object.keys(this.#raw).length || this[_loadError]) { throw new Error('cannot set ConfigData raw after load') } this.#raw = r diff --git a/workspaces/config/test/index.js b/workspaces/config/test/index.js index 99c1c3647ef77..bd19ee48bef4f 100644 --- a/workspaces/config/test/index.js +++ b/workspaces/config/test/index.js @@ -1317,3 +1317,26 @@ t.test('workspaces', async (t) => { t.match(logs[0], ['info', /^found workspace root at/], 'logged info about workspace root') }) }) + +t.test('env-replaced config from files is not clobbered when saving', async (t) => { + const path = t.testdir() + const opts = { + shorthands: {}, + argv: ['node', __filename, `--userconfig=${path}/.npmrc`], + env: { TEST: 'test value' }, + definitions: { + registry: { default: 'https://registry.npmjs.org/' }, + }, + npmPath: process.cwd(), + } + const c = new Config(opts) + await c.load() + c.set('test', '${TEST}', 'user') + await c.save('user') + const d = new Config(opts) + await d.load() + d.set('other', '${SOMETHING}', 'user') + await d.save('user') + const rc = readFileSync(`${path}/.npmrc`, 'utf8') + t.match(rc, 'test=${TEST}', '${TEST} is present, not parsed') +})