Proposal: Improve library's architecture (a.k.a. NextGen)

[iMoses]

TL;DR This proposal changes the API but keeps most of the original hueristics completely intact. Tests were added and adopted, but with the exception of deprecated methods (e.g. util methods), no tests were removed and all previously defined tests still pass.

You can check the code here: https://github.com/iMoses/node-config/tree/next-gen

Disclaimer: some breaking changes can be reverted while others cannot. If you feel strongly about some of the changes let me know and let's discuss the available possibilities.

Motivation

About half of today's open issues are related to immutability inconsistencies and sub-modules support. while many closed issues are related to non-intuitive methods of initializing the library and means of adding support for this and that, which can come down to more control over the library's internal mechanisms.

While looking for possible solutions I came to realize that the main issue with sub-modules is our lack of support for multiple instances, and the main issue with that is our reliance on environment variables for initialize the config instance.

The second major issue is that we initialize the main instance ourselves when loaded, which prevents us from introducing new features to manipulate the library's options before initialization.

These problems can only be solved by changing the initialization logic of the library, which wouldn't have been much of a problem if we weren't expected to return an instance already initialized and containing the resulted configuration files on top of the instance's root... (backwards compatibility) Which brings us back to the immutability issues that are complex due to the same logical problems.

If we change the Config instance API and remove direct access to the configuration object (at least from the root) we can enforce mutability and delay files loading until the first access to the configuration object, instead of the config instance as we do today.

It all comes down to that:

• Allow the creation of multiple instances
• Remove reliance on environmental variables
• Better enforce immutability
• Lazy-load configuration

Along with changes to the parser to support validators and middleware we can cover most of today's open issues in one fine swipe, which will require a major version with many breaking changes effecting only advance users.

I think it's worth it :)

Breaking changes

• Removed config.util methods
• Removed runtime.json support
• Removed custom-environment-variables.* support
• Changed location of asyncConfig and deferConfig
• asyncConfig now uses config.whenReady to resolve
• get() and has() are available only on the config instance - removes all reserved words
• Changed Parser API to a programmatic solution, as opposed to an external file
• getArgv() slightly differs from its predecessor - in case of multiple matching arguments it returns the last (previous logic was to return the first occurrence)
• Removed raw() as it is now obsolete (complex objects are not extended by default)

[x] Removed config.util methods

We have many utilities which are exposed as part of our API and I claim that most of these shouldn't be exposed at all. This isn't the library's purpose, no one installs node-config so that they can use config.util.extendDeep, and exposing them as part of our API is forcing us to keep support for them or else we introduce a breaking change, which makes it a fragile API that is forced to carry legacy code.

My proposal removes config.util completely.

Internal utilities are placed at config/lib/utils with a disclaimer that we will not guarantee stability between versions. Users can decide to use them at their own risk. Anything worthwhile should be placed on top of the config instance, while we keep utils strictly internal.

[x] Removed runtime.json support

Support was removed in an effort to dispose of deprecated code and inconsistent heuristics, and can be simply restored by calling config.parseFile manually.

config.parseFile(process.env.NODE_CONFIG_DIR + '/runtime.json');

Removed custom-environment-variables.* support

Support was removed in an effort to dispose of inconsistent heuristics.

The new architecture adds middleware support to the parser which can be used to apply templates. A template engine can be used to apply the same logic in a consistent manner, unlike custom-environment-variables.*, and replace it with a cross-extension ability to set values from environment variables and much more.

See Parser.lib.middleware.configTemplate() for examples.

Changed location of asyncConfig and deferConfig

Previously these methods were in separate files outside of the /lib directory to allow easy access from configuration files without trigger the initialization of the config instance. With the new architecture this isn't necessary anymore and we can simply export these methods as part of out instance.

Previous version:

const { asyncConfig } = require('config/async');
const { deferConfig } = require('config/defer');

New version:

const { asyncConfig, deferConfig } = require('config');

I added shim files for now with a deprecation warning.

asyncConfig now uses config.whenReady to resolve

Previous version:

const config = require('config');
const { resolveAsyncConfigs } = require('config/async');
resolveAsyncConfigs(config)
  .then(() => require('./main'));

New version:

const config = require('config');
config.whenReady.then(() => require('./main'));

get() and has() are available only on the config instance

The main reason for this is to remove reserved words completely. I don't think the library should reserve any keys.

Previous version:

const config = require('config');
const db = config.get('services.db');
console.log(db.has('host'));  // true

New version:

const config = require('config');
const db = config.get('services.db');
console.log(db.has('host'));  // Error: method has doesn't exist

Changed Parser API to a programmatic solution

Explained in the Parser API reference.

It changes quite a lot, but I figured since it's a very new feature users who already use it are early adopters and won't mind as much, since it's a much cleaner API with validators and middleware :)

getArgv() slightly differs from its predecessor

In case of multiple matching arguments it returns the last (previous logic was to return the first occurrence). It actually seemed like a bug-fix to me, since that's what I would've expected to be the correct behavior. I also added support for boolean and space separated cli-args.

// in case of `node index.js --NODE_ENV=dev --NODE_ENV=prod`
console.log(utils.getArgv('NODE_ENV'));  // prod

Supported syntax:
    --VAR_NAME=value    value separated by an equality sign
    --VAR_NAME value    value separated by spaces
    --BOOL_VAR          boolean - no value automatically returns true

Config

Config is an internal class that's used to create configuration instances, as it did in previous versions.

Configuration instances are created by three means:

1. Config.create(options)
2. Config.subModule(moduleName)
3. A default instance that is initiated and returned by node-config

The default instance initialization options comes from env-vars and cli-args, using the same heuristics as previous versions.

module.exports = new Config(null, getDefaultInstanceOptions());

