pkgjs / parseargs

Polyfill of `util.parseArgs()`
Apache License 2.0
121 stars 9 forks source link

How might I add support for defaults? (And env?) #82

Closed shadowspawn closed 1 year ago

shadowspawn commented 2 years ago

(This is a bring-your-own-feature experiment, not a prototype of parseArgs implementation.)

Default values is our current canonical example of something that might get added in the future. A complication for some uses of defaults is telling whether the option value came from the user or the defaults. Another feature is support for options coming from environment variables.

Can we add all three of those in user land now?

const { parseArgs } = require('@pkgjs/parseargs');

const options = {
  str: { type: 'string', short: 's' },
  env: { type: 'string', env: 'ENV', short: 'e' },
  def: { type: 'string', default: 'DEFAULT',short: 'd' }
};

const result = parseArgs({ options });

result.valueOrigins = {};

Object.keys(result.values).forEach(optionName => {
  result.valueOrigins[optionName] = 'cli';
});
Object.entries(options)
  .filter(([optionName, detail]) => result.values[optionName] === undefined)
  .forEach(([optionName, detail]) => {
    if (detail.env  && process.env[detail.env] !== undefined) {
        result.values[optionName] = process.env[detail.env];
        result.valueOrigins[optionName] = 'env';
    } else if (detail.default !== undefined) {
        result.values[optionName] = detail.default;
        result.valueOrigins[optionName] = 'default';
    }
  }); 

console.log(result);
% ENV=EEE node index.js --str SSS  ppp 
{
  values: { str: 'SSS', env: 'EEE', def: 'DEFAULT' },
  positionals: [ 'ppp' ],
  valueOrigins: { str: 'cli', env: 'env', def: 'default' }
}
ljharb commented 2 years ago

env vars are traditionally SCREAMING_SNAKE_CASED; i don't think it makes sense for this have any env var awareness whatsoever. ENV-based defaults and hardcoded defaults are conceptually identical.

Something either was provided by the user, or it wasn't. The parseArgs configurer is the one who provided the default; if that default came from env, they know that.

shadowspawn commented 2 years ago

I am guessing your comments are because you expected a simpler defaults-only piece of code?

As an author I layered on some things I wanted to try, and one of the things I wanted to include was a non-boolean description of the value origin. I'm not trying to do polished QA for example code.

ljharb commented 2 years ago

It's because I think the complexity of a non-boolean option isn't warranted, and the complexity of trying to map between NAMES_LIKE_THIS and --names-like-this shouldn't be touched by a core utility with a ten foot pole.

shadowspawn commented 2 years ago

This is a bring-your-own-feature experiment, not a prototype of parseArgs implementation. (I'll add a clarification to original comment.)

It's because I think the complexity of a non-boolean option isn't warranted

What informed my example was being involved in discussions about how to layer config files on top of a parser with defaults and env but not config. People had difference requirements around priorities and whether "multiple" (array) options would add to other value sources or replace them. Making the source of the value explicit gave the author more to work with.

and the complexity of trying to map between NAMES_LIKE_THIS and --names-like-this shouldn't be touched by a core utility with a ten foot pole

I didn't go there. 🙅 Manual configuration is less fraught.

options['names-like-this'] = { type: 'string', env: 'NAMES_LIKE_THIS' }
iansu commented 2 years ago

I'm experimenting with parseArgs (this polyfill, not what landed in Node.js) and missing being able to specify a default value. Now that strict mode is the default if I don't provide a value for one of my values (that naming is a bit confusing) parseArgs just throws and I don't get an opportunity to provide my own default after the fact. I guess I would have to disable strict mode to make this work currently, unless I'm missing something...

bakkot commented 2 years ago

@iansu parseArgs shouldn't throw unless the user passes --opt and then doesn't provide a value for it, which is different from not providing --opt at all. Passing --opt and leaving off the value is generally an error on the part of the user; if they wanted the default value for opt they would not have passed the flag at all.

So doing

let { values } = parseArgs({ args: [], options: { opt: { type: 'string' } } });
console.log(values.opt ?? 'default');

should work fine, strict or not.

shadowspawn commented 2 years ago

Now that strict mode is the default if I don't provide a value for one of my values

Rewording: it sounds like you are not supplying a definition for one of your options. After parsing, the option will either have a parsed value or you can give it a default value (per previous example).

iansu commented 2 years ago

I guess I was missing something 🤦

I did get this to work. Thanks for the help. It would still be nice to add support for defaults as well as required options in the future.

lirantal commented 2 years ago

I'm not particularly fond of coupling environment variables into parsed arguments but I do like the idea of default values, as well as indicating whether the default value has been assigned or not.

shadowspawn commented 2 years ago

Rolling your own support for just defaults is a one-liner. Per https://github.com/pkgjs/parseargs/pull/142#issuecomment-1214175193

let options = { foo: { type: 'string', default: 'bar' }, ...etc };
let { values } = parseArgs({ options });

Object.entries(options).forEach(([k, v]) => { values[k] ??= options[k].default });
shadowspawn commented 1 year ago

Node.js v18.11.0 includes support for defaults. Closing this stale example of bring-your-own-code.