undefined values are merged

[kcwitt]

Based on the following code:

    const merge = require('merge-options');
    const result = merge({value: 10}, {value: undefined});

result.value should be '10', but it is 'undefined'.

Whether it really should return 10 is arguable, so my argument is that {} is functionally equivalent to {value: undefined} when getting a value from on object, so as inputs to merge they should both provide equivalent output. Where they are not functionally equivalent is when iterating the objects keys, but when dealing with options (and being mindful if the distinction between undefined and null) the rule of least surprise (for me at least) is to completely ignore undefined values (but still allow null values to supersede values of lower precedence).

The practical application of this is putting environment variables (which may or may not be defined) as the highest precedence item in the merge, but only if it has been defined.
const port = merge(defaults, {port: process.env.PORT});

[schnittstabil]

Fair enough, but the readme clearly states:

merge-options considers plain objects as Option Objects, everything else as Option Values.

As undefined is not a plain object, your examples describe the expected behavior.

Please note: the more recent (non-recursive) spread operator works the same way:

const defaults = { port: 443 };
const options = { port: process.env.PORT };

mergeOptions(defaults, options)
// => { port: undefined }

{...defaults, ...options}
// => { port: undefined }

---

Hence, in your case, I would suggest to make your environment variables more useful:

// index.js
const mergeOptions = require('merge-options');

const envKeys = [ 'PORT' ];
const env = Object.fromEntries(Object.entries(process.env)
    .filter(([k]) => envKeys.includes(k))
    .map(([k, v]) => [k.toLowerCase(), v])
);

console.log('env:', env)
const options = mergeOptions({ port: 443 }, env);
console.log('options:', options);

$ PORT=8080 node index.js
# env: { port: '8080' }
# options: { port: '8080' }

$ node index.js
# env: {}
# options: { port: 443 }

[achingbrain]

Would you accept a PR that added this behaviour as a config option similar to concatArrays called ignoreUndefined or similar?

I run into situations where this would be useful all the time but don't think the world needs Yet Another Options Merging Module.

[schnittstabil]

👍 @achingbrain I don't see a problem with an option – ignoreUndefined sounds good to me.

[schnittstabil]

Thanks to @achingbrain (merge-options@^2.0.0):

const mergeOptions = require('merge-options').bind({ignoreUndefined: true});

const result = mergeOptions({port: 443}, {port: process.env.PORT});
console.log(result);

$ PORT=8080 node index.js 
# { port: '8080' }

$ node index.js 
# { port: 443 }