remove initial state of brcast

cssinjs / theming

Unified CSSinJS theming solution for React

300 stars 39 forks source link

remove initial state of brcast #23

Closed oliviertassinari closed 7 years ago

oliviertassinari commented 7 years ago

I'm not sure that it makes sense to provide an initial value here

https://github.com/vesparny/brcast/blob/master/index.js#L1

iamstarkov commented 7 years ago

Why it wouldn’t?

iamstarkov commented 7 years ago

We rely on this feature, check out broken tests

iamstarkov commented 7 years ago

I might miss something, so if it is the case please explain further what did I miss

oliviertassinari commented 7 years ago

You are right, this change is not really relevant. As I mentionned before, I have been upgrading the styling solution on Material-UI. I have started with this lib but ended-up reimplementing it to address three issues:

null theme callback
default theme value
wrong propagation of theme when scoped.

I'm not finish yet with the rewrite. I will ping you on the PR so you can have a look at it and eventually port the changes here. Thanks for making this lib!

iamstarkov commented 7 years ago

Can I ask you to describe aforementioned issues you faced in more details?

oliviertassinari commented 7 years ago

I will, once I have finished the migration. I need to close the circle loop by pushing the changes in production to be sure they are actually something I need and I can consider issue.