Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deepmerge not working with Set #202

Open
itsdarrylnorris opened this issue Jul 31, 2020 · 6 comments
Open

deepmerge not working with Set #202

itsdarrylnorris opened this issue Jul 31, 2020 · 6 comments

Comments

@itsdarrylnorris
Copy link

Problem

I am running into a problem using the JavaScript Set function that after merging two objects for some reason it's not keeping the array from set.

Here is the code:

var merge = require("deepmerge");
var valuesWithSet = {
  testObject: new Set(["hello", "world"]),
};

console.log("valuesWithSet", valuesWithSet);
console.log(merge({}, valuesWithSet));

Output:

valuesWithSet { testObject: Set(2) { 'hello', 'world' } }
{ testObject: {} } // This should have array from set.

Solution

We need to keep values from set. The output should looks like this:

{ testObject: Set(2) { 'hello', 'world' } }
@TehShrike
Copy link
Owner

The current version of deepmerge is still using this naive object detection (typeof new Set() returns 'object').

You probably want to pass is-plain-obj in to isMergeableObject, per this part of the readme.

This will cause the set to be used directly on the new object (not cloned).

Related: #152 which is fixed in the v5 branch

@Gaurav2048
Copy link

Can also use Array.from(#set) to get an array and then pass to deepmerge. #TehSrike do you suggest doing it in the library?

@RebeccaStevens
Copy link

@Gaurav2048 are you asking about having the library automatically convert sets to arrays? If so I don't think this should be supported as that would change the structure of the resulting object. I opened another issue (#204) a little while ago for handling Set and Map merging.

@TehShrike
Copy link
Owner

If you want that kind of behavior, you would probably want to use klona in your customMerge function.

@RebeccaStevens
Copy link

@TehShrike I think it might be worth having a customClone option in addition to the customMerge option. This library's current built in clone method fails when it tries to clone non plain objects. The option would also address the concerns raised in #163.

@TehShrike
Copy link
Owner

There might be traps in there somewhere but my initial reaction is that makes sense to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants