Skip to content
This repository has been archived by the owner on Jan 23, 2021. It is now read-only.

Overwriting array properties. #13

Closed
bobrenjc93 opened this issue Jan 13, 2016 · 4 comments
Closed

Overwriting array properties. #13

bobrenjc93 opened this issue Jan 13, 2016 · 4 comments

Comments

@bobrenjc93
Copy link

Is this intended behavior?

deepAssign({a: [1]}, {a: []}) => Object {a: Array[1]}

Based on the behavior of Object.assign (this module is just a recursive version of it correct?), I would have assumed it should have returned Object {a: Array[0]}

@schnittstabil
Copy link
Contributor

@bobrenjc93 In my opinion that is the expected behavior, because we Recursively Object.assign():

deep_assign({a: [1]}, {a: []})

// should be similar to
{a: deep_assign([1], [])}

// which have to be the same as 
{a: Object.assign([1], [])}

(If Object {a: Array[1]} means {a: [1]})

This is consistent to the current undefined behavior.

@bobrenjc93
Copy link
Author

I completely understand the recursive nature of this function. The question is what should be the base case. In my opinion, I think it only makes to recurse further if the value is a true object and not an array. In the event the value is not an object, the recursion should halt and behave the same way you'd expect a non recursive Object.assign for that particular value.

Perhaps it would be good to have the context under which this module was developed. I'm currently trying to use this module for a redux application and the behavior I described above was what I would expect in a recursive Object.assign module.

@schnittstabil
Copy link
Contributor

I think it only makes to recurse further if the value is a true object and not an array.

True Object is the crux of the matter: e.g Javascript also considers functions to be objects, and there seems no generally accepted definition for True Objects.

npm and bower contain myriad packages for merging/extending/… objects – most of them without a precise semantic. If you agree with me that Plain Objects are the one and only True Objects, schnittstabil/merge-options may meet your requirements.

As Object.assign does not distinguish Objects and Arrays I see no justification to do so in deep-assign.

@bobrenjc93
Copy link
Author

Ah okay makes sense. merge-options is exactly what I need, I'll go use that. Thanks!

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

No branches or pull requests

2 participants