Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Aphrodite , glamor, and styled to v10 #1095

Closed
jeremy-coleman opened this issue May 3, 2019 · 14 comments
Closed

Update Aphrodite , glamor, and styled to v10 #1095

jeremy-coleman opened this issue May 3, 2019 · 14 comments

Comments

@jeremy-coleman
Copy link

jeremy-coleman commented May 3, 2019

ive updated aphrodite glamor and styled with some small changes to use jss10 , do these fall under the scope of the monorepo?

Are you willing to implement it?
Yes

The glamor impl wouldnt work without removing the original versions selector normalization plugin.

Aphrodite version works mostly fine, i made several variants id like some feedback on

Styled version i removed the theming v1.3 and related code, i also removed attribute checking but can easily add it back in 1 line

Would someone like to look it over in a test repo or something?

@kof
Copy link
Member

kof commented May 3, 2019

I am working on a new styled interface implementation #1094
Also we are working on a hooks based api #1089
Regarding aphoridte api - currently I don't have any bandwidth for it.
Glamor API - where did you find it on this org?

@jeremy-coleman
Copy link
Author

jeremy-coleman commented May 3, 2019

The glamor impl isnt from the jss org but i think it has its place. The big benefit is that is uses data selectors so u dont have to worry about over/under writing classname or style.

For the aphrodite version, I also put the stylesheet creator inside a class constructor , with 3 different create() equivalents for overriding the classname and optional css() call built in . I can easily add a styled() call to create react components also if thatd be useful - one thing i dont understand is why to remove the stylesheet on component unmount?

I also wanted to explore adding a static theme property on the class , (which would be shared across all instances) . This way, no need for react context at all

@kof
Copy link
Member

kof commented May 4, 2019

The big benefit is that is uses data selectors

Not sure what you mean by that

why to remove the stylesheet on component unmount

A large CSSOM at runtime can negatively impact the rendering, it needs to be quite big though for this. Also there have been some limits in older browser about amount of total CSS one can render.

This way, no need for react context at all

We use context because it gives us a way to change the theme and trigger a rerender. Without a need for this just import a static theme from a module.

@HenriBeck
Copy link
Member

I would also be up for importing the styled API into this monorepo as we are doing in #1094

I don't think the other to syntaxes were previous versions of a css-in-js implementation where the community has moved away.

@kof
Copy link
Member

kof commented May 4, 2019

The styled API in #1094 will be as close as possible to emotion/SC. Also some other parts in the old styled are simply not needed because we assume one can use injectSheet/new hooks api in such cases.

@jeremy-coleman
Copy link
Author

By data selector, i mean the generateRuleId uses data-(ruleName) attributes on the target element instead of className

@kof
Copy link
Member

kof commented May 5, 2019

How is that a big benefit, I don't get it @jeremy-coleman

@jeremy-coleman
Copy link
Author

jeremy-coleman commented May 7, 2019

Because you dont have to deal with over riding class names. The write logic can be used with any api. It lets you write <div {...mystyles}/>

@kof
Copy link
Member

kof commented May 7, 2019

You mean in case where a class name is used otherwise, you can use data attribute and not have to concatenate classes, is that it?

@jeremy-coleman
Copy link
Author

Yes, anyway, let me know which impl’s you want , if you have slack or something i can show you a few different versions and discuss which features should be in the final api

@kof
Copy link
Member

kof commented May 8, 2019

Our focus is right now the new hooks api, the new styled api and releasing stable v10

@kof
Copy link
Member

kof commented May 8, 2019

So if you are using glamor/aphrodite implementation feel free to update them to v10 in the corresponding repos.

@jeremy-coleman
Copy link
Author

jeremy-coleman commented May 11, 2019

I was testing the aphrodite impl yesterday, and cannot figure out what the problem is... i even tried switching out the original merge and generate functions with the ones from the mui jss implementation and still got parse warnings. stuck:\ I know it shouldn't be required to work with mui , but probably half the users would want to do that.

@kof
Copy link
Member

kof commented May 11, 2019

@jeremy-coleman ping me on gitter with details

@kof kof closed this as completed Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants