[Core] Removes mergeAndPrefix(), refactor uses mergeStyles() and prepareStyles()

[newoga]

This commit removes all instances of mergeAndPrefix. It also aims to use mergeStyles and prepareStyles in a more consistent manner. All uses of mergeAndPrefix were switched to mergeStyles OR prepareStyles depending on the following condition:

• If the computed style being is passed directly to a html tag's style attribute (ie. div, span, li, etc.) OR an externally managed component's style prop (such as ReactTransitionGroup), use prepareStyles
• If the computed style being passed to an internal component's style prop, or any related style props, use mergeStyles

This prevents any top level/parent components from prematurely preparing styles (styles should only be prepared when attaching to a DOM node).

I did searches across src and found places where that pattern were not followed and switched them as well.

[newoga]

@alitaheri @oliviertassinari Please take your time on this one and review/test very carefully. I want to start refactoring styleUtils so that we can work on removing the style-propable mixin. I also want to see if there are any performance optimizations (like memoization) with our use of the merge and prepare functions.

I think removing mergeAndPrefix is the first step.

Related to #2852

[oliviertassinari]

@newoga I will review it. That's a very good first step 👍.

[newoga]

@oliviertassinari @alitaheri What are your thoughts on switching prepareStyles to not optionally accept an array of styles that can be merged. To me this is kind of confusing and abuses single responsibility principle a little bit.

I think it's more explicit to use merge only when you want to merge styles together, then use prepare when you want to take a merged result and perform "final" framework related modifications to it like auto prefixing or direction changing. Therefore, the method signatures would look like this:

function merge(...args) {
  // impl
}

function prepare(muiTheme, style) {
  // impl
}

// If you want to merge themes while preparing, do:
prepare(muiTheme, merge(styleA, styleB));

We don't have to do in in this PR, but wanted to get your feedback on it either way.

[alitaheri]

@newoga Good work 👍 Thank you ^_^

We don't have to do in in this PR, but wanted to get your feedback on it either way.

I like this idea very much! Yeah I think this is how it should be.

[oliviertassinari]

CardMedia doesn't seem to take Material Ui component. Shouldn't we use prepareStyles?

[newoga]

Yeah I think you're right, my mistake!

Thanks for catching. 😄

[oliviertassinari]

@newoga Nice work, I have checked every components of the docs, looking for You're calling ensureDirection() on the same style object twice. (occurs when prepareStyles is called twice).
I couldn't find one 👍.

[newoga]

@oliviertassinari @alitaheri Thanks much for your review!

This is just the first step, if everything looks good with just the overall removal of mergeAndPrefix and some of the general cleanup of mergeStyles and prepareStyles, I say we can merge and I'll get a new PR with a utils/style.js refactor in short order 😄

I'm going to squash the couple of small merge and prepare fixes that we found now.

[alitaheri]

@newoga Thank you. This is certainly a great step toward better code quality 👍 👍 👍

[newoga]

@alitaheri No problem, slowly but surely we will get there! 😄

@alitaheri @oliviertassinari
I squashed the commits and reset style-propable.js back to what it was originally but added a deprecation warning to the mergeAndPrefix() method. If all is good, feel free to merge!

[alitaheri]

👍 👍 Thank you @newoga