Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for config files #1584

Closed
Mingun opened this issue Aug 14, 2021 · 35 comments
Closed

Add support for config files #1584

Mingun opened this issue Aug 14, 2021 · 35 comments
Assignees

Comments

@Mingun
Copy link

Mingun commented Aug 14, 2021

Problem

Adding support of config files when you have an option with a default value and you want the following priorities, is complicated.

  1. Value from the command line
  2. Otherwise value from the config file
  3. Otherwise the default value

Because commander has no built-in support for config files, you can't use all its power, because options object that you get by calling opts() contains mixed default parameters (point 1 from above) and cli parameters (3) and you should manually inspect each option with default to correctly apply the config file (2) parameters. I would like if commander could do this for me.

The current workaround looks like this:

import { Command, Option } from "commander";

/// All options with defaults should be declared there
const DEFAULTS = {
  optionWithDefault: "default value",
};
let options = new Command()
  .option("--config <json>")
  .addOption(
    new Option("--option-with-default <value>")
      .default(DEFAULTS.optionWithDefault)
  )
  .parse()
  .opts();

// Remove all default options from the parsed options
for (const key of Object.keys(DEFAULTS)) {
  if (DEFAULTS[key] === options[key]) {
    delete options[key];
  }
}

const cliOptions = options;
const config = options.config;
delete options.config;

options = Object.assign({}, DEFAULTS);
if (config) {
  options = Object.assign(options, readFileAsJSON(config));
}

but it is incomplete: if I specify --option-with-default "default value" in the command line the code above will think that it is default value and override it with the config option value, which is not expected. To solve this problem I'll have to parse options manually, which effectively makes commander useless in such scenario.

Proposal

The proposed API could looks like:

import path from "path";

let options = new Command()
  .option("--option <value>")
  .addOption(
    new Option("--default <value>")
      .default("default")
  )
  .addOption(
    new Option("--config <file>")
      // Optional function should return Object with properties with values
      // from the config file
      // By default uses the specified function that could process both .json
      // and .js (as CommonJS modules, like eslint) config files
      .config(val => require(path.resolve(val)))
      // If config not specified and have a default, this file tried to read
      // using config read function from `.config()`
      .default(".config.js")
  )

The examples for the CLI definition below:

.config.js does not exist

  • Command: prog
    Result:
    { default: "default" }
  • Command: prog --default a
    Result:
    { default: "a" }
  • Command: prog --option b
    Result:
    { default: "default", option: "b" }

.config.js contains option

module.exports = {
  option: "config",
};
  • Command: prog
    Result:
    { default: "default", option: "config" }
  • Command: prog --default a
    Result:
    { default: "a", option: "config" }
  • Command: prog --option b
    Result:
    { default: "default", option: "b" }

.config.js contains default

module.exports = {
  default: "config",
};
  • Command: prog
    Result:
    { default: "config" }
  • Command: prog --default a
    Result:
    { default: "a" }
  • Command: prog --option b
    Result:
    { default: "config", option: "b" }
@shadowspawn
Copy link
Collaborator

shadowspawn commented Aug 14, 2021

Commander does not explicitly track whether an option has been specified on the command-line so I can see that applying the priorities for adding in values from config files is currently tricky. Some of the same issues come up with mixing in environment variables.

One work-around approach might be to not specify the default value to Commander, and instead modify the Help to display the external default value (in your example, from DEFAULTS object). Then you can hopefully merge the potential values in the desired order: from DEFAULTS, config file if specified, and command-line.

@shadowspawn
Copy link
Collaborator

A couple of thoughts.

Regarding proposed API. I think there would be support at the Command level so a startup file can be read without a corresponding option. I'm not sure at the moment if also makes sense as an Option method, or whether handled by client using the Command support.

Support for subcommands requires some thought. I think of config files as being global normally. There is probably a low chance of reuse of option names between program and subcommand, but a higher chance of potentially conflicting reuse between subcommands. So for a full solution the config options may need to be qualified by sub-command path if they can be imported globally.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Aug 24, 2021