function getDefaultInstanceOptions() {
  return clearUndefinedKeys({
    configDir: utils.getOption('NODE_CONFIG_DIR', './config'),
    environment: utils.getOption('NODE_CONFIG_ENV') || utils.getOption('NODE_ENV'),
    hostname: utils.getOption('HOST') || utils.getOption('HOSTNAME'),
    appInstance: utils.getOption('NODE_APP_INSTANCE'),
    strict: Boolean(utils.getOption('NODE_CONFIG_STRICT_MODE')),
    freeze: !utils.getOption('ALLOW_CONFIG_MUTATIONS'),
    legacy: true,
  });
}

Common Usage

Users who've been using the library according to the Common Usage section recommendations won't be affected (in most cases) by these changes.

// autoload defaults on first access
const config = require('config');
console.log(config.get('some.key'));  // value
console.log(config.has('another.key'));  // boolean

You can also access the config property directly if you prefer it over the instance API.

const { config } = require('config');
console.log(config.some.key);  // value

Autoload and Mutation methods

Configuration instances will automatically load with the default options provided to the constructor (see Config.loadDefaults(legacy)), unless a mutation methods has been used in which case the autoload mechanism is disabled.

Mutation methods have access to modify the configuration object without accessing the external property. These are the only methods which are available to mutate the configuration object, and they are automatically locked once the configuration object is frozen.

• Config.loadFiles(options)
• Config.parseFile(filename)
• Config.extend(object, source)

That's important to keep in mind when using one of these methods. A common pitfall is to use a mutation method and forget about it canceling the autoload mechanism.

// overrides default options
// only the content of variables.yaml will be available
const config = require('config')
  .parseFile(__dirname + '/scripts/variables.yaml');

Which can be simply solved by executing the Config.loadDefault(legacy) manually.

// loads defaults before parsing variables.yaml
const config = require('config')
  .loadDefaults(true)
  .parseFile(__dirname + '/scripts/variables.yaml');

asyncConfig and deferConfig

All library files were moved to /lib, and since they can now be loaded directly from the config instance without triggering the autoload sequence it acts as a simpler API.

const { deferConfig, asyncConfig } = require('config');

Config.config

The is the jewel in the crown, the reason for this major change in the library's API.

In previous versions the configuration instance is initialized when the module is loaded and automatically reads options, loads files, run validations and freezes the configuration object, which caused many problems. Moving the configuration object to a separate property allows us to observe access and delay the autoload sequence until it is used.

This property is lazily loaded so until accessed the following will not be activated.

On first access this property triggers the resolve mechanism, composed of these steps:

• Executes loadDefaults(true) only in case none of the mutation methods were used
• Resolves all deferConfig values
• Disable all mutation methods
• Freeze sources and the configuration object

Once resolved the frozen configuration object is set to this property's value.

Note that Config.get() and Config.has() access this property, so it can trigger the resolve mechanism.

const config = require('config');
// only resolves once config.get is used
// until then we can use mutation methods (loadFiles, parseFile, extend)
console.log(config.get('services.db.host'));  // localhost

Config.whenReady

This property is lazily loaded so unless accessed the following will not take effect.

On first access this property triggers the resolve mechanism for both deferConfig and asyncConfig. The property returns a promise which resolves after all asyncConfig were replaced with their final values.

This is a replacement mechanism for resolveAsyncConfigs(config).

Config.options

A frozen copy of the options passed to the instance constructor, mainly for debugging purposes.

Config.parser

Property that's used to access the Parser instance used by the instance.

Config.sources

Returns an array of sources used to compose the configuration object. The array is composed of objects, each with a source string and a data object containing the source's content.

Note that when using sub-modules, any mutations caused by a sub-module will also contain the module property which hold the module name of the effecting sub-module. (see Config.subModule)

This is a replacement mechanism for config.util.getConfigSources().

const config = require('config');
config.parseFile('/absolute/path/to/file.js');
config.loadFiles({configDir: __dirname + '/ultra-config'});
config.extend(JSON.parse(process.env.MY_ENV_VAR), '$MY_ENV_VAR');
config.extend({defaultProp: 'MyValue'});
console.log(config.sources);

Would output:

[
  {source: '/absolute/path/to/file.js', data: {/* ... */}},
  {source: '/absolute/path/to/ultra-config/defaults.json', data: {/* ... */}},
  {source: '/absolute/path/to/ultra-config/local.yml', data: {/* ... */}},
  {source: '$MY_ENV_VAR', data: {/* ... */}},
  {source: 'config.extend()', data: {defaultProp: 'MyValue'}},
]

Config.create(options)

Create's a new configuration instance that's completely independent.

Should be used by package owners who wish to use node-config without effecting dependent packages who may use the library themselves. This method is also useful for testing purposes.

const myConfig = require('config').create({
  configDir: __dirname + '/config',
  appInstance: process.platform,
  environment: 'package',
});

options will be passed down to Config.loadFiles(options) in case of an autoload or a manual execution of Config.loadDefaults(legacy).

• configDir - path of the configuration dir
• appInstance - appInstance used to match files
• environment - environment used to match files, defaults to "development"
• hostname - hostname used to match files, defaults to os.hostname()

Additional options are:

• strict - passed down to strictValidations, replaces NODE_CONFIG_STRICT_MODE
• legacy - passed down to loadDefaults as part of the autoload mechanism
• freeze - if set to false the configuration object will never be locked

Config.get(key)

Same as in previous versions.

const config = require('config');
console.log(config.get('services.db.host'));  // localhost

Config.has(key)

Same as in previous versions.

const config = require('config');
console.log(config.has('services.db.host'));  // true

[x] Config.loadFiles(options)

Uses Config.loadConfigFiles(options) and passes matching files to Config.parseFile(filename).

Accepts an options object which is passed down to loadConfigFiles:

• configDir - path of the configuration dir
• appInstance - appInstance used to match files
• environment - environment used to match files, defaults to "development"
• hostname - hostname used to match files, defaults to os.hostname()

