nconf icon indicating copy to clipboard operation
nconf copied to clipboard

argv-store should convert the strings "true" and "false" to booleans

Open meaku opened this issue 12 years ago • 10 comments

Example

node app.js --myBool true results in nconf.get('myBool') // 'true'

I think converting boolean-string to real booleans would be nice, because that's what you are expecting - i think the same applies for the env-store.

I will attach a pull request if you agree on my proposal.

meaku avatar Feb 15 '13 18:02 meaku

I agree that parsing "true" and "false" into booleans would be useful. Right now I am using optimist to load any boolean parameters but it would be simpler to just use nconf

nisaacson avatar Apr 02 '13 13:04 nisaacson

What if you'd like myBool (or any variable) to actually be the string "true" though? Some sort of flag at the command-line level would likely be needed, adding complexity.

My suggestion would be to cast it to a bool on your end, like so:

!!nconf.get('myBool'); // => true if truthy, false if falsy.

dguzzo avatar Sep 19 '13 01:09 dguzzo

I think it's more common to use true and false as booleans. It makes more sense to apply the manual cast in case of a string usage of myBool because it would happen less often than the other way round. Don't you agree?

meaku avatar Sep 23 '13 12:09 meaku

In theory you could relegate command line arguments to being flags proper, i.e., booleans, and have environment variables and config file settings' values still be parsed as strings, but you'd probably have to convince the author, Charlie Robbins, that argv should always be booleans, which would break backwards compatibility. It'd probably be worth surveying usage patterns, but I don't think we should assume that command line flags are always meant to be booleans; many CLI programs that are configurable via flags accept various types. Type -h or --help with any number of *nix programs to see what I mean.

I'm not saying I don't sympathize with your want, but perhaps there's some sort of compromise?

dguzzo avatar Sep 24 '13 04:09 dguzzo

I ran into this problem as well, and had to work around this using a trick:

  1. if you would like to overwrite the boolean param to be true, assign it to be 'true' in your command line
  2. if you would like to overwrite the boolean param to be false, assign it to be empty string '' instead of 'false' in your command line

In this way, your app does not require any change at all

niyue avatar Apr 19 '14 09:04 niyue

Or nconf could be updated to use commander, which in addition to being more current and widely used than optimist, has the advantage of having typed arguments with coercion (solving this exact problem). And as a bonus, commander is also not deprecated ;)

BobDickinson avatar Sep 17 '14 21:09 BobDickinson

Any updates on this? Had to deal with this situation myself today.

@BobDickinson (old) suggestion seemed reasonable at the time. I'm throwing yargs into the mix as well.

EDIT: Don't mind me. I noticed nconf is already using yargs.

nfantone avatar Jul 29 '16 19:07 nfantone

Our workaround is that we create a memory store and read the env vars into it (filtering to just pick the ones we want, and converting booleans and null):

conf.use("memory");
var memStore = conf.stores["memory"];
Object.keys(process.env).filter(function (key)
{
    return true; // Filter here as appropriate
}).forEach(function (key) 
{
    var value = process.env[key];
    if (value == "true")
    {
        value = true;
    }
    else if (value == "false")
    {
        value = false;
    }
    else if (value == "null")
    {
        value = null;
    }
    memStore.set(key, value);
});
memStore.readOnly = true;

BobDickinson avatar Jul 29 '16 20:07 BobDickinson

@BobDickinson You could consider using JSON.parse instead of that chain of conditionals.

Also, regarding boolean values, what about using simple yargs flag?

So, instead of --foo=true (which will be parsed as 'foo': 'true'), you could just use --foo and get the result you want. Set the default value to false and that's it.

EDIT: This was suggested before here: https://github.com/indexzero/nconf/issues/72#issuecomment-24974177

In theory you could relegate command line arguments to being flags proper

nfantone avatar Jul 29 '16 20:07 nfantone

JSON.parse() assumes that the value being parsed is valid JSON. I have no idea what values might be in cmd line args or env vars and whether they will be valid JSON. For example, a string with a brace in the middle might be a perfectly valid config value, but will throw an exception if you call JSON.parse on it. And if the string was actually a JSON object, using the output of JSON.parse would change the behavior by parsing that as an object instead of treating it like a string (you might want that behavior, but it's definitely a different contract than the behavior I was trying to fix).

My goal was to fix a specific problem without causing any unintended side-effects, and I prefer to be explicit as opposed to trying to save a couple of lines of code. YMMV.

BobDickinson avatar Jul 30 '16 00:07 BobDickinson