To implement environment variable support with the desired balance of priority for default/env/cli, I have a PR which uses a new property to add and consult the "source" of the option value. I had config file in mind as another priority level when I was writing it. (Currently private property in the PR: #1587 )

if (defaultValue !== undefined) {
   this._setOptionValueWithSource(name, defaultValue, 'default');
}
// env is second lowest priority source, above default
if (this.getOptionValue(optionKey) === undefined || this._optionValueSources[optionKey] === 'default') {
   ...
}

@shadowspawn
Copy link
Collaborator

Related old issues: #151 #328

@jakobrosenberg
Copy link

jakobrosenberg commented Sep 4, 2021

To whom it may concern, this is how I currently manage configs.

const config = require('./path/to/config')

const useConfig = process.argv.find((arg) => ['--help', '-h', '-d', '--use-defaults'].includes(arg));
const defaults = useConfig ? config : {}

program
  .option('-i, --install-dir <dir>'), 'location', defaults.installDir
  .option('-d, --use-defaults'), 'silent install', false
  .action(async options => {
    /** iterate over options and prompt missing fields */
  })
  ...

As for manually specifying a config file, would it be possible to do something like

const softRequire = path => {try {return require(path)} catch(e){}}

const configPath = getSingleOption(process.argv)('-c --config') || 'mylib.config.js' //made up commander helper

const config = softRequire(configPath)}
const defaults = { /** options */}

const options = { ...defaults, ...config }

program
  .option('-c --config', 'config location')  // dummy option. Only included for `--help`
  .option('-i --install-dir', 'install location', options.installDir)
  ...

EDIT: updated bottom script

@alexweininger
Copy link

Would love to see this feature in commander. It's currently pretty complicated to implement defaults, a config file, and the ability to override the config file via command line args.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 11, 2021

Assuming config can use a dictionary (hash) of option config values, what keys should be supported? The mapping from dash-separated to dashSeparated is a programmer detail, so I am thinking at least the flags themselves should be supported since these are what end-users work with.

program.option('-p, --port-number <number>');
// possible config objects
{ '-p': 8080 }
{ '--port-number': 9000 }

For keys that do not start with a dash, Commander could perhaps look for a long option flag that matches (e.g. port-number), or look for the JavaScript style name (e.g. portNumber) for programmers configuring their own programs.

{ 'port-number': 8080 } // ????
{ 'portNumber': 9000 } // ????

@alexweininger
Copy link

I think it would be best to only allow the JavaScript style names of options for keys of the config file. This would mitigate conflicts, and users are familiar with JavaScript style property names in JSON.

I bet it would be possible for commander to generate a JSON Schema file for the config file.

@Mingun
Copy link
Author

Mingun commented Sep 11, 2021

Yes, I support @alexweininger -- supporting only JavaScript style will be enough, at least in the beginning

@jakobrosenberg
Copy link

What would the equivalent of this be with a builtin config path option?

// softRequire is like require, but doesn't throw error on missing files
const softRequire = path => {try {return require(path)} catch(e){}}

// made up commander helper
const configPath = getSingleOption(process.argv)('-c --config') || 'mylib.config.js' 

const options = softRequire(configPath)} || {}

program
  .option('-c --config', 'config location')  // dummy option. Only included for `--help`
  .option('-i --install-dir', 'install location', options.installDir)
  ...

I'm a little on the fence about a native configs feature.

  • It adds complexity to the codebase
  • It adds more documentation
  • Config resolution becomes magic
  • Don't expand the API and documentation to solve something that could adequately be solved with the existing API and an example. It conceals the basics and edge cases that could have been solved with knowledge of the basics are either not possible or require further API expansions...
  • This sounds like something that could easily be solved with a plugin or tiny helper library.

My preferred solution would be a loadConfig helper which takes the option as the first parameter and a the default path as the second parameter.

const { loadConfig, program } = require('commander')