const config = require('config')
  .loadFiles({
    configDir: __dirname + '/resources/configuration',
    appInstance: process.platform,
    environment: 'override',
    hostname: 'node-config',
  });

Config.parseFile(filename)

Extends the internal configuration object with the parsed content of filename.

const config = require('config')
  .parseFile(__dirname + '/runtime.json');

Config.extend(object, source)

Merges object into the internal configuration object and puts a new source into Config.sources with the given source. (source is optional and defaults to config.extend())

const config = require('config')
  .extend({some: {key: 'value'}})
  .extend(JSON.parse(process.env.MY_APP_CONF), '$MY_APP_CONF');

Config.loadDefaults(legacy)

Applies the options provided to the instance constructor on Config.loadFiles(options) and run strictness validation. If legacy is true then it also extends the configuration object with $NODE_CONFIG_JSON and --NODE_CONFIG_JSON, if available.

const config = require('config')
  .loadDefaults()
  .parseFile(`${process.env.HOME}/.config/services.json`);
console.log(config.get('services.db.host'));  // localhost

[x] Config.collectConfigFiles(options)

Used internally by Config.loadFiles(options).

This method is the heart of the library which holds the heuristics for collecting configuration files given a set of options. It returns an array containing matching configuration files according to their resolution order.

Accepts an options object with the following keys:

• configDir - path of the configuration dir
• appInstance - appInstance used to match files
• environment - environment used to match files, defaults to "development"
• hostname - hostname used to match files, defaults to os.hostname()

See Config.loadFiles(options) for an example.

Config.subModule(moduleName)

Sub-modules is a mechanism to create new configuration instances which are derived from their initiating module instance. The moduleName is also the path in which our sub-module is located. If the path already exists it will be used, otherwise we create the path set an new object at the end of it.

Sub-modules shares reference between their config and their main-modules config properties.

const config = require('config');
const subModule = config.subModule('TestModule');
console.log(config.get('TestModule') === subModule.config);  // identical

Notice that moduleName can contain dot notations to describe a deeper path.

const config = require('config');
const subModule = config.subModule('TestModule.Suite1');
console.log(config.get('TestModule.Suite1') === subModule.config);  // identical

Sub-modules also share sources with their main-modules. Any mutation executed from a sub-module will register the module property on the source item containing the module's name, and the data propery will emulate the sub-module's path.

const config = require('config').loadDefaults();
const subModule = config.subModule('TestModule').extend({defaultKey: 'defaultValue'});
console.log(config.sources === subModule.sources);  // identical
console.log(config.sources);

Would output:

[
  {source: '/path/to/config/default.js', data: {/* ... */}},
  //  ...
  {module: 'TestModule',
   source: 'config.extend()', 
   data: {TestModule: {defaultKey: 'defaultValue'}}},
]

Parser

I took some opinionated decisions that we can open a discussion about.

I removed the register methods for TypeScript and CoffeeScript. I don't think that's an action we should take, interfering with the registration options of a transpiler. Users of TypeScript and CoffeeScript should be familiar with these concepts. We basically treat these extensions as regular JavaScript files, and we expect the users to register these before they call the node-config.

I removed our support for multiple packages at the same parser function. Since it's now very easy to override parsers I think we should only offer the most common and let anyone who wants another package simply switch the parser.

I added a POC of a template engine as a default middleware (also a way to show off middleware). I'd like to discuss the idea further since this is only a POC version.

Parser.parse(filename)

Used internally by the mutation method Config.parseFile(filename).

• Extracts the extension from filename
• Calls the matching parser according to the extension
• Runs the result through our middleware and reduce the results
• Returns the result

Parser.readFile(filename)

Reads a file and returns its content if all validators successfully passed, otherwise it returns null.

Used internally by parsers to read a file and pass it to the parsing library.

lib.iniParser = function(filename) {
  const content = this.readFile(filename);
  return content ? require('ini').parse(content) : null;
};

[x] Parser.readDir(dirname)

Returns an array containing the content of the dirname directory.

lib.dirParser = function(dirname) {
  return this.readDir(dirname).map(filename => {
    filename = Path.join(dirname, filename);
    return [filename, this.parse(filename)];
  });
};

[x] Parser.collectFiles(configDir, allowedFiles)

Used internally by Config.collectConfigFiles.

// returns an array of matching filenames within __dirname
// keeps files order even when multiple directories are given
config.parser.collectFiles(`${__dirname}/http:${__dirname}/www`, ['index.html', 'index.htm', 'index.php', 'index.js']);

Parser.validators

An array of validators that are executed as part of Parser.readFile(filename).

Defaults to:

validators = [
  lib.validators.gitCrypt(!utils.getOption('CONFIG_SKIP_GITCRYPT')),
]

You can override the array to replace or clear the active validators.

const config = require('config');
config.parser.validators = [];
config.parser.validators = [
  // validate content is unicode
  (filename, content) => isUnicode(content),
];

Parser.middleware

An array of middleware that are executed as part of Parser.parse(filename).

Defaults to:

middleware = [
  lib.middleware.configTemplate(lib.middleware.configTemplate.defaultHandlers),
]

Important! The configTemplate middleware is only a POC at this point and should not be counted on. It is used here only as an example, but structure and functionality (or existence) may change.

You can override the array to replace or clear the active middleware.

const config = require('config');
config.parser.middleware = config.parser.middleware.concat([
  // check if es-module and has default then return content.default
  (filename, content) => content.__esModule && content.default ? content.default : content,
]);

Parser.definition

An array of parser definitions that are used with both Config.collectConfigFiles(options) and Parser.parse(filename).

It is recommended not to handle this property directly. Instead you can use Parser.set(ext, parser) and Parser.unset(ext) to add, remove and override parser, and Parser.resolution to set the resolution order in a simple manner.

Defaults to:

