From 7c89e74469520f80a0922987c761ebd808d8608c Mon Sep 17 00:00:00 2001
From: isaacs <i@izs.me>
Date: Mon, 15 Mar 2021 16:59:33 -0700
Subject: [PATCH] update lib/*.js to use new config structures

---
 bin/npx-cli.js         |  8 +++---
 lib/completion.js      | 10 ++++---
 lib/config.js          | 19 ++++++++------
 lib/doctor.js          |  2 +-
 lib/npm.js             | 38 +++++++++++++--------------
 lib/publish.js         |  5 ++--
 test/lib/completion.js | 13 ++++++----
 test/lib/config.js     | 59 ++++++++++++++++++++++++------------------
 test/lib/npm.js        | 18 ++++++++-----
 test/lib/publish.js    | 23 +++++++++++-----
 10 files changed, 115 insertions(+), 80 deletions(-)

diff --git a/bin/npx-cli.js b/bin/npx-cli.js
index f4a419972f7cf..a495090c64c78 100755
--- a/bin/npx-cli.js
+++ b/bin/npx-cli.js
@@ -24,11 +24,11 @@ const removed = new Set([
   ...removedOpts
 ])
 
-const { types, shorthands } = require('../lib/utils/config.js')
-const npmSwitches = Object.entries(types)
-  .filter(([key, type]) => type === Boolean ||
+const { definitions, shorthands } = require('../lib/utils/config/index.js')
+const npmSwitches = Object.entries(definitions)
+  .filter(([key, {type}]) => type === Boolean ||
     (Array.isArray(type) && type.includes(Boolean)))
-  .map(([key, type]) => key)
+  .map(([key]) => key)
 
 // things that don't take a value
 const switches = new Set([
diff --git a/lib/completion.js b/lib/completion.js
index 3ee68cdacaf95..b111887706e3a 100644
--- a/lib/completion.js
+++ b/lib/completion.js
@@ -29,13 +29,13 @@
 // as an array.
 //
 
-const { types, shorthands } = require('./utils/config.js')
+const { definitions, shorthands } = require('./utils/config/index.js')
 const deref = require('./utils/deref-command.js')
 const { aliases, cmdList, plumbing } = require('./utils/cmd-list.js')
 const aliasNames = Object.keys(aliases)
 const fullList = cmdList.concat(aliasNames).filter(c => !plumbing.includes(c))
 const nopt = require('nopt')
-const configNames = Object.keys(types)
+const configNames = Object.keys(definitions)
 const shorthandNames = Object.keys(shorthands)
 const allConfs = configNames.concat(shorthandNames)
 const isWindowsShell = require('./utils/is-windows-shell.js')
@@ -147,6 +147,10 @@ class Completion extends BaseCommand {
     // take a little shortcut and use npm's arg parsing logic.
     // don't have to worry about the last arg being implicitly
     // boolean'ed, since the last block will catch that.
+    const types = Object.entries(definitions).reduce((types, [key, def]) => {
+      types[key] = def.type
+      return types
+    }, {})
     const parsed = opts.conf =
       nopt(types, shorthands, partialWords.slice(0, -1), 0)
     // check if there's a command already.
@@ -256,7 +260,7 @@ const isFlag = word => {
   const split = word.match(/^(-*)((?:no-)+)?(.*)$/)
   const no = split[2]
   const conf = split[3]
-  const type = types[conf]
+  const {type} = definitions[conf]
   return no ||
     type === Boolean ||
     (Array.isArray(type) && type.includes(Boolean)) ||
diff --git a/lib/config.js b/lib/config.js
index c29253e430a33..7d0147712c1de 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -1,4 +1,5 @@
-const { defaults, types } = require('./utils/config.js')
+// don't expand so that we only assemble the set of defaults when needed
+const configDefs = require('./utils/config/index.js')
 
 const mkdirp = require('mkdirp-infer-owner')
 const { dirname } = require('path')
@@ -70,7 +71,7 @@ class Config extends BaseCommand {
       case 'get':
       case 'delete':
       case 'rm':
-        return Object.keys(types)
+        return Object.keys(configDefs.definitions)
       case 'edit':
       case 'list':
       case 'ls':
@@ -100,7 +101,7 @@ class Config extends BaseCommand {
           break
         case 'list':
         case 'ls':
-          await (this.npm.flatOptions.json ? this.listJson() : this.list())
+          await (this.npm.config.get('json') ? this.listJson() : this.list())
           break
         case 'edit':
           await this.edit()
@@ -117,7 +118,7 @@ class Config extends BaseCommand {
     if (!args.length)
       throw this.usageError()
 
-    const where = this.npm.flatOptions.global ? 'global' : 'user'
+    const where = this.npm.config.get('global') ? 'global' : 'user'
     for (const [key, val] of Object.entries(keyValues(args))) {
       this.npm.log.info('config', 'set %j %j', key, val)
       this.npm.config.set(key, val || '', where)
@@ -147,14 +148,15 @@ class Config extends BaseCommand {
     if (!keys.length)
       throw this.usageError()
 
-    const where = this.npm.flatOptions.global ? 'global' : 'user'
+    const where = this.npm.config.get('global') ? 'global' : 'user'
     for (const key of keys)
       this.npm.config.delete(key, where)
     await this.npm.config.save(where)
   }
 
   async edit () {
-    const { editor: e, global } = this.npm.flatOptions
+    const global = this.npm.config.get('global')
+    const e = this.npm.config.get('editor')
     const where = global ? 'global' : 'user'
     const file = this.npm.config.data.get(where).source
 
@@ -165,7 +167,8 @@ class Config extends BaseCommand {
     const data = (
       await readFile(file, 'utf8').catch(() => '')
     ).replace(/\r\n/g, '\n')
-    const defData = Object.entries(defaults).reduce((str, [key, val]) => {
+    const entries = Object.entries(configDefs.defaults)
+    const defData = entries.reduce((str, [key, val]) => {
       const obj = { [key]: val }
       const i = ini.stringify(obj)
         .replace(/\r\n/g, '\n') // normalizes output from ini.stringify
@@ -210,7 +213,7 @@ ${defData}
 
   async list () {
     const msg = []
-    const { long } = this.npm.flatOptions
+    const long = this.npm.config.get('long')
     for (const [where, { data, source }] of this.npm.config.data.entries()) {
       if (where === 'default' && !long)
         continue
diff --git a/lib/doctor.js b/lib/doctor.js
index fbe44714140aa..ae69b6a77af02 100644
--- a/lib/doctor.js
+++ b/lib/doctor.js
@@ -11,7 +11,7 @@ const { promisify } = require('util')
 const ansiTrim = require('./utils/ansi-trim.js')
 const isWindows = require('./utils/is-windows.js')
 const ping = require('./utils/ping.js')
-const { defaults: { registry: defaultRegistry } } = require('./utils/config.js')
+const { registry: { default: defaultRegistry } } = require('./utils/config/definitions.js')
 const lstat = promisify(fs.lstat)
 const readdir = promisify(fs.readdir)
 const access = promisify(fs.access)
diff --git a/lib/npm.js b/lib/npm.js
index 0534e630606e4..d80067a11c20f 100644
--- a/lib/npm.js
+++ b/lib/npm.js
@@ -36,24 +36,18 @@ const proxyCmds = new Proxy({}, {
   },
 })
 
-const { types, defaults, shorthands } = require('./utils/config.js')
+const { definitions, flatten, shorthands } = require('./utils/config/index.js')
 const { shellouts } = require('./utils/cmd-list.js')
 
 let warnedNonDashArg = false
 const _runCmd = Symbol('_runCmd')
 const _load = Symbol('_load')
-const _flatOptions = Symbol('_flatOptions')
 const _tmpFolder = Symbol('_tmpFolder')
 const _title = Symbol('_title')
 const npm = module.exports = new class extends EventEmitter {
   constructor () {
     super()
     require('./utils/perf.js')
-    this.modes = {
-      exec: 0o755,
-      file: 0o644,
-      umask: 0o22,
-    }
     this.started = Date.now()
     this.command = null
     this.commands = proxyCmds
@@ -62,8 +56,8 @@ const npm = module.exports = new class extends EventEmitter {
     this.version = require('../package.json').version
     this.config = new Config({
       npmPath: dirname(__dirname),
-      types,
-      defaults,
+      definitions,
+      flatten,
       shorthands,
     })
     this[_title] = process.title
@@ -140,9 +134,6 @@ const npm = module.exports = new class extends EventEmitter {
       if (!er && this.config.get('force'))
         this.log.warn('using --force', 'Recommended protections disabled.')
 
-      if (!er && !this[_flatOptions])
-        this[_flatOptions] = require('./utils/flat-options.js')(this)
-
       process.emit('timeEnd', 'npm:load')
       this.emit('load', er)
     })
@@ -162,14 +153,19 @@ const npm = module.exports = new class extends EventEmitter {
   }
 
   async [_load] () {
+    process.emit('time', 'npm:load:whichnode')
     const node = await which(process.argv[0]).catch(er => null)
+    process.emit('timeEnd', 'npm:load:whichnode')
     if (node && node.toUpperCase() !== process.execPath.toUpperCase()) {
       log.verbose('node symlink', node)
       process.execPath = node
       this.config.execPath = node
     }
 
+    process.emit('time', 'npm:load:configload')
     await this.config.load()
+    process.emit('timeEnd', 'npm:load:configload')
+
     this.argv = this.config.parsedArgv.remain
     // note: this MUST be shorter than the actual argv length, because it
     // uses the same memory, so node will truncate it if it's too long.
@@ -177,33 +173,37 @@ const npm = module.exports = new class extends EventEmitter {
     // don't show that.  (Regrettable historical choice to put it there.)
     // Any other secrets are configs only, so showing only the positional
     // args keeps those from being leaked.
+    process.emit('time', 'npm:load:setTitle')
     const tokrev = deref(this.argv[0]) === 'token' && this.argv[1] === 'revoke'
     this.title = tokrev ? 'npm token revoke' + (this.argv[2] ? ' ***' : '')
       : ['npm', ...this.argv].join(' ')
+    process.emit('timeEnd', 'npm:load:setTitle')
 
+    process.emit('time', 'npm:load:setupLog')
     this.color = setupLog(this.config)
+    process.emit('timeEnd', 'npm:load:setupLog')
     process.env.COLOR = this.color ? '1' : '0'
 
+    process.emit('time', 'npm:load:cleanupLog')
     cleanUpLogFiles(this.cache, this.config.get('logs-max'), log.warn)
+    process.emit('timeEnd', 'npm:load:cleanupLog')
 
     log.resume()
-    const umask = this.config.get('umask')
-    this.modes = {
-      exec: 0o777 & (~umask),
-      file: 0o666 & (~umask),
-      umask,
-    }
 
+    process.emit('time', 'npm:load:configScope')
     const configScope = this.config.get('scope')
     if (configScope && !/^@/.test(configScope))
       this.config.set('scope', `@${configScope}`, this.config.find('scope'))
+    process.emit('timeEnd', 'npm:load:configScope')
 
+    process.emit('time', 'npm:load:projectScope')
     this.projectScope = this.config.get('scope') ||
       getProjectScope(this.prefix)
+    process.emit('timeEnd', 'npm:load:projectScope')
   }
 
   get flatOptions () {
-    return this[_flatOptions]
+    return this.config.flat
   }
 
   get lockfileVersion () {
diff --git a/lib/publish.js b/lib/publish.js
index bfea1e139dbfc..ed05d1902a704 100644
--- a/lib/publish.js
+++ b/lib/publish.js
@@ -8,7 +8,7 @@ const pacote = require('pacote')
 const npa = require('npm-package-arg')
 const npmFetch = require('npm-registry-fetch')
 
-const { flatten } = require('./utils/flat-options.js')
+const flatten = require('./utils/config/flatten.js')
 const otplease = require('./utils/otplease.js')
 const { getContents, logTar } = require('./utils/tar.js')
 
@@ -141,7 +141,8 @@ class Publish extends BaseCommand {
   publishConfigToOpts (publishConfig) {
     // create a new object that inherits from the config stack
     // then squash the css-case into camelCase opts, like we do
-    return flatten({...flatten(this.npm.config.list[0]), ...publishConfig})
+    // this is Object.assign()'ed onto the base npm.flatOptions
+    return flatten(publishConfig, {})
   }
 }
 module.exports = Publish
diff --git a/test/lib/completion.js b/test/lib/completion.js
index 708f138251d19..c6ef901a7ed7d 100644
--- a/test/lib/completion.js
+++ b/test/lib/completion.js
@@ -63,11 +63,14 @@ const cmdList = {
   plumbing: [],
 }
 
+// only include a subset so that the snapshots aren't huge and
+// don't change when we add/remove config definitions.
+const definitions = require('../../lib/utils/config/definitions.js')
 const config = {
-  types: {
-    global: Boolean,
-    browser: [null, Boolean, String],
-    registry: [null, String],
+  definitions: {
+    global: definitions.global,
+    browser: definitions.browser,
+    registry: definitions.registry,
   },
   shorthands: {
     reg: ['--registry'],
@@ -80,7 +83,7 @@ const deref = (cmd) => {
 
 const Completion = requireInject('../../lib/completion.js', {
   '../../lib/utils/cmd-list.js': cmdList,
-  '../../lib/utils/config.js': config,
+  '../../lib/utils/config/index.js': config,
   '../../lib/utils/deref-command.js': deref,
   '../../lib/utils/is-windows-shell.js': false,
 })
diff --git a/test/lib/config.js b/test/lib/config.js
index 3aeb29f8d3426..14cd816171da5 100644
--- a/test/lib/config.js
+++ b/test/lib/config.js
@@ -1,4 +1,5 @@
 const t = require('tap')
+
 const requireInject = require('require-inject')
 const { EventEmitter } = require('events')
 
@@ -22,12 +23,21 @@ const redactCwd = (path) => {
 t.cleanSnapshot = (str) => redactCwd(str)
 
 let result = ''
-const types = {
-  'init-author-name': String,
-  'init-version': String,
-  'init.author.name': String,
-  'init.version': String,
-}
+
+const configDefs = require('../../lib/utils/config')
+const definitions = Object.entries(configDefs.definitions)
+  .filter(([key, def]) => {
+    return [
+      'init-author-name',
+      'init.author.name',
+      'init-version',
+      'init.version',
+    ].includes(key)
+  }).reduce((defs, [key, def]) => {
+    defs[key] = def
+    return defs
+  }, {})
+
 const defaults = {
   'init-author-name': '',
   'init-version': '1.0.0',
@@ -35,7 +45,7 @@ const defaults = {
   'init.version': '1.0.0',
 }
 
-const flatOptions = {
+const cliConfig = {
   editor: 'vi',
   json: false,
   long: false,
@@ -43,7 +53,6 @@ const flatOptions = {
 }
 
 const npm = {
-  flatOptions,
   log: {
     info: () => null,
     enableProgress: () => null,
@@ -53,10 +62,10 @@ const npm = {
     data: new Map(Object.entries({
       default: { data: defaults, source: 'default values' },
       global: { data: {}, source: '/etc/npmrc' },
-      cli: { data: flatOptions, source: 'command line options' },
+      cli: { data: cliConfig, source: 'command line options' },
     })),
     get (key) {
-      return flatOptions[key]
+      return cliConfig[key]
     },
     validate () {
       return true
@@ -70,7 +79,7 @@ const npm = {
 const usageUtil = () => 'usage instructions'
 
 const mocks = {
-  '../../lib/utils/config.js': { defaults, types },
+  '../../lib/utils/config/index.js': { defaults, definitions },
   '../../lib/utils/usage.js': usageUtil,
 }
 
@@ -110,13 +119,13 @@ t.test('config list overrides', t => {
     },
     source: '~/.npmrc',
   })
-  flatOptions['init.author.name'] = 'Bar'
+  cliConfig['init.author.name'] = 'Bar'
   npm.config.find = () => 'cli'
   result = ''
   t.teardown(() => {
     result = ''
     npm.config.data.delete('user')
-    delete flatOptions['init.author.name']
+    delete cliConfig['init.author.name']
     delete npm.config.find
   })
 
@@ -129,12 +138,12 @@ t.test('config list overrides', t => {
 t.test('config list --long', t => {
   t.plan(2)
 
-  npm.config.find = key => key in flatOptions ? 'cli' : 'default'
-  flatOptions.long = true
+  npm.config.find = key => key in cliConfig ? 'cli' : 'default'
+  cliConfig.long = true
   result = ''
   t.teardown(() => {
     delete npm.config.find
-    flatOptions.long = false
+    cliConfig.long = false
     result = ''
   })
 
@@ -147,7 +156,7 @@ t.test('config list --long', t => {
 t.test('config list --json', t => {
   t.plan(2)
 
-  flatOptions.json = true
+  cliConfig.json = true
   result = ''
   npm.config.list = [{
     '//private-reg.npmjs.org/:_authThoken': 'f00ba1',
@@ -158,7 +167,7 @@ t.test('config list --json', t => {
 
   t.teardown(() => {
     delete npm.config.list
-    flatOptions.json = false
+    cliConfig.json = false
     npm.config.get = npmConfigGet
     result = ''
   })
@@ -246,13 +255,13 @@ t.test('config delete key --global', t => {
     t.equal(where, 'global', 'should save global config post-delete')
   }
 
-  flatOptions.global = true
+  cliConfig.global = true
   config.exec(['delete', 'foo'], (err) => {
     t.ifError(err, 'npm config delete key --global')
   })
 
   t.teardown(() => {
-    flatOptions.global = false
+    cliConfig.global = false
     delete npm.config.delete
     delete npm.config.save
   })
@@ -401,13 +410,13 @@ t.test('config set key --global', t => {
     t.equal(where, 'global', 'should save global config')
   }
 
-  flatOptions.global = true
+  cliConfig.global = true
   config.exec(['set', 'foo', 'bar'], (err) => {
     t.ifError(err, 'npm config set key --global')
   })
 
   t.teardown(() => {
-    flatOptions.global = false
+    cliConfig.global = false
     delete npm.config.set
     delete npm.config.save
   })
@@ -555,7 +564,7 @@ sign-git-commit=true`
 t.test('config edit --global', t => {
   t.plan(6)
 
-  flatOptions.global = true
+  cliConfig.global = true
   const npmrc = 'init.author.name=Foo'
   npm.config.data.set('global', {
     source: '/etc/npmrc',
@@ -595,7 +604,7 @@ t.test('config edit --global', t => {
   })
 
   t.teardown(() => {
-    flatOptions.global = false
+    cliConfig.global = false
     npm.config.data.delete('user')
     delete npm.config.save
   })
@@ -612,7 +621,7 @@ t.test('completion', t => {
   testComp(['npm', 'config'], ['get', 'set', 'delete', 'ls', 'rm', 'edit', 'list'])
   testComp(['npm', 'config', 'set', 'foo'], [])
 
-  const possibleConfigKeys = [...Object.keys(types)]
+  const possibleConfigKeys = [...Object.keys(definitions)]
   testComp(['npm', 'config', 'get'], possibleConfigKeys)
   testComp(['npm', 'config', 'set'], possibleConfigKeys)
   testComp(['npm', 'config', 'delete'], possibleConfigKeys)
diff --git a/test/lib/npm.js b/test/lib/npm.js
index 87cbea8f2c617..dfb3cca64e1ff 100644
--- a/test/lib/npm.js
+++ b/test/lib/npm.js
@@ -42,7 +42,7 @@ const npmlog = require('npmlog')
 
 const npmPath = resolve(__dirname, '..', '..')
 const Config = require('@npmcli/config')
-const { types, defaults, shorthands } = require('../../lib/utils/config.js')
+const { definitions, shorthands } = require('../../lib/utils/config')
 const freshConfig = (opts = {}) => {
   for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e)))
     delete process.env[env]
@@ -50,8 +50,7 @@ const freshConfig = (opts = {}) => {
   process.env.npm_config_cache = CACHE
 
   npm.config = new Config({
-    types,
-    defaults,
+    definitions,
     shorthands,
     npmPath,
     log: npmlog,
@@ -160,7 +159,7 @@ t.test('npm.load', t => {
       npm.load(third)
       t.equal(thirdCalled, true, 'third callbback got called')
       t.match(logs, [
-        ['timing', 'npm:load', /Completed in [0-9]+ms/],
+        ['timing', 'npm:load', /Completed in [0-9.]+ms/],
       ])
       logs.length = 0
 
@@ -288,6 +287,11 @@ t.test('npm.load', t => {
 
       t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope')
       t.match(logs.filter(l => l[0] !== 'timing' || !/^config:/.test(l[1])), [
+        [
+          'timing',
+          'npm:load:whichnode',
+          /Completed in [0-9.]+ms/,
+        ],
         [
           'verbose',
           'node symlink',
@@ -296,7 +300,7 @@ t.test('npm.load', t => {
         [
           'timing',
           'npm:load',
-          /Completed in [0-9]+ms/,
+          /Completed in [0-9.]+ms/,
         ],
       ])
       logs.length = 0
@@ -328,12 +332,12 @@ t.test('npm.load', t => {
         [
           'timing',
           'command:config',
-          /Completed in [0-9]+ms/,
+          /Completed in [0-9.]+ms/,
         ],
         [
           'timing',
           'command:get',
-          /Completed in [0-9]+ms/,
+          /Completed in [0-9.]+ms/,
         ],
       ])
       t.same(consoleLogs, [['scope=@foo\n\u2010not-a-dash=undefined']])
diff --git a/test/lib/publish.js b/test/lib/publish.js
index d1a1cd630c0e6..f61377b54f997 100644
--- a/test/lib/publish.js
+++ b/test/lib/publish.js
@@ -11,7 +11,11 @@ const log = require('npmlog')
 log.level = 'silent'
 
 // mock config
-const {defaults} = require('../../lib/utils/config.js')
+const {definitions} = require('../../lib/utils/config')
+const defaults = Object.entries(definitions).reduce((defaults, [key, def]) => {
+  defaults[key] = def.default
+  return defaults
+}, {})
 
 const config = defaults
 
@@ -488,11 +492,12 @@ t.test('able to publish after if encountered multiple configs', t => {
     }, null, 2),
   })
 
-  const configList = [
-    { ...defaults, tag },
-    { ...defaults, registry: `https://other.registry`, tag: 'some-tag' },
-    defaults,
-  ]
+  const configList = [defaults]
+  configList.unshift(Object.assign(Object.create(configList[0]), {
+    registry: `https://other.registry`,
+    tag: 'some-tag',
+  }))
+  configList.unshift(Object.assign(Object.create(configList[0]), { tag }))
 
   const Publish = requireInject('../../lib/publish.js', {
     libnpmpublish: {
@@ -502,7 +507,13 @@ t.test('able to publish after if encountered multiple configs', t => {
     },
   })
   const publish = new Publish({
+    // what would be flattened by the configList created above
+    flatOptions: {
+      defaultTag: 'better-tag',
+      registry: 'https://other.registry',
+    },
     config: {
+      get: key => configList[0][key],
       list: configList,
       getCredentialsByURI: (uri) => {
         t.same(uri, registry, 'gets credentials for expected registry')