const options = loadConfig('-c --config', './my-default-config-path.js')

program
  .option('-c --config', 'config location')  // dummy option. Only included for `--help`
  .option('-i --install-dir', 'install location', options.installDir)
  ...

@shadowspawn
Copy link
Collaborator

What would the equivalent of this be with a builtin config path option?

Actually, I'm still thinking about lower-level details and don't have a builtin config path option in my head! So this code is a bit different, but where my thinking is up to:

program
  .option('-c --config <file>', 'config location')
  .option('-i --install-dir <file>', 'install location');
program.parse();

if (program.opts().config) {
  const configHash = require(program.opts().config);
  program.assignConfig(configHash); // does not override CLI values
}

console.log(program.opts().installDir); // may come from config or command-line

@alexweininger
Copy link

If there was an easier way to discern if an option has been passed in by the user or not, implementing a config file while using commander becomes a lot easier. I think solving this problem first might be more of a priority than to create a fully integrated config file feature.

Quoting from #1584 (comment)

but it is incomplete: if I specify --option-with-default "default value" in the command line the code above will think that it is default value and override it with the config option value, which is not expected. To solve this problem I'll have to parse options manually, which effectively makes commander useless in such scenario.

@shadowspawn
Copy link
Collaborator

Hopefully solved that problem internally while implementing .env(): #1584 (comment)

@jakobrosenberg
Copy link

@shadowspawn I like your example, but how would it handle this scenario?

//mycli.js
const { loadConfig, program } = require('commander')

// specify default config
const env = process.env.NODE_ENV || 'development'
const defaultConfigPath = `./config/${env}.config.js`

// use user provided config or fallback to defaultConfigPath
const options = loadConfig('-c --config', defaultConfigPath )

program
  .option('-c --config', 'config location')  // dummy option. Only included for `--help`
  .option('-i --install-dir', 'install location', options.installDir)
  .option('-a --architecture <arch>', 'system architecture', 'hardcoded-value')
  ...

Expectations:

  • If I run mycli, it should correctly use the values specified in the config corresponding to the environment.
  • If I run mycli --help it should print the correct defaults that corresponds to the environment.
  • If I run mycli --help --config myconfig.js it should print the defaults that I provided in my custom config.

Note for anyone who thinks configs shouldn't be treated as defaults: Please consider that these are the default values that options will default to. Showing any other value would be misleading.

@hildjj
Copy link

hildjj commented Sep 15, 2021

Just so we're looking at the whole problem at once, here is a list of all of the places that config information might come from in a full-featured CLI, in order of increasing precedence. Everywhere I say FILE, i mean myCli.js, myCli.cjs, myCli.mjs, or myCli.json, and there might be a built-in mechanism to extend configs contained in other packages:

  1. Built-in defaults, which might be system-dependent (e.g. the NODE_ENV example above)
  2. $XDG_CONFIG_HOME/myCli/FILE no error on file not existing, error if not readable, can't be parsed, or invalid option
  3. ../FILE (etc. Keep going until root directory, or you find one). Maybe allow for a root = true like eslint. Files closer to pwd take precedence. Some use cases might not want the searching.
  4. --config FILE on command line (this might turn off directory searching). Error if file does not exist.
  5. $MYCLI_[var] environment variables
  6. Command line

For each bit of config found, it needs to be carefully combined with the existing set, depending on type. Simple types like bool, string, and choices overwrite. Counts are specified as numbers in files, then overwrite. Variadic options get concatenated. Coerced options run through the (value, previous) => handler. There are probably a few other edge cases.

Note that a program might want to opt out of or into any of these (except the command line, I guess. :) ).

@shadowspawn
Copy link
Collaborator

#1584 (comment)

I was not imagining an overlap between defaults and a configuration file in that way. Interesting though, and clear outline of scenario.

I think of the configuration file(s) being outside the help, with perhaps a way to view the details:

npm config list
git config --global --list

Are there some (well-known) programs or frameworks that behave the way you describe?