definition = [
  {ext: 'js', parser: lib.jsParser},
  {ext: 'ts', parser: lib.jsParser},
  {ext: 'json', parser: lib.jsonParser},
  {ext: 'json5', parser: lib.jsonParser},
  {ext: 'hjson', parser: lib.hjsonParser},
  {ext: 'toml', parser: lib.tomlParser},
  {ext: 'coffee', parser: lib.jsParser},
  {ext: 'iced', parser: lib.jsParser},
  {ext: 'yaml', parser: lib.yamlParser},
  {ext: 'yml', parser: lib.yamlParser},
  {ext: 'cson', parser: lib.csonParser},
  {ext: 'properties', parser: lib.propertiesParser},
  {ext: 'xml', parser: lib.xmlParser},
  {ext: 'ini', parser: lib.iniParser},  // this is new!
  {ext: 'd', parser: lib.dirParser},  // this is new!
]

Parser.resolution

A dynamic property that returns an array of parser.definition extensions. It's the same as executing parser.definition.map(def => def.ext). What's special about this propery is its setter which allows for a simple way to filter out and reorder parser.definition.

const config = require('config');
config.parser.resolution = ['js', 'json', 'json5', 'yaml', 'd'];

This removes parsers who are not included in the array and set this order for the existing parsers. Note that setting an array with an undefined (unknown) extension would result in an exception.

Parser.set(extension, parser)

Adds, or overrides in case it exists, a parser to Parser.definition.

const config = require('config');
const hoconParser = require('hoconparser');
config.parser.set('hocon', function(filename) {
  const content = this.readFile(filename);
  return content ? hoconParser(content) : content;
});
// autoload will now support hocon files

Parser.unset(extension)

Removes a parser from Parser.definition by extension.

const config = require('config');
// remove support for yaml and yml files
config.parser.unset('yaml').unset('yml');

Parser.reset()

Clears all default

const config = require('config');
config.parser.reset();
console.log(config.parser.validators);  // []
console.log(config.parser.middleware);  // []
console.log(config.parser.definition);  // []
console.log(config.parser.resolution);  // []

Parser.lib

A frozen object containing all the default parsers, validators and middleware.

const config = require('config');
config.parser.set('rss', config.parser.lib.xmlParser);  // equals to
config.parser.set('rss', function(filename) {
  return this.lib.xmlParser(filename);
});

Parser.lib.validators.gitCrypt(strict)

lib.validators.gitCrypt = strict => function(filename, content) {
  if (/^.GITCRYPT/.test(content)) {
    if (strict) throw new Error(`Cannot read git-crypt file ${filename}`);
    console.warn(`WARNING: skipping git-crypt file ${filename}`);
    return false;
  }
  return true;
};

Parser.lib.middleware.configTemplate(commandHandlers)

Okay, first the repeating disclaimer that this is just a POC.

I'd created an extremely simple, case-insensitive DSL that allows piping of command and applying flag modifiers.

It currently contains two commands and three options.

• "File::" - Command used to read a file using Parser.parse(filename). Returns an object
  • "string" - Modifier used to read the file using Parser.readFile(filename) instead. Returns a string
• "Env::" - Command used to set value of corresponding environment variable
  • "json" - Modifier used to parse the value using JSON.parse(str)
• "secret" - Modifier that can be used on any value to defines its property as non-enumerable. Replaces what used to be config.util.makeHidden()

example.json5:

{
  dbHost: 'localhost',
  dbPass: 'ENV::DB_PASSWORD',
  httpServer: 'ENV::HTTP_SERVER|json',
  ssl: {
    key: 'FILE::/credentials/ssl.key|string',
    pem: 'FILE::/credentials/ssl.pem|string',
  },
  runtime: 'FILE::ENV::CUSTOM_RUNTIME_JSON_PATH',
  secretKey: 'ENV::SPECIAL_SECRET_KEY|secret'
}

example.yaml:

dbHost: localhost
dbPass: ENV::DB_PASSWORD
httpServer: ENV::HTTP_SERVER|json
ssl:
  key: FILE::/credentials/ssl.key|string
  pem: FILE::/credentials/ssl.pem|string
runtime: FILE::ENV::CUSTOM_RUNTIME_JSON_PATH
secretKey: ENV::SPECIAL_SECRET_KEY|secret

Related issues

Solved or no longer relevant due to new architecture:

• #549 - Immutability
• #546 - SubModules
• #543 - SubModules
• #517 - SubModules
• #514 - Immutability
• #478 - Obsolete
• #472 - Obsolete
• #471 - Immutability
• #424 - SubModules
• #412 - Obsolete
• #338 - Obsolete
• #329 - Immutability
• #226 - SubModules
• #225 - SubModules

We can provide a middleware solution:

• #539 - Middleware
• #536 - Middleware
• #521 - Middleware / Parser
• #400 - Middleware
• #372 - Middleware
• #324 - Parser

[lorenwest]

How do you propose migration of existing users?

If you could install this and use features as-needed vs. rewrite, I'd be supportive.

[markstos]

Breaking changes can considered as part of different categories:

1. Already deprecated. No action needed.
2. Expected to be used a very small number of users. Document the change.
3. Expected to be used by a significant number of users. Provide a plugin, clear migration path, or just don't break it.

Given that framework, here's my specific feedback on the breaking changes:

Removed runtime.js support.

it was deprecated in 2014 when there was a push towards immutability. There have been no bug reports about this since 2014. I think it's fair to consider it "not used".

Removed custom-environment-variables.* support

A proof-of-concept for a replacement was provided. I would like to a 1-to-1 plugin or built-in feature provided that continues to support the feature. The PoC demonstrates this could be done with a custom middleware parser.

Changed location of asyncConfig and deferConfig

This two changes can be considered separately. asyncConfig is so new that backcompat is not a concern.

defer() is somewhat widely used. I'm OK with this, but it also seems fairly easy to leave shim files at config/async.js and config/defer.js (and config/raw.js?). The old structure can be deprecated during one major release and removed in later release. The docs can immediately updated to highlight the new recommended syntax so new users start using the new syntax immediately.

I realize it's a one-line change in a config file, but I also see that in my project I have over 100 config files where I would need to make the one line change. :)

get() and has() are available only on the config instance

I think I use this feature somewhere. Without this feature, some people will restore to raw property access: ex: db.host. That's worse because it won't catch a typo like db.hots, or db.cluster.hosts might throw an error where db.has('cluster.hosts') might throw an error.

This feature is not essentially and I realize removing it may simplify some things, but it does have benefits. Can it be implemented as a plugin?

Other feedback (related to get() and has() on sub-objects)

• The docs for Config.create should be updated to document which options are required
• What's the recommended way to create a new config object where you have the configuration data as an object instead of in files? Should there be an option to Config.create() for this purpose that ignores files all other? Such a solution might be an alternative to having get() and has() on sub-objects. For example:

const dbConfig =  config.create({ config: config.get('customer.db') })
dbconfig.get('hosts')

getArgv() slightly differs from its predecessor

This behavior was probably "undefined" before. Impacted user s are expected to a very small number. The new behavior does seem more sensible.

[iMoses]

If you could install this and use features as-needed

@lorenwest I'm not sure what you mean by that.. I listed all the breaking changes so we can discuss the necessity and migration process of each, and most of them demostrate a way to reproduce the same functionality.

BTW, I'm under the assumption that most users are not using any of the advanced features, meaning that they won't have any migration process at all!

Removed config.util methods

Most effected users will be power users who have used config.util methods. If I userstand correctly, the most commnly used methods are serModuleDefaults and loadConfigFiles both of which are now natively supported and provide better stability and finer control using Config.loadFiles(), Config.parseFile(), Config.extend() and Config.subModule().

Removed runtime.json support

I agree with @markstos, this one is already depracated for a long while, and a user can easily use Config.parseFile() to migrate their code and continue using this feature.

Removed custom-environment-variables.* support

This is the biggest change and I think we should discuss the template engine a bit before deciding on the migration process for this feature. Honestly, this is the only breaking change I'm not really sure what to do with, but i truly believe that we should provide a more consistent solution via a template engine.

get() and has() are available only on the config instance

@markstos your second suggestion is actually already supported, you were just looking at the wrong feature because that's exactly what Config.subModule() is for:

const dbConfig =  config.subModule('customer.db');
dbconfig.get('hosts')

Others

What's left is getArgv() which shouldn't really effect anyone and is basically a bug fix in my opinion, asyncConfig and Parser changes, both so new that other than early adopters we won't acctually break anything.
I added shim files with depracation warning to config/async, config/defer and config/raw to ease the migration process.

[iMoses]

@lorenwest I added back support for custom-environment-variables.* as @markstos suggested.

It's now part of the loadDefault(legacy) legacy mode (that's the autoload default) and there's a special middleware for dealing with matching filenames that is set by default. One less thing to worry about.

That basically leaves us with two major changes.

• get() and has() will not be set on values. The sub-module mechanism can cover for it
• config.util is gone - that's a big part of the motivation for this change

Is this good enough? Any input?

Plus, I was hoping we'd discuss the template engine here, meaning I wanna hear if you think support these options is enough or do you have any other ideas for a template engine. for example, the current implementation doesn't support partials and can apply "secret" only to env-vars and files.

I know we're all working people and you don't have much time for this, but I'd appreciate any sort of a response that can help us move forward.

Config.prototype.loadDefaults = function(legacy = false) {
  this.loadFiles(this.options);
  if (legacy) {
    const extendWith = (value, source) => {
      try { value && this.extend(JSON.parse(value), source); }
      catch(e) { console.error(`${source} is malformed JSON`); }
    };
    // these lines are new, to handle custom-environment-variables
    const legacyFiles = [];
    this.parser.resolution.forEach(extName =>
      legacyFiles.push(`custom-environment-variables.${extName}`));
    this.parser.collectFiles(this.options.configDir, legacyFiles).forEach(this.parseFile);

    extendWith(process.env.NODE_CONFIG, '$NODE_CONFIG');
    extendWith(utils.getArgv('NODE_CONFIG'), '--NODE_CONFIG');
  }
  strictValidations(this.sources, this.options);
  return this;
};

// new middleware, set by default
lib.middleware.customEnvironmentVariables = function(filename, content) {
  if (Path.basename(filename).startsWith('custom-environment-variables.')) {
    utils.collect(content, Boolean).forEach(([ value, key, object ]) => {
      if (typeof value === 'string') {
        object[key] = process.env[value];
      }
      else if (value.__format === 'json') {
        try {
          object[key] = JSON.parse(process.env[value.__name]);
        }
        catch(e) {
          throw new Error(`Failed to parse "${value.__name}" environment variable`);
        }
      }
    });
  }
  return content;
};

I hope this also demonstrates how easy it is to provide solutions for these problems with the new infrastructure :)

P.S. would you prefer it if I make a PR at this point?

[sudosoul]

Haven’t reviewed yet, but +1 for thinking on custom-environment-variables.* support

…

