adds option to ignore undefined values

[achingbrain]

It's common to take options arguments and pass some of them on to other functions.

If the incoming option is not set, this can result in passing undefined on to other functions which can cause surprise if the other function sets up its own default values.

function someOtherFunc (options = {}) {
  options.key = options.key || 'default-value'

  console.info(options.key)
}

function someFunc (options = {}) {
  // do some work
  // ...

  return someOtherFunc(mergeOptions({
    key: 'overridden-default-value'
  }, {
    key: options.key
  }))
}

someFunc()
// prints 'default-value'

This PR adds a ignoreUndefined option to not set values to undefined:

function someFunc (options = {}) {
  // do some work
  // ...

  return someOtherFunc(mergeOptions.call({ ignoreUndefined: true }, {
    key: 'overridden-default-value'
  }, {
    key: options.key
  }))
}

someFunc()
// prints 'overridden-default-value'

Fixes #13

[coveralls]

Coverage remained the same at 100.0% when pulling dc00af7 on achingbrain:add-ignore-undefined-config-option into 37dc96c on schnittstabil:master.

[coveralls]

Coverage remained the same at 100.0% when pulling dc00af7 on achingbrain:add-ignore-undefined-config-option into 37dc96c on schnittstabil:master.

[coveralls]

Coverage remained the same at 100.0% when pulling dc00af7 on achingbrain:add-ignore-undefined-config-option into 37dc96c on schnittstabil:master.

[coveralls]

Coverage remained the same at 100.0% when pulling e1e89f8 on achingbrain:add-ignore-undefined-config-option into 37dc96c on schnittstabil:master.

[achingbrain]

I would consider defaulting this option to true as it's by far the more common use case (for me) when passing options around (e.g. respect null, ignore undefined).

I'd argue for it to be false if this was a general-purpose object-merging module but given the name I don't think it is.

[schnittstabil]

Even though I understand your reasoning for defaulting to true, it is more important for me to be backward-compatible and compatible to the more recent (non-recursive) spread operator.

[achingbrain]

I think the spread operator is trying to be closer to general-purpose object-merging than merging options specifically - there are plenty of modules out there already that do that recursively.

As for backwards compatibility, you can just release a major.

Up to you though.

[schnittstabil]

I have to publish a major anyway, because of node>=8.

Regarding your example above, I would argue that the common use case looks like this, which already works at v1.0.0 as expected:

const mergeOptions = require('merge-options');

function someFunc (options = {}) {
  return someOtherFunc(mergeOptions({
    key: 'overridden-default-value'
  }, options))
}

Please note that you still may bind custom options, which may mitigate your concerns:

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

mergeOptions({foo: 'bar'}, {foo: undefined})
//=> {foo: 'bar'}

[schnittstabil]

[email protected] is out now.