If I run mycli --help --config myconfig.js it should print the defaults that I provided in my custom config.

Two issues:

  • the help and version options are handled as soon as they are reached (left to right)
  • I imagine the config value being a different value than the default, and not appearing in the help

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 16, 2021

#1584 (comment)

Wide coverage, thanks.

here is a list of all of the places that config information might come from

Another custom place is a key of package.json.

Everywhere I say FILE, i mean myCli.js, myCli.cjs, myCli.mjs, or myCli.json

yaml format has come up in previous discussions (#151), probably outside familiar javascript tooling examples (like eslint). If Commander handles the file opening, there could be a custom parse routine.

Variadic options get concatenated.

I did see recently that mocha does this with options which can safely be repeated: https://mochajs.org/#merging

With environment variables (.env()) I had decided it was a simple model to have environment variables override default values, whatever the type of the option.

@hildjj
Copy link

hildjj commented Sep 16, 2021

+1 to package.json. I think it goes between 2 & 3.

+1 to yaml being possible. Allowing for custom parsers is better than taking a dependency to do yaml out of the box, imo.

I don't think that env variables should always change the help to show a different default. If you go out of your way like @jakobrosenberg did in his example, you can work around that somewhat. Also, in that example, if your config file is .js, you can do the NODE_ENV check in there and just have one file. I get that only solves that one particular case, but it's something to remember; we don't have to provide ultimate flexibility in places where the user can use JS to customize.

@jakobrosenberg
Copy link

One thing to keep in mind through all this is separation of responsibilities so we don't end up with a Frankenstein's monster or Homer's car.

There are numerous config resolvers that could easily be used with Commander. All you need is just the path of the config file, which could be solved with a simple helper that returns the value of a single argument, eg. getOption('-c --config').

Adding a small helper vs a full-blown config resolver would add

  • less complexity to the code base
  • less API surface + documentation
  • more flexibility
  • fewer potential dependencies
  • clearer separation of responsibility
  • easier to reason about code

@alexweininger
Copy link

Hopefully solved that problem internally while implementing .env(): #1584 (comment)

If we could expose this so that we can consume this in the same way you did when implementing .env() I think that would solve the issues and allow people to implement config files on their own. I think this is better than providing some standard way to implement a config file through commander.

@shadowspawn
Copy link
Collaborator

Lots of good comments. Selected highlights:

@Mingun #1584 (comment)

Adding support of config files when you have an option with a default value and you want the following priorities, is complicated.

@alexweininger #1584 (comment)

It's currently pretty complicated to implement defaults, a config file, and the ability to override the config file via command line args.

@alexweininger #1584 (comment)

If there was an easier way to discern if an option has been passed in by the user or not, implementing a config file while using commander becomes a lot easier.

@hildjj #1584 (comment)

For each bit of config found, it needs to be carefully combined with the existing set, depending on type.

@hildjj #1584 (comment)

+1 to yaml being possible. Allowing for custom parsers is better than taking a dependency to do yaml out of the box, imo.

@jakobrosenberg #1584 (comment)

There are numerous config resolvers that could easily be used with Commander.

The two things I am currently thinking about are:

  • low level: expose option value source (default/env/cli) for custom implementations
  • medium level: add routine to do a careful merge of config object into option values, taking into account priorities, and custom option processing

For now, I am thinking of leaving it up to user to locate desired implicit config file (or package.json property), and parsing config file. There are so many possibilities and patterns used by utilities, and I don't want to constrain user into particular pattern unless it will cover wide range of real world use. But it needs to be easy to put together.

@alexweininger
Copy link

@shadowspawn

The two things I am currently thinking about are:

  • low level: expose option value source (default/env/cli) for custom implementations
  • medium level: add routine to do a careful merge of config object into option values, taking into account priorities, and custom option processing

Both of these sound great to me. And I totally agree with:

leaving it up to user to locate desired implicit config file (or package.json property), and parsing config file

@shadowspawn
Copy link
Collaborator

@jakobrosenberg

My preferred solution would be a loadConfig helper which takes the option as the first parameter and a the default path as the second parameter.

// made up commander helper [combined from examples]
const options = loadConfig('-c --config', './my-default-config-path.js', process.argv)

(This is to find config file from arguments, before doing full parse and help.)

I see the attraction for you to allow using the config as defaults so they are displayed in help, as in your example.

The full parse does recursive processing of subcommands with a lot of complexity, both for processing valid syntax and for showing good error messages. My instinct is that while it may be possible to robustly extract an option value for a single program where you know the syntax and configuration you are using, it would be hard to do robustly for a general solution.

@jakobrosenberg
Copy link

@shadowspawn for Commander to imports configs, that would be my favorite, yes. But I would still prefer that Commander handles arguments parsing and leaves module importing to native tools like require/import or dedicated libraries.

const configPath = getOption('-c --config')
const config = require(configPath)

Having a getOption would improve the argument parsing capabilities of Commander and I can think of numerous scenarios that could benefit from it. For instance, I often use prompt or enquirer with Commander, but I always add a --silent option that lets me enable/disable defaults. With getOption you could do something like this:

// we want to include the defaults if we're doing a silent install or if we're printing the help page
const useDefaults = getOption('-s --silent') || getOption('-h --help')
const defaults = useDefaults ? require('./config.js') : {}

program
  .option('-i, --install-dir <dir>'), 'location', defaults.installDir
  .option('-s --silent', 'install using default values')
  .action( async options => {
    // if we didn't use `--install-dir` or `--silent`, installDir is undefined and we will prompt the user
    options.installDir = options.installDir || await promptInstallDir()
  })

You could even combine the above example with config import. That would allow us to obtain config values by
CLI arguments -> user config -> program defaults (with silent option) -> prompt

example
// we want to include the defaults if we're doing a silent install or if we're printing the help page
const useDefaults = getOption('-s --silent') || getOption('-h --help')
const defaults = useDefaults ? require('./config.js') : {}

// load config
const configPath = getOption('-c --config')
const config = require(configPath)
Object.assign(defaults, config)

program
  .option('-i, --install-dir <dir>'), 'location', defaults.installDir
  .option('-s --silent', 'install using default values')
  .action( async options => {
    // if we didn't use `--install-dir` or `--silent`, installDir is undefined and we will prompt the user
    options.installDir = options.installDir || await promptInstallDir()
  })

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 19, 2021

It was more complicated enforcing the syntax than I expected, but I have some proof of concept code working. Example usage:

const { program, Option } = require('commander');

program
  .option('-d, --debug')
  .option('-c, --color')
  .option('-C, --no-color')
  .option('-o, --optional [value]')
  .option('-r, --required <value>', 'string', 'default string')
  .option('-f, --float <number>', 'number', parseFloat)
  .option('-f', 'number', parseFloat)
  .option('-l, --list <values...>')
  .addOption(new Option('-p, --port <number>').env('PORT'))
  .option('--use-config')
program.parse();

if (program.opts().useConfig) {
  const config = {
    debug: true,
    color: true,        // same as CLI --color
    color: false,       // same as CLI --no-color
    '--no-color': true, // same as CLI --no-color
    '-o': true,         // same as CLI -o
    'optional': 'config string',
    'required': 'config string',
    'float': '3.4',
    'list': ['apple', 'pear'],
    'port': '443'
  };
  program.mergeConfig(config);
}

console.log(program.opts());
$ node merge.js                                
{ required: 'default string' }

$ node merge.js --use-config                   
{
  required: 'config string',
  useConfig: true,
  debug: true,
  color: false,
  optional: 'config string',
  float: 3.4,
  list: [ 'apple', 'pear' ],
  port: '443'
}

$ PORT=9000 node merge.js -r RRR --use-config -o OOO            
{
  required: 'RRR',
  useConfig: true,
  optional: 'OOO',
  port: '9000',
  debug: true,
  color: false,
  float: 3.4,
  list: [ 'apple', 'pear' ]
}

(Edit: fixed priority order of env and config)

@shadowspawn
Copy link
Collaborator

Is it best to display a merge error if a config key does not match a declared option?

Displaying an error is helpful for picking up mistakes early, but prevents directly using a flat configuration file across multiple subcommands with mixture of unique options (by ignoring unknowns).

@Mingun
Copy link
Author

Mingun commented Sep 19, 2021

Is it best to display a merge error if a config key does not match a declared option?

I think, you can return the error from mergeConfig and allow the user to decide if he/she wants to display that error message.

'--no_color': true, // same as CLI --no-color

Wouldn't it be better to name key as --no-color to match the CLI option?

@shadowspawn
Copy link
Collaborator

--no_color was a typo, thanks. I'll fix that! (You can tell I have not implemented the error for unrecognised option key...)

@shadowspawn
Copy link
Collaborator

FYI @jakobrosenberg
I was thinking about partial parsing again after seeing this argparse python code: https://stackoverflow.com/questions/3609852/which-is-the-best-way-to-allow-configuration-options-be-overridden-at-the-comman/5826167#5826167

And came up with something along the lines you were suggesting:

function getSingleOption(flags, name, args) {
  const findOne = new Command;
  findOne
    .allowUnknownOption()
    .helpOption(false)
    .option(flags);
  findOne.parse(args);
  return findOne.opts()[name];
}

console.log(getSingleOption('-c, --config <value>', 'config', process.argv));
console.log(getSingleOption('--use-config', 'useConfig', process.argv));
% node partial.js                                                         
undefined
undefined

% node partial.js --help --before --config foo --use-config --after --help
foo
true

@shadowspawn
Copy link
Collaborator

shadowspawn commented Sep 25, 2021

I was not sure my mergeConfig approach in #1607 was going to solve enough config related use cases, and switched focus to providing the low-level routines to allow clients to work with priorities and option sources: #1613

@jakobrosenberg
Copy link

jakobrosenberg commented Sep 25, 2021

@shadowspawn , sorry completely missed your previous message.

Love the example. - Though the second parameter (name) should probably be derived from flags.

function getSingleOption(flags, args) {
  // flagToCamelCase for lack of better words ^^
  const name = flagToCamelCase(flags) 
  ...
}

Regarding the PR, I can't fully wrap my head around it, but I assume it abstracts the commented logic.

/** resolve defaults, configs and env */
...

const defaults = { ...myDefaults, ...myConfig, ...myEnv }

program
  .option('--cheese [type]', 'cheese type', defaults.cheese)

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Sep 27, 2021
@CheshireSwift
Copy link

CheshireSwift commented Sep 28, 2021

I just wanted to voice support for a lighter touch approach, rather than Commander being responsible for locating/opening/parsing config files itself.

There's already fantastic tools that make this easy (e.g. cosmiconfig). If Commander exposes a way to detect an option's source (or even a way to merge an arbitrary object into the options), then those existing tools can be leveraged easily, while still leaving the door open to whatever more niche approaches a given project might need.

@shadowspawn
Copy link
Collaborator

Did a deep dive on config files, but for now just added .getOptionValueSource() and .setOptionValueWithSource() in Commander v8.3.0. Lighter touch approach for people rolling their own solutions!

@jakobrosenberg
Copy link

@shadowspawn Sorry for resurrecting this, but I have a question about how to accomplish the following. Is it possible to get the value of install <dir> before running the program?

// pseudo function that grabs the dir value
const installDir = getValue(['install', 'dir'])

// if app is already installed, use existing config as defaults
const defaults = require(installDir + '/config.js') || getDefaults()

program
  .command('install <dir>')
  .option('--secure', 'set to secure', defaults.secure)
  .action((dir, options) => { ... })

@shadowspawn
Copy link
Collaborator

I don't see an easy way to get a subcommand argument without parsing.

A different approach might be to read the local configuration in a .hook('preAction', ...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants