Fix yarn config set for boolean values (#1237)

[jsynowiec]

Summary

All config values were treated as strings but some config keys require booleans, e.g. strict-ssl or ignore-scripts. See: #1237

Test plan

1. Set strict-ssl to false
2. Run yarn config list
3. Verify if strict-ssl is set to boolean false

[sebmck]

Can you change this to `val === 'false' || val === 'true', it's not super obvious what this does on first glance.

[jsynowiec]

It won't work for upper/mixed-case letters, and simply usingval.toLowerCase() === 'false' || val.toLowerCase() === 'true' (or introducing another, intermediate variable to hold the lowercase value) will break on the default true value. The typeof check is required.

The regex can be replaced, e.g.

if (typeof val === 'string' && (val.toLowerCase() === 'false' || val.toLowerCase() === 'true')) {

but it's ugly :/

[sebmck]

Not a fan of the IIFE and spreading the arguments into it, we should do something like:

const key = args[0];
let val = args[1];

if (key === 'true' ...) {
   val = val === 'true';
}

We don't really need to handle the alternate casing of true.

[jsynowiec]

const key = args[0];
let val = args[1];

You'll loose the default true value this way from the val assignment (I didn't want to introduce a regression). It could be replaced with let val = args[1] || true but it's not the same as a default value (falsy values don't work, most importantly 0), hence IIFE.

if (key === 'true' ...) {
   val = val === 'true';
}

See my previous comment.

We don't really need to handle the alternate casing of true.

What about calling config set without the value? AFAIK, NPM defaults to true for flags and an empty string for other keys?

[timdp]

This also affects npm_config_* environment variables, right? npm_config_production=false yarn is being treated as yarn --prod for me because !!'false' is true.

[sebmck]

Created #1590 to fix this, pending approval.