On Jul 31, 2019, at 7:31 PM, iMoses ***@***.***> wrote: @lorenwest I added back support for custom-environment-variables.* as @markstos suggested. It's now part of the loadDefault(legacy) legacy mode (that's the autoload default) and there's a special middleware for dealing with matching filenames that is set by default. One less thing to worry about. That basically leaves us with two major changes. get() and has() will no be set on values. The sub-module mechanism covers for it config.util is gone - a big part of the motivation for this change Is this good enough? Any input? Plus, I was hoping we'd discuss the template engine here, meaning I wanna hear if you think support these options is enough or do you have any other ideas for a template engine. for example, the current implementation doesn't support partials and can apply "secret" only to env-vars and files. I know we're all working people and you don't have much time for this, but I'd appreciate any sort of a response that can help us move forward. Config.prototype.loadDefaults = function(legacy = false) { this.loadFiles(this.options); if (legacy) { const extendWith = (value, source) => { try { value && this.extend(JSON.parse(value), source); } catch(e) { console.error(`${source} is malformed JSON`); } }; const legacyFiles = ['custom-environment-variables']; this.parser.collectFiles(this.options.configDir, legacyFiles).forEach(this.parseFile); extendWith(process.env.NODE_CONFIG, '$NODE_CONFIG'); extendWith(utils.getArgv('NODE_CONFIG'), '--NODE_CONFIG'); } strictValidations(this.sources, this.options); return this; }; lib.middleware.customEnvironmentVariables = function(filename, content) { filename = Path.basename(filename); if (filename.startsWith('custom-environment-variables.')) { utils.collect(content, Boolean) .forEach(([ value, object, key ]) => { if (typeof value === 'string') { object[key] = prototype.env[value]; } else if (value.__format === 'json') { try { object[key] = JSON.parse(prototype.env[value.__name]); } catch(e) { throw new Error(`Failed to parse "${value.__name}" environment variable`); } } }); } return content; }; I hope this also demonstrates how easy it is to provide solutions for these problems with the new infrastructure :) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[iMoses]

• What's the recommended way to create a new config object where you have the configuration data as an object instead of in files? Should there be an option to Config.create() for this purpose that ignores files all other? Such a solution might be an alternative to having get() and has() on sub-objects. For example:

@markstos BTW, I forgot to give you a direct answer about that.

The autoload mechanism only functions if none of the mutation methods were called. Once called you must call loadDefaults(legacy) manually to reproduce its effects when you need them.

If someone wants to use the library without files the recommended way is to use the mutation methods: config.extend(object), config.parseFile(filename) and config.loafFiles({configDir, environment, appInstance, hostname}).

const config = require('config');
config.extend({a: 'b'});
config.get('a');
// no files were loaded!

OR

const config = require('config').create();  // not the default instance!
config.extend({a: 'b'});
config.get('a');
// no files were loaded!

FYI, There are technically three undocumented options that Config support: config, module and parser. These are used internally as part of the sub-module mechanism and might be used in the future by external tools to extend this library, but I don't want to advertise them because I consider them harmful.

[lorenwest]

You're right that we're all working people and have lives, and frankly this is a bit overwhelming. This amounts to a major rewrite with multiple design changes, many of which weren't discussed until this thread.

I can't get past the starting gate - why? I understand your motivations and aren't in the same place as you with regard to the need for this much change.

For example - sub-modules. If you see the solution to subModules as multiple instances, then multiple instances could be used as motivation for a change. But multiple instances aren't how my company solved for sub-modules, so at least in my case, multiple instances are a distraction at best, and un-necessary complexity at worse. It changes client code by requiring them to have independent variables for each sub module. This may (or may not) be cleaner, but in my opinion that ship has sailed, and forcing people to change is inappropriate.

Middleware and configurable configuration. If you think it's good to have one config experience at one company (or one project) and a different configuration experience at a different company (or project), you may think pluggable middleware and pluggable template engines are a good thing. For configs I happen to think consistency trumps flexibility, so I'm not convinced the config library needs to be infinitely configurable like other .js libraries. Those libraries need configurations found in node-config to configure them. At what point will someone see the need to write a config library for node-config?

Before embarking I suggested we discuss issues independently - you may have gotten a feel for the direction of node-config before embarking on such an ambitious project.

I understand you've put a lot of thought into these solutions, and truly appreciate your effort. There's a lot of goodness here. In my opinion the best way to move forward is to use this thread as a framework for discussion of individual features, and add functionality one feature at a time. Most users of this library aren't screaming for immediate change.

[iMoses]

^ Most users of this library aren't screaming for immediate change.

@lorenwest I truly don't understand you.. Let's start with the fact that most users of this library won't even feel this change. The default heuristics of the library remains (99%) intact and users who have no need for the advanced features won't be affected at all. All tests still pass (excluding the removed config.util).

So we're only talking about the power users who requested these features to begin with. Half of the open issues here, some dating back 4-5 years, relate to the patchy implementation of immutable config which still causes multiple errors for people using the config.util methods (including setModuleDefaults) and many issues which are related to the fact that the library always provide a single instance and loads only with external options from env-vars and cli-args, which makes this library unusable for package owners who don't wanna pollute user configuration in case they too wanna use this library and unstable for people who have to override and revert env-vars just to change configuration from within nodejs.

I don't think that anyone should wait until they must push "immediate changes" and have a major release. I'm looking at new PRs and I see that most of them are very patchy, not because contributers are bad programmers but because the library is currently in a bit of a mass. We have a lot of util methods, most of which violate the DRY principle and could have been merged into a single utility which I have shown in my code. The new parser feature, although better than nothing, is far from being comfortable, and these changes I'm offering truly address each and every one of these issues and then some.

Of course it's a major change and requires a major version, it's meant to take this library to the next major stage - supporting programmatic configuration and improving the API. This is my proposal and this is the main issue I wanted to discuss, but if you wanna start discussing other matters I'm all in, let's talk.

Now we can discuss anything you'd like, but honestly it feels like I need to force a response out of you on these matters. For example, why only give examples and not start listing the subjects you'd like to discuss according to priority so we can embark in that discussion?

I'm not attached to the code but I do think it makes things easier that I can simply show you what I thought about and we can go from there. You can even play with the library, install it locally to judge the changes yourself, there's no downside to it other than my lost time that shouldn't be your concern.

Let's start with the burning issues and move downwards from there.. I'm still unsure from your responses what is your exact opinion on each of these changes.

P.S.

