From ac7267c8e32d56f6a679f0177b12a9747eb666cc Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Tue, 6 Oct 2026 14:43:45 -0700 Subject: [PATCH] fix(perf): do not list network interfaces when loading config definitions The `local-address` definition built its list of allowed values with `os.networkInterfaces()` when the definitions module loaded, so every npm command enumerated the machine's network interfaces at start-up. The list is only needed to validate a `local-address` value. - `local-address`'s `type` is now a memoized getter, with `hint` and `usage` given so that `Definition` does not read it to derive them - `Definition` copies property descriptors, so a getter stays a getter - `getTypesFromDefinitions` keeps an accessor type as an accessor; nopt only reads the types of keys that are set - `bin/npx-cli.js` skips accessor types when it collects boolean switches, so npx does not compute the list either Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- bin/npx-cli.js | 6 +++ test/bin/npx-cli.js | 26 ++++++++++ .../config/lib/definitions/definition.js | 3 +- .../config/lib/definitions/definitions.js | 19 ++++++-- workspaces/config/lib/index.js | 12 ++++- .../config/test/definitions/definitions.js | 19 ++++++++ workspaces/config/test/index.js | 48 +++++++++++++++++++ 7 files changed, 128 insertions(+), 5 deletions(-) diff --git a/bin/npx-cli.js b/bin/npx-cli.js index e2e1b87906abe..21e389335444d 100755 --- a/bin/npx-cli.js +++ b/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) diff --git a/test/bin/npx-cli.js b/test/bin/npx-cli.js index 67a8d3319fc18..22a4eebf8bc0a 100644 --- a/test/bin/npx-cli.js +++ b/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']) diff --git a/workspaces/config/lib/definitions/definition.js b/workspaces/config/lib/definitions/definition.js index c151506b24476..c91503efcd14c 100644 --- a/workspaces/config/lib/definitions/definition.js +++ b/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) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index bd5c82363937c..f29c8e639e8a0 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/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: '', + usage: '--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. diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index 37dfecf800df6..e9d1b8e3dc718 100644 --- a/workspaces/config/lib/index.js +++ b/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') } diff --git a/workspaces/config/test/definitions/definitions.js b/workspaces/config/test/definitions/definitions.js index 3f89aef92dc19..1a5f9b845263c 100644 --- a/workspaces/config/test/definitions/definitions.js +++ b/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, '') + t.equal(defs['local-address'].usage, '--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() }) diff --git a/workspaces/config/test/index.js b/workspaces/config/test/index.js index 9f83d988ee0a2..d8e8313ad3ea6 100644 --- a/workspaces/config/test/index.js +++ b/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: '', + usage: '--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') +})