Skip to content

Commit

Permalink
fix: remove granular config timers
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukekarrys
lukekarrys committed Apr 24, 2024
1 parent 28019d5 commit 03634be
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 37 deletions.
26 changes: 11 additions & 15 deletions workspaces/config/lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,27 +233,24 @@ class Config {
throw new Error('attempting to load npm config multiple times')
}

const timeEnd = time.start('config:load')

// first load the defaults, which sets the global prefix
time.start('config:load:defaults', () => this.loadDefaults())
this.loadDefaults()

// next load the builtin config, as this sets new effective defaults
await time.start('config:load:builtin', () => this.loadBuiltinConfig())
await this.loadBuiltinConfig()

// cli and env are not async, and can set the prefix, relevant to project
time.start('config:load:cli', () => this.loadCLI())

time.start('config:load:env', () => this.loadEnv())
this.loadCLI()
this.loadEnv()

// next project config, which can affect userconfig location
await time.start('config:load:project', () => this.loadProjectConfig())
await this.loadProjectConfig()

// then user config, which can affect globalconfig location
await time.start('config:load:user', () => this.loadUserConfig())
await this.loadUserConfig()

// last but not least, global config file
await time.start('config:load:global', () => this.loadGlobalConfig())
await this.loadGlobalConfig()

// set this before calling setEnvs, so that we don't have to share
// private attributes, as that module also does a bunch of get operations
Expand All @@ -262,9 +259,7 @@ class Config {
// set proper globalPrefix now that everything is loaded
this.globalPrefix = this.get('prefix')

time.start('config:load:setEnvs', () => this.setEnvs())

timeEnd()
this.setEnvs()
}

loadDefaults () {
Expand Down Expand Up @@ -590,7 +585,8 @@ class Config {

async #loadFile (file, type) {
// only catch the error from readFile, not from the loadObject call
await time.start(`config:load:file:${file}`, () => readFile(file, 'utf8').then(
log.silly(`config:load:file:${file}`)
await readFile(file, 'utf8').then(
data => {
const parsedConfig = ini.parse(data)
if (type === 'project' && parsedConfig.prefix) {
Expand All @@ -601,7 +597,7 @@ class Config {
return this.#loadObject(parsedConfig, type, file)
},
er => this.#loadObject(null, type, file, er)
))
)
}

loadBuiltinConfig () {
Expand Down
54 changes: 32 additions & 22 deletions workspaces/config/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,10 @@ loglevel = yolo
})
logs.length = 0
await config.load()
t.match(logs, [['verbose', 'config', 'error loading user config', {
message: 'weird error',
}]])
t.match(logs.find(l => l[0] === 'verbose'),
['verbose', 'config', 'error loading user config', {
message: 'weird error',
}])
logs.length = 0
})

Expand Down Expand Up @@ -375,7 +376,7 @@ loglevel = yolo

