Skip to content

Commit

Permalink
fix: move to private attributes where possible
Browse files Browse the repository at this point in the history
_loadError and _valid still have some magic to them
  • Loading branch information
wraithgar committed Apr 12, 2023
1 parent b8006ad commit 667cff5
Showing 1 changed file with 57 additions and 64 deletions.
121 changes: 57 additions & 64 deletions workspaces/config/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,12 @@ const confTypes = new Set([
'cli',
])

const _loaded = Symbol('loaded')
const _get = Symbol('get')
const _find = Symbol('find')
const _loadObject = Symbol('loadObject')
const _loadFile = Symbol('loadFile')
const _checkDeprecated = Symbol('checkDeprecated')
const _flatten = Symbol('flatten')
const _flatOptions = Symbol('flatOptions')

class Config {
#loaded = false
#flatten
// populated the first time we flatten the object
#flatOptions = null

static get typeDefs () {
return typeDefs
}
Expand Down Expand Up @@ -113,9 +109,7 @@ class Config {
}
}

// populated the first time we flatten the object
this[_flatOptions] = null
this[_flatten] = flatten
this.#flatten = flatten
this.types = types
this.shorthands = shorthands
this.defaults = defaults
Expand Down Expand Up @@ -159,26 +153,27 @@ class Config {
}
Object.freeze(this.list)

this[_loaded] = false
this.#loaded = false
}

get loaded () {
return this[_loaded]
return this.#loaded
}

get prefix () {
return this[_get]('global') ? this.globalPrefix : this.localPrefix
return this.#get('global') ? this.globalPrefix : this.localPrefix
}

// return the location where key is found.
find (key) {
if (!this.loaded) {
throw new Error('call config.load() before reading values')
}
return this[_find](key)
// TODO single use?
return this.#find(key)
}