• About your sub-modules note please review Config.subModule and not Config.create. The method doesn't change the logic and although it does create a new instance, the instance shares it's configuration object and sources with its main module, so it isn't independent in any way.
• Regarding middleware, I view it as a better internal mechanism that we can or cannot expose so that power users would be able to modify it. We can chose not to allow external editing of middleware and I still see it as a good feature which make for a better infrastructure.

Notice that I'm and was very cooperative with all of your requests, not just on this issue. I don't mind the feedback, I appreciate it and want to make you sure about these change before agreeing to anything. I just need your cooperation with flagging your concerns so we can discuss/solve them and move forward.

[iMoses]

There's one more point I want to make..

The biggest breaking change here is replacing config.util with a stable API (anything else we can revert). You may see it as big changes, but technically we're just providing the same functionality we had with config.util methods but in a more stable and consistent manner. I honestly believe that this deserves a major release.

[markstos]

@lorenwest I agree that "node-config shouldn't need a config", but I don't see that being proposed here. What I do see is a fresh approach that addresses whole categories of bug reports we receive. About two dozen have been carefully documented. If we didn't want node-config to address those issues, why have we left those two dozen issues open, waiting for someone to move them forward?

This is a lot of change at once, but when a problem is approached with a new design an incremental approach is not a good fit.

Your mentioned your company went another direction with sub-module support. Is there is a specific alternate approach that you propose as an alternative what's laid out here?

This is a solid "next-gen" proposal for node-config and avoiding a fork would allow us to pool maintenance effort.

[lorenwest]

Here's a proposal for the rollout:

Phase 1 - code restructure. Separate code structure changes into one or more minor releases with 100% backward compatibility. This forms the foundation for the work requiring major revision change, and gets it installed and street tested independently. It forces us to separate features requiring a major revision from those we can introduce without disruption.

Phase 2 - deprecation warnings. Another point release introducing (squelchable) log statements announcing deprecation for incompatible usage in the upcoming major revision.

Phase 3 - major release. Bundle features/commits requiring a major release into one NPM publish. By this time we've discussed each feature independently, and have a clear understanding of the cost/value of introducing the features as non-backward compatible. Also, a fair amount of time has passed with the code restructure in the wild that new issues are reduced to new features.

I appreciate your guidance on the code restructure, and think the phase 1 changes can go through quickly. My company has a deep understanding and appreciation for the value of sub-module configs, and I want to start a separate thread on that feature. It may or may not need a major revision update.

The motivation for this proposal is to introduce separate concerns into a managed rollout. This allows us to clearly understand each feature, mitigate risk, and minimize community disruption.

ps: @iMoses - for some reason github doesn't recognize you as a contributor if branches from a different repository are merged into this repository. I suggest future pull requests to come from branches of this repository so the auto-generated contributor list recognizes you as a core contributor.

[iMoses]

ps: @iMoses - for some reason github doesn't recognize you as a contributor if branches from a different repository are merged into this repository. I suggest future pull requests to come from branches of this repository so the auto-generated contributor list recognizes you as a core contributor.

This is a bug caused by me pushing changes from an email address that wasn't registered in Github (my work email). I fixed it but I have a feeling that the contributers are cached and will change only on the next commit to this repository. (hover the contributers above the code and see that I appear there)

Regarding your proposal, I'll see what can be done, though most of the new features relay on the files being lazily loaded, which in turn requires the extraction of the configuration object from the root of the instance into a separate property, and that's already a breaking change. Without this feature we can't use the new parser, meaning no middleware and validators, no mutation methods, no create or sub-module methods... I'm not sure what else will be left.

I can see how we can slowly add new functionality, instead of publishing it all at once, but it seems that the initial requirement for it all is a breaking change by itself and that would have to be one of the first things to be fixed/changed.

