fix(perf): do not list network interfaces when loading config definitions

bin/npx-cli.js

@@ -26,6 +26,12 @@ const removed = new Set([
 
 const { definitions, shorthands } = require('@npmcli/config/lib/definitions')
 const npmSwitches = Object.entries(definitions)
+  // a type computed on first use (local-address) is a list of values, never a
+  // switch: skip it rather than compute it
+  .filter(([, def]) => {
+    const desc = Object.getOwnPropertyDescriptor(def, 'type')
+    return !(desc && desc.get)
+  })
   .filter(([, { type }]) => type === Boolean ||
     (Array.isArray(type) && type.includes(Boolean)))
   .map(([key]) => key)

test/bin/npx-cli.js

@@ -23,6 +23,32 @@ t.test('npx foo -> npm exec -- foo', async t => {
   t.strictSame(argv, ['node', npm, 'exec', '--', 'foo'])
 })
 
+t.test('npx does not compute lazily typed definitions', async t => {
+  let reads = 0
+  const realDefs = require('@npmcli/config/lib/definitions')
+  mockGlobals(t, {
+    'process.argv': ['node', npx, '--lazy', 'x', '--yes', 'foo'],
+  })
+  tmock(t, '{BIN}/npx-cli.js', {
+    '{LIB}/cli.js': () => {},
+    '@npmcli/config/lib/definitions': {
+      ...realDefs,
+      definitions: {
+        ...realDefs.definitions,
+        lazy: Object.defineProperty({ key: 'lazy' }, 'type', {
+          enumerable: true,
+          get: () => {
+            reads++
+            return [null, 'x']
+          },
+        }),
+      },
+    },
+  })
+  t.strictSame(process.argv, ['node', npm, 'exec', '--lazy', 'x', '--yes', '--', 'foo'])
+  t.equal(reads, 0, 'the lazy type is not read')
+})
+
 t.test('npx -- foo -> npm exec -- foo', async t => {
   const { argv } = mockNpx(t, ['node', npx, '--', 'foo'])
   t.strictSame(argv, ['node', npm, 'exec', '--', 'foo'])

workspaces/config/lib/definitions/definition.js

@@ -38,7 +38,8 @@ class Definition {
     this.key = key
     // if it's set falsey, don't export it; otherwise, we do by default
     this.envExport = true
-    Object.assign(this, def)
+    // copy accessors as accessors, so a definition can compute its type lazily
+    Object.defineProperties(this, Object.getOwnPropertyDescriptors(def))
     this.validate()
     if (!this.defaultDescription) {
       this.defaultDescription = describeValue(this.default)

workspaces/config/lib/definitions/definitions.js

@@ -63,14 +63,22 @@ const shell = isWindows ? process.env.ComSpec || 'cmd'
   : process.env.SHELL || 'sh'
 
 const { networkInterfaces } = require('node:os')
+// Listing the network interfaces can take several milliseconds, and the list
+// is only needed to validate a local-address value. So `type` below is a
+// getter, and the list is built on first use.
+let localAddresses
 const getLocalAddresses = () => {
+  if (localAddresses) {
+    return localAddresses
+  }
   try {
-    return Object.values(networkInterfaces()).map(
+    localAddresses = Object.values(networkInterfaces()).map(
       int => int.map(({ address }) => address)
     ).reduce((set, addrs) => set.concat(addrs), [null])
   } catch (e) {
-    return [null]
+    localAddresses = [null]
   }
+  return localAddresses
 }
 
 const unicode = /UTF-?8$/i.test(
@@ -1385,8 +1393,13 @@ const definitions = {
   }),
   'local-address': new Definition('local-address', {
     default: null,
-    type: getLocalAddresses(),
+    get type () {
+      return getLocalAddresses()
+    },
     typeDescription: 'IP Address',
+    // given here so that Definition does not derive them from `type`
+    hint: '<local-address>',
+    usage: '--local-address <local-address>',
     description: `
       The IP address of the local interface to use when making connections to
       the npm registry.  Must be IPv4 in versions of Node prior to 0.12.

workspaces/config/lib/index.js

@@ -1105,7 +1105,17 @@ const getTypesFromDefinitions = (definitions) => {
 
   for (const [key, def] of Object.entries(definitions)) {
     defaults[key] = def.default
-    types[key] = def.type
+    if (Object.getOwnPropertyDescriptor(def, 'type')?.get) {
+      // a lazily computed type stays lazy: nopt only reads the types of the
+      // keys it is given
+      Object.defineProperty(types, key, {
+        enumerable: true,
+        configurable: true,
+        get: () => def.type,
+      })
+    } else {
+      types[key] = def.type
+    }
     if (def.deprecated) {
       deprecated[key] = def.deprecated.trim().replace(/\n +/, '\n')
     }

workspaces/config/test/definitions/definitions.js

@@ -132,6 +132,25 @@ t.test('local-address allowed types', t => {
     t.same(defs['local-address'].type, [null])
     t.end()
   })
+  t.test('only listed when the type is read, and only once', t => {
+    let calls = 0
+    const os = {
+      tmpdir: () => '/tmp',
+      networkInterfaces: () => {
+        calls++
+        return { eth0: [{ address: '10.0.0.1' }] }
+      },
+    }
+    const defs = mockDefs({ 'node:os': os })
+    t.equal(calls, 0, 'not listed when the definitions load')
+    t.equal(defs['local-address'].hint, '<local-address>')
+    t.equal(defs['local-address'].usage, '--local-address <local-address>')
+    t.equal(calls, 0, 'hint and usage do not read the type')
+    t.same(defs['local-address'].type, [null, '10.0.0.1'])
+    t.same(defs['local-address'].type, [null, '10.0.0.1'])
+    t.equal(calls, 1, 'listed once')
+    t.end()
+  })
   t.end()
 })
 

workspaces/config/test/index.js

@@ -2468,3 +2468,51 @@ t.test('global-ignore-file follows an explicit override', async t => {
     'cli override wins over computed default'
   )
 })
+
+t.test('a lazily computed type is only read for keys that are set', async t => {
+  let reads = 0
+  const lazyDefs = {
+    // not createDef: spreading the object would read the getter
+    lazy: new Definition('lazy', {
+      key: 'lazy',
+      default: null,
+      get type () {
+        reads++
+        return [null, 'a', 'b']
+      },
+      typeDescription: 'a or b',
+      hint: '<lazy>',
+      usage: '--lazy <lazy>',
+      description: 'computed on first use',
+    }),
+    ...createDef('plain', { default: 1, type: Number, description: 'eager' }),
+  }
+
+  const { types } = Config.getTypesFromDefinitions(lazyDefs)
+  t.equal(reads, 0, 'building the types does not read it')
+  t.equal(types.plain, Number)
+  t.same(Object.keys(types), ['lazy', 'plain'])
+  t.same(types.lazy, [null, 'a', 'b'])
+  t.equal(reads, 1)
+
+  const path = t.testdir()
+  const load = async (argv) => {
+    reads = 0
+    const config = new Config({
+      npmPath: `${path}/npm`,
+      env: {},
+      argv: [process.execPath, __filename, ...argv],
+      cwd: path,
+      definitions: lazyDefs,
+      shorthands: {},
+      flatten: () => {},
+    })
+    await config.load()
+    return config
+  }
+  await load(['--plain', '2'])
+  t.equal(reads, 0, 'not read when the key is not set')
+  const config = await load(['--lazy', 'b'])
+  t.ok(reads > 0, 'read to validate a value')
+  t.equal(config.get('lazy'), 'b')
+})