[_find] (key) {
#find (key) {
// have to look in reverse order
const entries = [...this.data.entries()]
for (let i = entries.length - 1; i > -1; i--) {
Expand All @@ -194,12 +189,12 @@ class Config {
if (!this.loaded) {
throw new Error('call config.load() before reading values')
}
return this[_get](key, where)
return this.#get(key, where)
}

// we need to get values sometimes, so use this internal one to do so
// while in the process of loading.
[_get] (key, where = null) {
#get (key, where = null) {
if (where !== null && !confTypes.has(where)) {
throw new Error('invalid config location param: ' + where)
}
Expand All @@ -214,32 +209,32 @@ class Config {
if (!confTypes.has(where)) {
throw new Error('invalid config location param: ' + where)
}
this[_checkDeprecated](key)
this.#checkDeprecated(key)
const { data } = this.data.get(where)
data[key] = val

// this is now dirty, the next call to this.valid will have to check it
this.data.get(where)[_valid] = null

// the flat options are invalidated, regenerate next time they're needed
this[_flatOptions] = null
this.#flatOptions = null
}

get flat () {
if (this[_flatOptions]) {
return this[_flatOptions]
if (this.#flatOptions) {
return this.#flatOptions
}

// create the object for flat options passed to deps
process.emit('time', 'config:load:flatten')
this[_flatOptions] = {}
this.#flatOptions = {}
// walk from least priority to highest
for (const { data } of this.data.values()) {
this[_flatten](data, this[_flatOptions])
this.#flatten(data, this.#flatOptions)
}
process.emit('timeEnd', 'config:load:flatten')

return this[_flatOptions]
return this.#flatOptions
}

delete (key, where = 'cli') {
Expand Down Expand Up @@ -290,8 +285,8 @@ class Config {
process.emit('timeEnd', 'config:load:global')

// set this before calling setEnvs, so that we don't have to share
// symbols, as that module also does a bunch of get operations
this[_loaded] = true
// private attributes, as that module also does a bunch of get operations
this.#loaded = true

// set proper globalPrefix now that everything is loaded
this.globalPrefix = this.get('prefix')
Expand All @@ -307,7 +302,7 @@ class Config {
this.loadGlobalPrefix()
this.loadHome()

this[_loadObject]({
this.#loadObject({
...this.defaults,
prefix: this.globalPrefix,
}, 'default', 'default values')
Expand All @@ -316,13 +311,13 @@ class Config {

// the metrics-registry defaults to the current resolved value of
// the registry, unless overridden somewhere else.
settableGetter(data, 'metrics-registry', () => this[_get]('registry'))
settableGetter(data, 'metrics-registry', () => this.#get('registry'))

// if the prefix is set on cli, env, or userconfig, then we need to
// default the globalconfig file to that location, instead of the default
// global prefix. It's weird that `npm get globalconfig --prefix=/foo`
// returns `/foo/etc/npmrc`, but better to not change it at this point.
settableGetter(data, 'globalconfig', () => resolve(this[_get]('prefix'), 'etc/npmrc'))
settableGetter(data, 'globalconfig', () => resolve(this.#get('prefix'), 'etc/npmrc'))
}

loadHome () {
Expand Down Expand Up @@ -363,7 +358,7 @@ class Config {
}
conf[key] = envVal
}
this[_loadObject](conf, 'env', 'environment')
this.#loadObject(conf, 'env', 'environment')
}

loadCLI () {
Expand All @@ -373,7 +368,7 @@ class Config {
nopt.invalidHandler = null
this.parsedArgv = conf.argv
delete conf.argv
this[_loadObject](conf, 'cli', 'command line options')
this.#loadObject(conf, 'cli', 'command line options')
}

get valid () {
Expand Down Expand Up @@ -541,7 +536,7 @@ class Config {
log.warn('invalid config', msg, desc)
}

[_loadObject] (obj, where, source, er = null) {
#loadObject (obj, where, source, er = null) {
const conf = this.data.get(where)
if (conf.source) {
const m = `double-loading "${where}" configs from ${source}, ` +
Expand All @@ -568,14 +563,14 @@ class Config {
const k = envReplace(key, this.env)
const v = this.parseField(value, k)
if (where !== 'default') {
this[_checkDeprecated](k, where, obj, [key, value])
this.#checkDeprecated(k, where, obj, [key, value])
}
conf.data[k] = v
}
}
}

[_checkDeprecated] (key, where, obj, kv) {
#checkDeprecated (key, where, obj, kv) {
// XXX(npm9+) make this throw an error
if (this.deprecated[key]) {
log.warn('config', key, this.deprecated[key])
Expand All @@ -587,18 +582,18 @@ class Config {
return parseField(f, key, this, listElement)
}

async [_loadFile] (file, type) {
async #loadFile (file, type) {
process.emit('time', 'config:load:file:' + file)
// only catch the error from readFile, not from the loadObject call
await readFile(file, 'utf8').then(
data => this[_loadObject](ini.parse(data), type, file),
er => this[_loadObject](null, type, file, er)
data => this.#loadObject(ini.parse(data), type, file),
er => this.#loadObject(null, type, file, er)
)
process.emit('timeEnd', 'config:load:file:' + file)
}

loadBuiltinConfig () {
return this[_loadFile](resolve(this.npmPath, 'npmrc'), 'builtin')
return this.#loadFile(resolve(this.npmPath, 'npmrc'), 'builtin')
}

async loadProjectConfig () {
Expand All @@ -613,7 +608,7 @@ class Config {
this.localPackage = await fileExists(this.localPrefix, 'package.json')
}

if (this[_get]('global') === true || this[_get]('location') === 'global') {
if (this.#get('global') === true || this.#get('location') === 'global') {
this.data.get('project').source = '(global mode enabled, ignored)'
this.sources.set(this.data.get('project').source, 'project')
return
Expand All @@ -625,23 +620,23 @@ class Config {
// up loading the "project" config where the "userconfig" will be,
// which causes some calamaties. So, we only load project config if
// it doesn't match what the userconfig will be.
if (projectFile !== this[_get]('userconfig')) {
return this[_loadFile](projectFile, 'project')
if (projectFile !== this.#get('userconfig')) {
return this.#loadFile(projectFile, 'project')
} else {
this.data.get('project').source = '(same as "user" config, ignored)'
this.sources.set(this.data.get('project').source, 'project')
}
}

async loadLocalPrefix () {
const cliPrefix = this[_get]('prefix', 'cli')
const cliPrefix = this.#get('prefix', 'cli')
if (cliPrefix) {
this.localPrefix = cliPrefix
return
}

const cliWorkspaces = this[_get]('workspaces', 'cli')
const isGlobal = this[_get]('global') || this[_get]('location') === 'global'
const cliWorkspaces = this.#get('workspaces', 'cli')
const isGlobal = this.#get('global') || this.#get('location') === 'global'

for (const p of walkUp(this.cwd)) {
// HACK: this is an option set in tests to stop the local prefix from being set
Expand Down Expand Up @@ -701,11 +696,11 @@ class Config {
}

loadUserConfig () {
return this[_loadFile](this[_get]('userconfig'), 'user')
return this.#loadFile(this.#get('userconfig'), 'user')
}

loadGlobalConfig () {
return this[_loadFile](this[_get]('globalconfig'), 'global')
return this.#loadFile(this.#get('globalconfig'), 'global')
}

async save (where) {
Expand All @@ -717,7 +712,6 @@ class Config {
}

const conf = this.data.get(where)
conf[_raw] = { ...conf.data }
conf[_loadError] = null

if (where === 'user') {
Expand Down Expand Up @@ -870,41 +864,40 @@ class Config {
}
}

const _data = Symbol('data')
const _raw = Symbol('raw')
const _loadError = Symbol('loadError')
const _source = Symbol('source')
const _valid = Symbol('valid')

class ConfigData {
#data
#source = null
#raw = null
constructor (parent) {
this[_data] = Object.create(parent && parent.data)
this[_source] = null
this[_loadError] = null
this[_raw] = null
this.#data = Object.create(parent && parent.data)
this.#raw = null
this[_valid] = true
}

get data () {
return this[_data]
return this.#data
}

get valid () {
return this[_valid]
}

set source (s) {
if (this[_source]) {
if (this.#source) {
throw new Error('cannot set ConfigData source more than once')
}
this[_source] = s
this.#source = s
}

get source () {
return this[_source]
return this.#source
}

set loadError (e) {
if (this[_loadError] || this[_raw]) {
if (this[_loadError] || this.#raw) {
throw new Error('cannot set ConfigData loadError after load')
}
this[_loadError] = e
Expand All @@ -915,14 +908,14 @@ class ConfigData {
}

set raw (r) {
if (this[_raw] || this[_loadError]) {
if (this.#raw || this[_loadError]) {
throw new Error('cannot set ConfigData raw after load')
}
this[_raw] = r
this.#raw = r
}

get raw () {
return this[_raw]
return this.#raw
}
}

Expand Down

0 comments on commit 667cff5

Please sign in to comment.