Regression in 2.0.0

[sindresorhus]

This commit b55d7b6 seems to cause problem in XO.

I've added a failing test: efa61ab

It seems at deep level it no longer creates a new object but reuses the existing one.

@schnittstabil Could you take a look?

[schnittstabil]

That happens, because we do not recurse into objects in v2, which may be configurable by introducing options (as discussed in #4):

    const fixture = {
        foo: {
            bar: false
        }
    };

    const run = x => {
        const opts = fn({}, fixture);
        //=> opts.foo === fixture.foo

        if (x === true) {
            opts.foo.bar = true;
            //=> opts.foo.bar === fixture.foo.bar === true
        }

        return opts.foo.bar;
    };

    t.true(run(true));
    //=> fixture.foo.bar === true
    t.false(run());

[sindresorhus]

we do not recurse into objects in v2

I must have missed that in the original discussion... But what's the point then of deep assign, if it doesn't assign deeply? It should really be the default behavior.

[schnittstabil]

deep-assign recurses, if there is a corresponding target property:

function User(name) {
  this.name = name;
}

var target = {
    user: new User('Alice')
};

var source = {
    user: new User('Bob')
};

// target.user exists, so deep-assign steps into target.user and source.user:
deepAssign(target, source);
//=> target.user.name === source.user.name === 'Bob'
// but: target.user !== source.user

However, deep-assign simply sets the target property, if there is no corresponding property:

var target = {
};

var source = {
    user: new User('Bob')
};

deepAssign(target, source);
//=> target.user === source.user

The reason for this is that we can not clone arbitrary source objects in a sensible way, e.g. objects with a prototype chain. (Also functions are objects too.)

[schnittstabil]

deep-assign is used as a option-merge-tool in XO. I think that will sooner or later be in conflict with deep-assigns main intention:

Recursive Object.assign()

An example:

const DEFAULT_CONFIG = {
  filter: () => true,
};

const opts = {
  filter: n => n % 2,
};

var config = deepAssign(DEFAULT_CONFIG, opts);
//=> config.filter === DEFAULT_CONFIG.filter

That's because all properties of opts.filter are copied to DEFAULT_CONFIG.filter 😒

I would suggest to create a new project for this, with defaults like array-slicing, plain-object-cloning, function-overwriting etc. I would be happy to help.

[schnittstabil]

@sindresorhus I started working on a option-merge-tool

[ruffle1986]

what's the problem with this? https://www.npmjs.com/package/deep-extend

[schnittstabil]

@ruffle1986 For most use cases: nothing - But I want some other semantics, e.g. the following doesn't seems reasonable to me:

deepExtend({}, {p: Promise.resolve(42)})
//=> { p: {} }

EDIT: But most importantly deep-extends copies buffers - I think that is not very sensible for merging options.

[schnittstabil]

I've just published merge-options, which is based on is-plain-obj.
@sindresorhus @ruffle1986 @coderhaoxin @sugarshin: reviews, comments and feature request are welcome

[etiktin]

I was trying to use deep-assign to merge a user provided options object with my defaults and encountered an issue similar to this #6 (comment) (function from defaults wasn't replaced by the function from options). Ended up, trying out merge-options and it worked well for me.
I think the readme should contain some examples of the edge cases and a link to merge-options in the related section.

[davej]

Yeah, I got caught by this also.

Probably should have been more vigilant but a quick mention of the issue might be useful in the Readme since it arguably deviates from the description of "Recursive Object.assign()".

[sindresorhus]

This module is now deprecated: b332062