search

kodyl / stilr

Encapsulated styling for your javascript components with all the power of javascript and CSS combined.
MIT License
236 stars 16 forks source link

Remove react as a dependency #31

Closed webpapaya closed 8 years ago

webpapaya commented 8 years ago

This PR closes #12

This PR adds the missing pieces in order to be able to remove react as a dependency from stilr. I had a look if react really did some module splitting as @ericelliott proposed but sadly the modules we need aren't split up.

So I decided to reimplement the createMarkup method so we don't rely on react. Before implementing the new create markup I wrote a couple of tests to verify that the new implementation is doing the same things as the old one did. I also went through the source of react to not miss anything.

One big issue I currently see is the lib/properties.js which is used to detect if a value should be suffixed with a px or not.

{opacity: 0} => opacity: 0;
{fontSize: 10} => opacity: 10px;

This list needs to be maintained somehow, because it's currently just copied and pasted from the react source. Therefor the copyright license is added at the top of this file (I have no idea if we need this).

danieljuhl commented 8 years ago

Really a great job! I haven't tested it yet though. But maybe we should put all the markup/CSS related stuff in a separate file?

webpapaya commented 8 years ago

Thanks. Yeah I thought of that as well, but I didn't want to make to many changes to stilr in one PR. I would then also extract those lines 97 and 103 into a renderer.js. Which I think would make sense to have all render related "helpers" in one dedicated file. But I personally would do this in a dedicated PR so we're not making to many changes at once.

What do you think of the about the properties.js should stilr maintain this list on their own or do you know any way we can keep the list in sync with the react guys.

I tested this PR on a small project which is using stilr and couldn't find any issues until now, but it would be awesome if you have a bigger project which is using stilr.

danieljuhl commented 8 years ago

I think this in general is very interesting - and something I have wanted for a long time. Remove React as a dependency, but unfortunately their CSS handling is deeply integrated. I love the work you have done here,

I will make some more feedback very soon, and hopefully we can make a release soon without React.

danieljuhl commented 8 years ago

I will do some tests in the start of next week, on some larger projects, to verify that the output is strictly equal.

webpapaya commented 8 years ago

Resolved merge conflicts. @danieljuhl did you have time to take a look on this PR.

alex3165 commented 8 years ago

Hello, What are the news with this PR ? I am having some issues because of multiple react are in my bundle.

webpapaya commented 8 years ago

As there is no interest in merging this I'm closing this PR.

ddneat commented 7 years ago

@webpapaya @danieljuhl I would really like to see this PR reopened and merged. The stilr API is simple and just awesome to use... let's enhance the dependencies just like that and remove react in favor of having a clean setup.

webpapaya commented 7 years ago

I would checkout https://github.com/Khan/aphrodite it has a similar api and is actively maintained. It seems that it is actually a drop in replacement, even though I didn't try it out yet.

danieljuhl commented 7 years ago

@davidspinat Please have a look in #39 - I'll make a beta release today, which you can test