fix: move all definitions to @npmcli/config package

Author: lukekarrys

DEPENDENCIES.md

@@ -187,11 +187,13 @@ graph LR;
   npmcli-config-->nopt;
   npmcli-config-->npmcli-eslint-config["@npmcli/eslint-config"];
   npmcli-config-->npmcli-map-workspaces["@npmcli/map-workspaces"];
+  npmcli-config-->npmcli-mock-globals["@npmcli/mock-globals"];
   npmcli-config-->npmcli-template-oss["@npmcli/template-oss"];
   npmcli-config-->proc-log;
   npmcli-config-->read-package-json-fast;
   npmcli-config-->semver;
   npmcli-docs-->ignore-walk;
+  npmcli-docs-->npmcli-config["@npmcli/config"];
   npmcli-docs-->npmcli-eslint-config["@npmcli/eslint-config"];
   npmcli-docs-->npmcli-template-oss["@npmcli/template-oss"];
   npmcli-docs-->semver;
@@ -644,10 +646,12 @@ graph LR;
   npmcli-arborist-->tcompare;
   npmcli-arborist-->treeverse;
   npmcli-arborist-->walk-up-path;
+  npmcli-config-->ci-info;
   npmcli-config-->ini;
   npmcli-config-->nopt;
   npmcli-config-->npmcli-eslint-config["@npmcli/eslint-config"];
   npmcli-config-->npmcli-map-workspaces["@npmcli/map-workspaces"];
+  npmcli-config-->npmcli-mock-globals["@npmcli/mock-globals"];
   npmcli-config-->npmcli-template-oss["@npmcli/template-oss"];
   npmcli-config-->proc-log;
   npmcli-config-->read-package-json-fast;
@@ -659,6 +663,7 @@ graph LR;
   npmcli-docs-->ignore-walk;
   npmcli-docs-->isaacs-string-locale-compare["@isaacs/string-locale-compare"];
   npmcli-docs-->jsdom;
+  npmcli-docs-->npmcli-config["@npmcli/config"];
   npmcli-docs-->npmcli-eslint-config["@npmcli/eslint-config"];
   npmcli-docs-->npmcli-template-oss["@npmcli/template-oss"];
   npmcli-docs-->rehype-stringify;
@@ -826,8 +831,8 @@ packages higher up the chain.
  - @npmcli/arborist
  - @npmcli/metavuln-calculator
  - pacote, libnpmhook, libnpmorg, libnpmsearch, libnpmteam, npm-profile
- - npm-registry-fetch, @npmcli/package-json, libnpmversion
- - @npmcli/git, make-fetch-happen, @npmcli/config, init-package-json
- - @npmcli/installed-package-contents, @npmcli/map-workspaces, cacache, npm-pick-manifest, @npmcli/run-script, read-package-json, promzard
- - @npmcli/docs, @npmcli/fs, npm-bundled, read-package-json-fast, unique-filename, npm-install-checks, npm-package-arg, npm-packlist, normalize-package-data, bin-links, nopt, npmlog, parse-conflict-json, @npmcli/mock-globals, read
- - @npmcli/eslint-config, @npmcli/template-oss, ignore-walk, semver, npm-normalize-package-bin, @npmcli/name-from-folder, json-parse-even-better-errors, fs-minipass, ssri, unique-slug, @npmcli/promise-spawn, hosted-git-info, proc-log, validate-npm-package-name, @npmcli/node-gyp, minipass-fetch, @npmcli/query, cmd-shim, read-cmd-shim, write-file-atomic, abbrev, are-we-there-yet, gauge, minify-registry-metadata, ini, @npmcli/disparity-colors, mute-stream, npm-audit-report, npm-user-validate
+ - @npmcli/docs, npm-registry-fetch, @npmcli/package-json, libnpmversion
+ - @npmcli/config, @npmcli/git, make-fetch-happen, init-package-json
+ - @npmcli/map-workspaces, @npmcli/installed-package-contents, cacache, npm-pick-manifest, @npmcli/run-script, read-package-json, promzard
+ - read-package-json-fast, nopt, @npmcli/mock-globals, @npmcli/fs, npm-bundled, unique-filename, npm-install-checks, npm-package-arg, npm-packlist, normalize-package-data, bin-links, npmlog, parse-conflict-json, read
+ - @npmcli/name-from-folder, json-parse-even-better-errors, npm-normalize-package-bin, ini, abbrev, proc-log, semver, @npmcli/eslint-config, @npmcli/template-oss, ignore-walk, fs-minipass, ssri, unique-slug, @npmcli/promise-spawn, hosted-git-info, validate-npm-package-name, @npmcli/node-gyp, minipass-fetch, @npmcli/query, cmd-shim, read-cmd-shim, write-file-atomic, are-we-there-yet, gauge, minify-registry-metadata, @npmcli/disparity-colors, mute-stream, npm-audit-report, npm-user-validate

