merge is one-level

[gutenye]

Given

const s = StyleSheet.create({                                                   
  red: { 
    '&.active': {                                                               
      width: '100px',                                                           
    }                                                                           
  },                                                                            
  blue: {
    '&.active': {                                                               
      height: '100px',                                                          
    }                                                                           
  },                                                                            
})  

<div className={cx(css(s.red, s.blue), 'active')}>                 

Got

.red-3089191041--blue-1941479325.active {
  height: 100px;
}

Expect

.red-3089191041--blue-1941479325.active {
  width: 100px;
  height: 100px;
}

[kof]

Yeah so you expected a deep merge there, but got a shallow merge.

[kof]

Can be done. Do you like to send a pr?

[gutenye]

I haven't read the source code yet.

[kof]

Not sure though if I want a library for deep merge or just write own simpler version, because libraries handle tons of cases we don't have.

[asvetliakov]

@kof Will you accept PR for this using lodash.merge ?

[kof]

I am not sure its a good idea, in our case here we know what kind of objects we accept, its not as generic as lodash. I guess it will come with a big amount of utility functions and increase the build size a lot.

[kof]

Just installed lodash.merge, it comes with 2200LOC (16kb minified not gzipped), covering all the cases we don't have: array like objects, typed arrays, isPlainObject check (we always have plain object), arguments etc etc.

[kof]

https://www.npmjs.com/package/deep-assign and https://www.npmjs.com/package/deepmerge seem to be better options in our case, I would go for the first one, because I know @sindresorhus makes very high code quality.

[kof]

Btw. the version aphrodite uses is even smaller, because more specific to supported cases: https://github.com/Khan/aphrodite/blob/master/src/util.js#L40

[asvetliakov]

Yes, lodash.merge it's pretty big. 60Kb
what about simplier version then (not tested)?:

function recursiveMerge(val1 = {}, val2 = {}) {
    for (const key of Object.keys(val2)) {
        if (val2.hasOwnProperty(key) {
          val1[key] = (typeof val2[key] === "object" && val2[key] != null) ? recursiveMerge(val1[key],   val2[key]) : val2[key];
      }
    }
    return val1;
}

const finalStyle = rules.map(rule => rule.style).reduce(recurisveMerge, {});

[sindresorhus]

Don't use deep-assign. It's broken and I haven't had time to fix it up. I would also not recommend creating your own version. It's very hard to get it right. Lots of edge-cases. Just use a Lodash method for this. It's high-quality.

[kof]

@sindresorhus thanks for heads up, we have a very predictable use case here, not a generic one. We have only a need to support plain objects and arrays. Basically only JSON subset, no regexp, no arguments etc., what would you do?

[kof]

I think we should go for a custom merge for our case.

[kof]

@gutenye any updates?