TehShrike / deepmerge

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

Deepmerge deletes values of ArrayBuffer and replaces to an empty object #232

Closed rakhmanoff closed 3 years ago

rakhmanoff commented 3 years ago

Issue type: bug

Hello,

I've used ngrx-store-localstorage package for my Angular application, it depends on deepmerge functionality.

Bug description: If one of the source objects contains a field with type "ArrayBuffer", it will be replaced with an empty object in resulting object. It is an unexpected behaviour. If I use Object.assign for the same action, then field with ArrayBuffer preserves, it is expected.

I've created a simple JSFiddle with an example: https://jsfiddle.net/2bat0oeh/17/ Also you can see the code and results right here:

const regularObject = {
  names: ['Peter', 'Jack']
};

const buffer = new ArrayBuffer(16);
const bufferView = new DataView(buffer);
bufferView.setInt8(12, 42); // put 42 in slot 12
const objectWithArrayBuffer = {
  buffer: buffer,
};

// call deepmerge and check the results: ArrayBuffer is lost
const deepMergeResultsWithArrayBuffer = deepmerge(regularObject, objectWithArrayBuffer);

console.log('Deepmerge with ArrayBuffer replaces it with {} in resulting object', 
    deepMergeResultsWithArrayBuffer);
console.log('Type of buffer field should be ArrayBuffer, is it true?: ',
    deepMergeResultsWithArrayBuffer.buffer instanceof ArrayBuffer);

// call Object.assign and check the results: ArrayBuffer is still here
const mergeResultsWithObjectAssign = Object.assign({}, regularObject, objectWithArrayBuffer);

console.log('Object.assign with ArrayBuffer keeps ArrayBuffer as it is',
    mergeResultsWithObjectAssign);
console.log('Type of buffer field should be ArrayBuffer, is it true?: ',
    mergeResultsWithObjectAssign.buffer instanceof ArrayBuffer);

Results from console:

Deepmerge with ArrayBuffer replaces it with {} in resulting object {names: Array(2), buffer: {…}}
Type of buffer field should be ArrayBuffer, is it true?:  false

Object.assign with ArrayBuffer keeps ArrayBuffer as it is {names: Array(2), buffer: ArrayBuffer(16)}
Type of buffer field should be ArrayBuffer, is it true?:  true
RebeccaStevens commented 3 years ago

It should work if you set the clone option to false.

const deepMergeResultsWithArrayBuffer = deepmerge(regularObject, objectWithArrayBuffer, { clone: false });
rakhmanoff commented 3 years ago

clone: false option works, thanks. According to the docs (clone option) it is deprecated. Do you plan to delete it in the future version and set value to the current default value true, so this functionality won't be supported anymore?

RebeccaStevens commented 3 years ago

Currently the plan for the clone option is to undeprecate it and make false the default behavior.

I'd also recommend setting the isMergeableObject option to the is-plain-object function. This is also planned to become the default behavior in the next version.

rakhmanoff commented 3 years ago

Thank you for the information!