If you look at the new API it may seem big (although it's less code then before the change), but if you see what was actually changed we're not talking about too much, and anything I was able to fix without this change I already did (new parser, async-config, deferred bug, multiple dirs..). From the top of my head it seems that the only thing I can add to the current version is support for ".d" extension for directories and making "raw" obsolete by skipping complex object entirely.

Also notice that depracation warnings were already added for the change in location of async, defer and raw and backward compitability is automatically enabled for most features. We're only talking about removing config.util and not having get() and has() on configuration values prototypes.

[iMoses]

@lorenwest honestly, I'm going over the code to see if there's anything I can push before the breaking change and I think the code is too messy to push anything "next-gen" without breaking something. If you prefer to have multiple minor breaking changes than a single big change that can be done, but I don't see the benefit of it, quite the opposite.

I can't move util to a different file because attachProtoDeep references the Config class directly and changing this would break both its API and setModuleDefaults API which relay on it. If I try to add the new dir parser it relays on loadFileConfigs but doesn't require all of it's logic, so we need to break that one as well.. I also can't add support for ".d" directories extension without duplicating more logic or creating more utility methods, and can't make "raw" obsolete without losing support for get() and has() on values prototypes.. A big part of my reason for this rewrite was to refactor all spaghetti code, and we have a lot of it..

We can introduce many minor methods and expose a lot more functionality to get there, but this will only increase the size of the change once we push to a new version because none of the "next-gen" functionality can be implemented without breaking anything..

The only thing I can do is update defer and async which stayed mostly the same. I agree with @markstos that "when a problem is approached with a new design an incremental approach is not a good fit". I think it's only gonna make things more complicated both for us and the users.

There's another option which I dislike but thought maybe you'd prefer.. I can try and shim important methods from config.util with the new methods to reduce even further the breaking changes and add deprecation warning to these as well.

Perhaps the best approach it to go out with the breaking change, considering we're still passing all tests, shim whatever we can and postpone new functionality to later minor releases (such as support for ".d"). That way we introduce as little changes as possible, even though we're still breaking a part of the API, and continue with additions and removal of deprecated methods later on..

Maybe writing really long docs is not the best method for us to discuss this.. If you're up to it we can have a quick call and I'll present to you in details my solutions..

[markstos]

@iMoses Thanks for thinking through how an incremental approach might work in this. It sounds painful, and ultimately results in a breaking-change release anyway. +1 for getting on a call with Loren West to discuss in real time.

[markstos]

@iMoses Could you document the signature for validators? The docs for Parser.validators say that it is an "array of validators", but I didn't see the definition of validation. I presume it's a function but could you document the signature? Thanks.

[jdmarshall]

I presumed based on the fact that this was still open that you were still toying with that idea. Also I confess I didn't check the dates.

Any thoughts on cherry-picking some of the bits for a 4.0 release? In particular I like the constructor, and the separate util. Parser is already kind of pulled out. Do you have a wish list?

[markstos]

Lots of good ideas here and also lots of breaking changes. Perhaps best published as a mostly-compatible fork.

@lorenwest and I have been volunteering as node-config maintainers for about a decade, largely fielding issues and PRs about features we don't use ourselves. A major release with a lot of breaking changes would help a lot of folks and improve a lot of things, but it would also surely bring with it a big upswing in issue tracker traffic, which we are already not keeping up with.

Published under a new name, the maintenance load would ramp more slowly... and some other folks could take the lead on the next decade of maintenance!

[x80486]

Not much to say for the implementation, but on the usability side, would be great if environment variables could be expanded, and somehow have some syntax to make sure they are set: ${REQUIRED_ENV_VAR:?error} or to default to some value in case they are not provided: ${REQUIRED_ENV_VAR:-default_value}.

This syntax is used by Docker, hence it's already familiar for most developers — so bonus points on that.

[markstos]

@x80486 The syntax you are showing is Bash syntax, which you are welcome to use before passing environment variables to node-config if you are variables are coming from the bash shell. The feature is called Shell Parameter Expansion, and you can read about it here:

https://tiswww.case.edu/php/chet/bash/bashref.html#Shell-Parameter-Expansion

Most developers I've met are not aware of the bash shell parameter expansion feature.

node-config already includes .has() method to check if an environment variable is defined, while the cascading config system already supports setting and overriding defaults. No further changes are planned there.

[x80486]

Probably that's true for the Node.js world, but in Java and Go this is quite the standard these days.

Frameworks like Micronaut, Quarkus, and Spring Boot supports this as a built-in feature — think of it like node-config already integrated.

In Go I've tested a couple of libraries as well, like gookit/config.

While the technologies are different, I think there is some resemblance in the way they all manage configuration in the end. Basically, the external/underlying library uses YAML syntax (among others) and you can directly use environment variables, so they get expanded automatically, defaulting (or failing) to some other values, when specified:

datasource:
  url: ${JDBC_URL:jdbc:mysql://localhost:3306/db}
  username: ${JDBC_USER:root}
  password: ${JDBC_PASSWORD:}

[markstos]

If you want to use a config file format that looks like YAML but processes environment variables, some plugin could written which parses a file format like that, but it's not something that needs to be in the core node-config project.

At the top of this thread, in the section on Parsers is a comment about how it would be easier to define external parsers so people can parse different file formats like this.

You could take the work on this branch and publish as a new config module which would have this feature, then create your own YAML parser which also supports this bash-style syntax for environment variables. What the project is missing now is not such features, but people willing to the do work of maintaining one of the popular config modules for Node.

[jdmarshall]

I am now mostly convinced that the correct solution for the config-after-get problem is to modify defer() to accept a list of properties the deferred function requires in order to resolve the deferred values.

This handily gets around the problem of calling get() by not calling get(), and also solves a problem with order of processing when multiple defer calls need to be chained

[markstos]

I like that idea. By being explicit about the deps we can be certain they are resolved first.

If a circular dep is detected we can crash with an error message.

[jdmarshall]

getArgv() slightly differs from its predecessor

In case of multiple matching arguments it returns the last (previous logic was to return the first occurrence). It actually seemed like a bug-fix to me, since that's what I would've expected to be the correct behavior. I also added support for boolean and space separated cli-args.

It is typical in CLIs that allow multiple uses of the same parameter for them to be both honored, rather than the latter. And given that we support reading of multiple NODE_ENV values, (eg, "production,us-west-1"), then to me it seems the better choice would be to process --NODE_ENV prod --NODE_ENV foo into "prod,foo" instead

[jdmarshall]

I've started checking off some of the items in this conversation that either have PRs/landed with a [x] and the ones where I am aware of work in progress (mostly mine) with a [...] so it's easier to see what is left to work on/negotiate about.

[markstos]

And given that we support reading of multiple NODE_ENV values, (eg, "production,us-west-1"), then to me it seems the better choice would be to process --NODE_ENV prod --NODE_ENV foo into "prod,foo" instead.

Since we do process comma-separated NODE_ENV values, I agree.

[jdmarshall]

That said though, I don't believe a library not involved in command line processing should even have functionality that processes command line arguments. It's... In 2025 it seems a very very odd choice. Maybe before commander.js and friends existed it was less odd.

[markstos]

Even more true @jdmarshall. Removing any CLI processing from node-config is even more sane choice. ENV vars can be set on the command line as well, so there's no need to also process arguments as well.

[jdmarshall]

const config = require('config');
const db = config.get('services.db');
console.log(db.has('host'));  // Error: method has doesn't exist

I solved this problem a different way, though I don't know that it will work here because of the way deferConfig works.

I didn't expose the underlying data at all. Everything was exposed via get(path). You could construct an instance that only cared about a submodule, and that worked by concatenating the paths. If the underlying data was immutable as with node-config, I could have just saved the new root.

[markstos]

Several good ideas here, but the scope here is so broad that's not a good fit as a single actionable issue that can be resolved. So I'm converting it to a discussion where big-picture conversion can continue, while focused, actionable related issues can be open to refine and implement details.