bin/npx-cli.js

@@ -24,7 +24,7 @@ const removed = new Set([
   ...removedOpts,
 ])
 
-const { definitions, shorthands } = require('../lib/utils/config/index.js')
+const { definitions, shorthands } = require('@npmcli/config/lib/definitions')
 const npmSwitches = Object.entries(definitions)
   .filter(([key, { type }]) => type === Boolean ||
     (Array.isArray(type) && type.includes(Boolean)))

docs/lib/index.js

@@ -3,7 +3,7 @@ const { join, basename, resolve } = require('path')
 const transformHTML = require('./transform-html.js')
 const { version } = require('../../lib/npm.js')
 const { aliases } = require('../../lib/utils/cmd-list')
-const { shorthands, definitions } = require('../../lib/utils/config/index.js')
+const { shorthands, definitions } = require('@npmcli/config/lib/definitions')
 
 const DOC_EXT = '.md'
 

docs/package.json

@@ -21,6 +21,7 @@
   },
   "devDependencies": {
     "@isaacs/string-locale-compare": "^1.1.0",
+    "@npmcli/config": "^6.1.7",
     "@npmcli/eslint-config": "^4.0.0",
     "@npmcli/template-oss": "4.14.1",
     "front-matter": "^4.0.2",

lib/base-command.js

@@ -2,7 +2,7 @@
 
 const { relative } = require('path')
 
-const definitions = require('./utils/config/definitions.js')
+const { definitions } = require('@npmcli/config/lib/definitions')
 const getWorkspaces = require('./workspaces/get-workspaces.js')
 const { aliases: cmdAliases } = require('./utils/cmd-list')
 

lib/commands/completion.js

@@ -34,7 +34,7 @@ const nopt = require('nopt')
 const { resolve } = require('path')
 
 const Npm = require('../npm.js')
-const { definitions, shorthands } = require('../utils/config/index.js')
+const { definitions, shorthands } = require('@npmcli/config/lib/definitions')
 const { commands, aliases, deref } = require('../utils/cmd-list.js')
 const configNames = Object.keys(definitions)
 const shorthandNames = Object.keys(shorthands)

lib/commands/config.js

@@ -1,13 +1,11 @@
-// don't expand so that we only assemble the set of defaults when needed
-const configDefs = require('../utils/config/index.js')
-
 const { mkdir, readFile, writeFile } = require('fs/promises')
 const { dirname, resolve } = require('path')
 const { spawn } = require('child_process')
 const { EOL } = require('os')
 const ini = require('ini')
 const localeCompare = require('@isaacs/string-locale-compare')('en')
 const pkgJson = require('@npmcli/package-json')
+const { defaults, definitions } = require('@npmcli/config/lib/definitions')
 const log = require('../utils/log-shim.js')
 
 // These are the configs that we can nerf-dart. Not all of them currently even
@@ -102,7 +100,7 @@ class Config extends BaseCommand {
       case 'get':
       case 'delete':
       case 'rm':
-        return Object.keys(configDefs.definitions)
+        return Object.keys(definitions)
       case 'edit':
       case 'list':
       case 'ls':
@@ -219,7 +217,7 @@ class Config extends BaseCommand {
     const data = (
       await readFile(file, 'utf8').catch(() => '')
     ).replace(/\r\n/g, '\n')
-    const entries = Object.entries(configDefs.defaults)
+    const entries = Object.entries(defaults)
     const defData = entries.reduce((str, [key, val]) => {
       const obj = { [key]: val }
       const i = ini.stringify(obj)

lib/commands/doctor.js

@@ -9,9 +9,7 @@ const semver = require('semver')
 const { promisify } = require('util')
 const log = require('../utils/log-shim.js')
 const ping = require('../utils/ping.js')
-const {
-  registry: { default: defaultRegistry },
-} = require('../utils/config/definitions.js')
+const { defaults } = require('@npmcli/config/lib/definitions')
 const lstat = promisify(fs.lstat)
 const readdir = promisify(fs.readdir)
 const access = promisify(fs.access)
@@ -364,10 +362,10 @@ class Doctor extends BaseCommand {
   }
 
   async checkNpmRegistry () {
-    if (this.npm.flatOptions.registry !== defaultRegistry) {
-      throw `Try \`npm config set registry=${defaultRegistry}\``
+    if (this.npm.flatOptions.registry !== defaults.registry) {
+      throw `Try \`npm config set registry=${defaults.registry}\``
     } else {
-      return `using default registry (${defaultRegistry})`
+      return `using default registry (${defaults.registry})`
     }
   }
 

lib/commands/publish.js

@@ -15,7 +15,7 @@ const { getContents, logTar } = require('../utils/tar.js')
 // keys that npm supports in .npmrc files and elsewhere.  We *may* want to
 // revisit this at some point, and have a minimal set that's a SemVer-major
 // change that ought to get a RFC written on it.
-const { flatten } = require('../utils/config/index.js')
+const { flatten } = require('@npmcli/config/lib/definitions')
 const pkgJson = require('@npmcli/package-json')
 
 const BaseCommand = require('../base-command.js')

lib/commands/unpublish.js

@@ -4,7 +4,7 @@ const npa = require('npm-package-arg')
 const npmFetch = require('npm-registry-fetch')
 const pkgJson = require('@npmcli/package-json')
 
-const { flatten } = require('../utils/config/index.js')
+const { flatten } = require('@npmcli/config/lib/definitions')
 const getIdentity = require('../utils/get-identity.js')
 const log = require('../utils/log-shim')
 const otplease = require('../utils/otplease.js')

lib/npm.js

@@ -6,7 +6,7 @@ const fs = require('fs/promises')
 // Patch the global fs module here at the app level
 require('graceful-fs').gracefulify(require('fs'))
 
-const { definitions, flatten, shorthands } = require('./utils/config/index.js')
+const { definitions, flatten, shorthands } = require('@npmcli/config/lib/definitions')
 const usage = require('./utils/npm-usage.js')
 const LogFile = require('./utils/log-file.js')
 const Timers = require('./utils/timers.js')

package-lock.json

@@ -182,6 +182,7 @@
       "license": "ISC",
       "devDependencies": {
         "@isaacs/string-locale-compare": "^1.1.0",
+        "@npmcli/config": "^6.1.7",
         "@npmcli/eslint-config": "^4.0.0",
         "@npmcli/template-oss": "4.14.1",
         "front-matter": "^4.0.2",
@@ -15668,6 +15669,7 @@
       "license": "ISC",
       "dependencies": {
         "@npmcli/map-workspaces": "^3.0.2",
+        "ci-info": "^3.8.0",
         "ini": "^4.1.0",
         "nopt": "^7.0.0",
         "proc-log": "^3.0.0",
@@ -15677,6 +15679,7 @@
       },
       "devDependencies": {
         "@npmcli/eslint-config": "^4.0.0",
+        "@npmcli/mock-globals": "^1.0.0",
         "@npmcli/template-oss": "4.14.1",
         "tap": "^16.3.4"
       },

scripts/fish-completion.js

@@ -3,7 +3,7 @@ const fs = require('fs/promises')
 const { resolve } = require('path')
 
 const { commands, aliases } = require('../lib/utils/cmd-list.js')
-const { definitions } = require('../lib/utils/config/index.js')
+const { definitions } = require('@npmcli/config/lib/definitions')
 
 async function main () {
   const file = resolve(__dirname, '..', 'lib', 'utils', 'completion.fish')

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

@@ -161,6 +161,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna
   "workspaces": null,
   "workspaces-update": true,
   "yes": null,
+  "npm-version": "{NPM-VERSION}",
   "metrics-registry": "https://registry.npmjs.org/"
 }
 `
@@ -256,6 +257,7 @@ message = "%s"
 metrics-registry = "https://registry.npmjs.org/" 
 node-options = null 
 noproxy = [""] 
+npm-version = "{NPM-VERSION}" 
 offline = false 
 omit = [] 
 omit-lockfile-registry-resolved = false 
@@ -341,6 +343,18 @@ userconfig = "{HOME}/.npmrc"
 `
 
 exports[`test/lib/commands/config.js TAP config list > output matches snapshot 1`] = `
+; "global" config from {GLOBALPREFIX}/npmrc
+
+globalloaded = "yes" 
+
+; "user" config from {HOME}/.npmrc
+
+userloaded = "yes" 
+
+; "project" config from {LOCALPREFIX}/.npmrc
+
+projectloaded = "yes" 
+
 ; "cli" config from command line options
 
 cache = "{NPMDIR}/test/lib/commands/tap-testdir-config-config-list-sandbox/cache" 
@@ -383,6 +397,7 @@ global-prefix = "{LOCALPREFIX}"
 globalconfig = "{GLOBALPREFIX}/npmrc" 
 init-module = "{HOME}/.npm-init.js" 
 local-prefix = "{LOCALPREFIX}" 
+npm-version = "{NPM-VERSION}" 
 ; prefix = "{LOCALPREFIX}" ; overridden by cli
 user-agent = "npm/{NPM-VERSION} node/{NODE-VERSION} {PLATFORM} {ARCH} workspaces/false" 
 ; userconfig = "{HOME}/.npmrc" ; overridden by cli

tap-snapshots/test/lib/docs.js.test.cjs

@@ -2390,7 +2390,7 @@ Object {
   "noProxy": "",
   "npmBin": "{CWD}/{TESTDIR}/docs.js",
   "npmCommand": "version",
-  "npmVersion": "1.1.1",
+  "npmVersion": "3.3.3",
   "npxCache": "{CWD}/cache/_npx",
   "offline": false,
   "omit": Array [],

test/fixtures/mock-npm.js

@@ -47,19 +47,28 @@ const setGlobalNodeModules = (globalDir) => {
   return globalDir
 }
 
-const getMockNpm = async (t, { mocks, init, load, npm: npmOpts }) => {
-  const mock = {
-    ...mockLogs(mocks),
-    outputs: [],
-    outputErrors: [],
-    joinedOutput: () => mock.outputs.map(o => o.join(' ')).join('\n'),
-  }
-
-  const Npm = tmock(t, '{LIB}/npm.js', {
+const buildMocks = (t, mocks) => {
+  const allMocks = {
     '{LIB}/utils/update-notifier.js': async () => {},
     ...mocks,
-    ...mock.logMocks,
-  })
+  }
+  // The definitions must be mocked since they are a singleton that reads from
+  // process and environs to build defaults in order to break the requiure
+  // cache. We also need to mock them with any mocks that were passed in for the
+  // test in case those mocks are for things like ci-info which is used there.
+  const definitions = '@npmcli/config/lib/definitions'
+  allMocks[definitions] = tmock(t, definitions, allMocks)
+
+  return allMocks
+}
+
+const getMockNpm = async (t, { mocks, init, load, npm: npmOpts }) => {
+  const { logMocks, logs, display } = mockLogs(mocks)
+  const allMocks = buildMocks(t, { ...mocks, ...logMocks })
+  const Npm = tmock(t, '{LIB}/npm.js', allMocks)
+
+  const outputs = []
+  const outputErrors = []
 
   class MockNpm extends Npm {
     async exec (...args) {
@@ -86,23 +95,29 @@ const getMockNpm = async (t, { mocks, init, load, npm: npmOpts }) => {
     }
 
     output (...args) {
-      mock.outputs.push(args)
+      outputs.push(args)
     }
 
     outputError (...args) {
-      mock.outputErrors.push(args)
+      outputErrors.push(args)
     }
   }
 
-  mock.Npm = MockNpm
-  if (init) {
-    mock.npm = new MockNpm(npmOpts)
-    if (load) {
-      await mock.npm.load()
-    }
+  const npm = init ? new MockNpm(npmOpts) : null
+  if (npm && load) {
+    await npm.load()
   }
 
-  return mock
+  return {
+    Npm: MockNpm,
+    npm,
+    outputs,
+    outputErrors,
+    joinedOutput: () => outputs.map(o => o.join(' ')).join('\n'),
+    logMocks,
+    logs,
+    display,
+  }
 }
 
 const mockNpms = new Map()
@@ -127,6 +142,7 @@ const setupMockNpm = async (t, {
   globals = {},
   npm: npmOpts = {},
   argv: rawArgv = [],
+  ...r
 } = {}) => {
   // easy to accidentally forget to pass in tap
   if (!(t instanceof tap.Test)) {

test/fixtures/sandbox.js

@@ -81,7 +81,7 @@ class Sandbox extends EventEmitter {
       get: this[_get].bind(this),
       set: this[_set].bind(this),
     })
-    this[_proxy].env = {}
+    this[_proxy].env = { ...options.env }
     this[_proxy].argv = []
 
     test.cleanSnapshot = this.cleanSnapshot.bind(this)
@@ -262,7 +262,9 @@ class Sandbox extends EventEmitter {
 
     const mockedLogs = mockLogs(this[_mocks])
     this[_logs] = mockedLogs.logs
+    const definitions = this[_test].mock('@npmcli/config/lib/definitions')
     const Npm = this[_test].mock('../../lib/npm.js', {
+      '@npmcli/config/lib/definitions': definitions,
       '../../lib/utils/update-notifier.js': async () => {},
       ...this[_mocks],
       ...mockedLogs.logMocks,
@@ -313,7 +315,9 @@ class Sandbox extends EventEmitter {
 
     const mockedLogs = mockLogs(this[_mocks])
     this[_logs] = mockedLogs.logs
+    const definitions = this[_test].mock('@npmcli/config/lib/definitions')
     const Npm = this[_test].mock('../../lib/npm.js', {
+      '@npmcli/config/lib/definitions': definitions,
       '../../lib/utils/update-notifier.js': async () => {},
       ...this[_mocks],
       ...mockedLogs.logMocks,

test/lib/cli-entry.js

@@ -135,11 +135,6 @@ t.test('load error calls error handler', async t => {
   const err = new Error('test load error')
   const { cli, exitHandlerCalled } = await cliMock(t, {
     mocks: {
-      '{LIB}/utils/config/index.js': {
-        definitions: null,
-        flatten: null,
-        shorthands: null,
-      },
       '@npmcli/config': class BadConfig {
         async load () {
           throw err