101/defaults should either mutate the `target` argument or clone and return an extended object, not both

[cflynn07]

101/defaults currently both mutates it's target argument and returns the mutated target argument. This isn't shown in the documentation. https://github.com/tjmehta/101#defaults

It should either:

1. clone target, extend target, and return target
   or
2. extend target and return undefined (or maybe return a boolean indicating if defaults were applied or not)

Mutating the argument and returning it is redundant.

Edit:
Came across this use of defaults in our code:

// unnecessary to assign return value to opts, implementor was unaware defaults mutates and returns
opts = defaults(opts, { foo: 'bar' });

[bkendall]

Why do you think an implantation change here is required? Is there a documented best practice you're referencing? I honestly don't see any problem with both mutating the target and returning said target - it's the most flexible. If the problem is documentation, we should expand the documentation to be more clear.

An interesting problem is that if you change the behavior to not return anything, you would break any code that resembles:

var arr = [{ id: 1 }, { id: 2 }]
// add defaults to each object
arr = arr.map(defaults({ foo: 'bar' }))
// if defaults didn't return anything, arr would now be [undefined, undefined]

[cflynn07]

I would say my primary issue is the ambiguous documentation of defaults behavior. The existing documentation doesn't indicate clearly if defaults returns or mutates or both.

Second, mutating and returning is redundant. If we're going to mutate, why return? We could use the return value to indicate something else.

If we change the return value that could be tagged as a new major version.

[cflynn07]

arr = arr.map(defaults({ foo: 'bar' }))
// if defaults didn't return anything, arr would now be [undefined, undefined]

I agree, we should not break that functionality. I propose we return a clone, and not mutate the original target argument to defaults

[tjmehta]

Nice thoughts here.
I have a few thoughts on this myself:

Immutability:
Immutable utils are becoming very popular in the JS community as a result of React. The current implementation of defaults is not immutable. There are pros and cons w/ creating immutable utils. It is definitely less flexible (bc a clone would be built into the method, like put vs set). However, if the mainstream use case is to always have a util be immutable, then providing a mutable util creates more work for the user (constantly having to clone their methods).

I have been thinking about this problem for some time, and lately I have been wanting to provide both versions (mutable and immutable) of each util. One idea is to namespace utils 101/immutable/set, but I am not completely sold on it.

Lacking documentation:
We should definitely correct the lacking documentation for defaults.
I really want to create a documentation website for 101 that has a search and filtering methods by tags..
I have ideas in my mind. I want embed something like https://tonicdev.com/npm/101 for each util as well.

[cflynn07]

Good thoughts on the state of mutability/immutability in common libraries in the nodejs ecosystem. I'll try to improve the defaults documentation and get that to you in a PR