TehShrike / deepmerge

A library for deep (recursive) merging of Javascript objects
MIT License
2.75k stars 216 forks source link

Please do not remove clone option #163

Open klis87 opened 4 years ago

klis87 commented 4 years ago

Thx for great library, it works great. The only thing I am worried about is deprecating clone. I am using this library for merging 2 states inside redux. If some path of a merged object was not affected by merging, I need this object not to be copied, as then reference stays the same which is very important for optimisations like PureComponent in React or in reselect. To better put what I am talking about:

it('should not clone unchanged objects', () => {
    const x = { y: { nested: 1, deeplyNested: { a: 100 } }, z: { nested: 2 } };
    const x2 = { y: { deeplyNested: { a: 101 } } };
    const x3 = merge(x, x2, { clone: false });
    expect(x3).toEqual({
      y: { nested: 1, deeplyNested: { a: 101 } },
      z: { nested: 2 },
    });
    expect(x).toEqual({
      y: { nested: 1, deeplyNested: { a: 100 } },
      z: { nested: 2 },
    });
    expect(x2).toEqual({ y: { deeplyNested: { a: 101 } } });
    expect(x.z).toBe(x3.z); // this would fail without clone option!
    expect(x.y).not.toBe(x3.y);
});
rrameshkumar76 commented 4 years ago

The options setting for clone to false is also very useful for us when we merge large json structures for performance reasons.

manuFL commented 4 years ago

Totally agree, I'm using react too... can you kindly explain the reasons why this has been set as deprecated? Thank you in advance

RebeccaStevens commented 3 years ago

I think the clone option should be extended to take a function that defines how things are cloned. In the code I gave in #204 I had to set to clone to false so that this lib wouldn't try to clone sets and maps like normal objects. Ideally, I'd like to tell tell it how to clone these types rather than telling it not to clone all together.