// warn logs are emitted as a side effect of validate
config.validate()
t.strictSame(logs, [
t.strictSame(logs.filter(l => l[0] === 'warn'), [
['warn', 'invalid config', 'registry="hello"', 'set in command line options'],
['warn', 'invalid config', 'Must be', 'full url with "http://"'],
['warn', 'invalid config', 'proxy="hello"', 'set in command line options'],
Expand All @@ -392,8 +393,7 @@ loglevel = yolo
['warn', 'invalid config', 'prefix=true', 'set in command line options'],
['warn', 'invalid config', 'Must be', 'valid filesystem path'],
['warn', 'config', 'also', 'Please use --include=dev instead.'],
['warn', 'invalid config', 'loglevel="yolo"',
`set in ${resolve(path, 'project/.npmrc')}`],
['warn', 'invalid config', 'loglevel="yolo"', `set in ${resolve(path, 'project/.npmrc')}`],
['warn', 'invalid config', 'Must be one of:',
['silent', 'error', 'warn', 'notice', 'http', 'info', 'verbose', 'silly'].join(', '),
],
Expand Down Expand Up @@ -570,7 +570,7 @@ loglevel = yolo
all: config.get('all'),
})

t.strictSame(logs, [
t.strictSame(logs.filter(l => l[0] === 'warn'), [
['warn', 'invalid config', 'registry="hello"', 'set in command line options'],
['warn', 'invalid config', 'Must be', 'full url with "http://"'],
['warn', 'invalid config', 'proxy="hello"', 'set in command line options'],
Expand Down Expand Up @@ -1183,8 +1183,9 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, path, 'localPrefix is the root')
t.same(config.get('workspace'), [join(path, 'workspaces', 'one')], 'set the workspace')
t.equal(logs.length, 1, 'got one log message')
t.match(logs[0], ['info', /^found workspace root at/], 'logged info about workspace root')
const info = logs.filter(l => l[0] === 'info')
t.equal(info.length, 1, 'got one log message')
t.match(info[0], ['info', /^found workspace root at/], 'logged info about workspace root')
})

t.test('finds other workspace parent', async (t) => {
Expand All @@ -1204,8 +1205,9 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, path, 'localPrefix is the root')
t.same(config.get('workspace'), ['../two'], 'kept the specified workspace')
t.equal(logs.length, 1, 'got one log message')
t.match(logs[0], ['info', /^found workspace root at/], 'logged info about workspace root')
const info = logs.filter(l => l[0] === 'info')
t.equal(info.length, 1, 'got one log message')
t.match(info[0], ['info', /^found workspace root at/], 'logged info about workspace root')
})

t.test('warns when workspace has .npmrc', async (t) => {
Expand All @@ -1225,9 +1227,10 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, path, 'localPrefix is the root')
t.same(config.get('workspace'), [join(path, 'workspaces', 'three')], 'kept the workspace')
t.equal(logs.length, 2, 'got two log messages')
t.match(logs[0], ['warn', /^ignoring workspace config/], 'warned about ignored config')
t.match(logs[1], ['info', /^found workspace root at/], 'logged info about workspace root')
const filtered = logs.filter(l => l[0] === 'info' || l[0] === 'warn')
t.equal(filtered.length, 2, 'got two log messages')
t.match(filtered[0], ['warn', /^ignoring workspace config/], 'warned about ignored config')
t.match(filtered[1], ['info', /^found workspace root at/], 'logged info about workspace root')
})

t.test('prefix skips auto detect', async (t) => {
Expand All @@ -1247,7 +1250,8 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, join(path, 'workspaces', 'one'), 'localPrefix is the root')
t.same(config.get('workspace'), [], 'did not set workspace')
t.equal(logs.length, 0, 'got no log messages')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 0, 'got no log messages')
})

t.test('no-workspaces skips auto detect', async (t) => {
Expand All @@ -1267,7 +1271,8 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, join(path, 'workspaces', 'one'), 'localPrefix is the root')
t.same(config.get('workspace'), [], 'did not set workspace')
t.equal(logs.length, 0, 'got no log messages')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 0, 'got no log messages')
})

t.test('global skips auto detect', async (t) => {
Expand All @@ -1287,7 +1292,8 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, join(path, 'workspaces', 'one'), 'localPrefix is the root')
t.same(config.get('workspace'), [], 'did not set workspace')
t.equal(logs.length, 0, 'got no log messages')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 0, 'got no log messages')
})

t.test('location=global skips auto detect', async (t) => {
Expand All @@ -1307,7 +1313,8 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, join(path, 'workspaces', 'one'), 'localPrefix is the root')
t.same(config.get('workspace'), [], 'did not set workspace')
t.equal(logs.length, 0, 'got no log messages')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 0, 'got no log messages')
})

t.test('excludeNpmCwd skips auto detect', async (t) => {
Expand All @@ -1328,7 +1335,8 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, join(path, 'workspaces', 'one'), 'localPrefix is the root')
t.same(config.get('workspace'), [], 'did not set workspace')
t.equal(logs.length, 0, 'got no log messages')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 0, 'got no log messages')
})

t.test('does not error for invalid package.json', async (t) => {
Expand All @@ -1354,8 +1362,9 @@ t.test('workspaces', async (t) => {
await config.load()
t.equal(config.localPrefix, path, 'localPrefix is the root')
t.same(config.get('workspace'), [join(path, 'workspaces', 'one')], 'set the workspace')
t.equal(logs.length, 1, 'got one log message')
t.match(logs[0], ['info', /^found workspace root at/], 'logged info about workspace root')
const filtered = logs.filter(l => l[0] !== 'silly')
t.equal(filtered.length, 1, 'got one log message')
t.match(filtered[0], ['info', /^found workspace root at/], 'logged info about workspace root')
})
})

Expand Down Expand Up @@ -1474,7 +1483,8 @@ t.test('catch project config prefix error', async t => {
logs.length = 0
// config.load() triggers the error to be logged
await config.load()
t.match(logs, [[
const filtered = logs.filter(l => l[0] !== 'silly')
t.match(filtered, [[
'error', 'config', `prefix cannot be changed from project config: ${path}`,
]], 'Expected error logged')
})

0 comments on commit 03634be

Please sign in to comment.