unclechu / node-deep-extend

Recursive extend module
MIT License
202 stars 53 forks source link

If two original values are linked to one object, the result will copy first object to both values #33

Closed lerayne closed 7 years ago

lerayne commented 7 years ago

Example:

original object, two fields, fontValueTitle and fontValueSubitle are pointing to one object

const genericFonts = [
  {
    "family": "serif",
  }, {
    "family": "sans-serif",
  },
]

const configDefaults = {
    fontValueTitle: genericFonts[0],
    fontValueSubitle: genericFonts[0],
}

then I'm doning this:

let config = {
    fontValueTitle: {
        family:"customFont1"
    }, 
    fontValueSubtitle: {
        family:"customFont2"
    }
}
config = deepExtend({...configDefaults}, config)

and the output is

{
    fontValueTitle: {
        family:"customFont2"
    }, 
    fontValueSubtitle: {
        family:"customFont2"
    }
}
mandrichenko commented 7 years ago

Any updates here?

unclechu commented 7 years ago

@lerayne @mandrichenko I'm not sure if it is a bug, and this behavior might not be obvious for you but I think it's correct. Look, you specify target object in first argument which one you want to extend, not to clone, just extend (if you want to clone it you specify new empty object as first argument like this deepExtend({}, foo, bar), so, it is not supposed to be cloning any branches, just add or replace some values, and it doesn't care about if some branch is the same object as another branch.

In your case you expect it to be cloned, so, correct usage would be:

config = deepExtend({}, {...configDefaults}, config)

And keep in mind when you writing in javascript that spread-operator do cloning just on the same level, skipping any child-object branches.

unclechu commented 7 years ago

I just added a test to demonstrate it: https://github.com/unclechu/node-deep-extend/commit/ffe04c39a4d0c58044789592fe7b686a3bfe44b5