-
Notifications
You must be signed in to change notification settings - Fork 1
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
Learnings from JSS #5
Comments
@kof Definitely, this project started out very much as an experiment (and still is somewhat), |
I am interested in adding what is needed, to allow material-ui use it. I believe material-ui requirements can't be so different. A big deal breaker was the SSR (undeterministic id's) which is changed now. |
I think the entire css in js community would profit if we combine our forces. The amount of work is sooo huge that nobody can do that all alone. I have recently added a roadmap to jss https://github.com/cssinjs/jss#roadmap which reflects the biggest challenges in the project. |
@kof I agree that it would be great to pool effort towards a project. I'm still intending on writing an overview of the requirements/difficulties/findings I encountered trying to find a solution for material ui with One or two of the requirements might not be considered a "best fit" for css in js, but when the library ( I'm in my last week at my current job and things are suddenly very busy, but I'll write out more on the weekend. I'm off next week. |
There are a few difficult issues that I encountered trying to rig up a solution for An important thing to keep in mind while reading this is that the requirements of I think you'll find that some of the design decisions in We really want to make CSS in JS work for us and our users. createStyleSheet and theming
Whatever Right now, Classnames/selectorsWe have a lot of users who use regular CSS. They expect to be able to inspect their application, copy a className and override styles. This means a couple of things: A) Selectors need to be identifiable and ideally have some indication of the component they are from B is likely very different to how a lot of CSS in JS libraries are handling selectors, but it's fairly central to being able to provide the type of API that In order to differentiate selectors there are several mechanisms:
Specificity, passing classes, etc.This is a big challenge for all CSS in JS libraries. It's also fairly critical to provide decent override mechanisms for users when using CSS in JS inside a library. A user should be able to just pass their own class and as long as the selector is of the same specificity, it should override it. Users of the library should also be able to leverage the same CSS in JS solution built into There are several design decisions in place to help accomodate this without too much mess with something like individual stylesheet priority if you have a huge number of stylesheets (1 per component, for example). By default, there is only a single This still doesn't solve specificity problems within a single stylesheet element ("group") though, and adding a priority system for individual "sheets" is not something I wanted to do. Instead I've added support for descendant selectors, this allows you to leverage CSS specificity to pass a classname into a nested component and override the styles without worrying about style order. Sure, it won't help if you're trying to override the root node, but it does help with a lot of use cases. I have more to add, but wanted to reply with something to get the discussion going! |
Yes theming for react is a missing part in react-jss, however jss itself is ready, you can do a lot of things with it, for e.g. I am experimenting right now with aphrodisiac
Yes, one way is to have component name in every selector, another way is to have component name in a style tag attribute. As we have style tag per component. The last one is a matter of passing meta property when creating sheets. Also when people use react, they can use it's dev tool to identify the component. This has been no problem so far.
Need more context to understand use cases, don't get it. Are you talking about at-runtime rule style modifications which should not affect the generated selector? If yes it is already this way.
It needs to be ensured that users style tag or link tag is later on in the dom. This might require a small change in jss but not a big deal. As long jss generated selectors are flat (no nesting), they are easily overwritable. Being able to choose between using classes or styles objects to theme a component is exactly what I am trying to accomplish by theme-standard which needs a lot of documentation as of now, @rofrischmann works on adding examples and validator library, the last one is needed to ensure the theme passed by user is what component expects and provide meaningful errors.
For this, theme-standard has subthemes, which allows to nest themes same way we nest components.
This can be achieved with style-per-component approach as well by inserting generated styles one after another and putting the first one before any link/style appearance in the head. Also one style for everybody can be achieved, see aphrodisiac
You mean the passing style element when creating sheet to the renderer, so that it takes your existing one which is placed to ensure specificity, right? You and me added this to jss some time ago. CSS specificityThe general rule I can define for sure to avoid this kind of problems is: "Don't use multiple classes on one element, especially if they are from different sources.". As the library requirements are to provide class based theming as well as style object based. You run into specificity conflicts if
I think both points can be achieved. All in all I haven't seen so far big jss specific problems. It is all around react integration and theming. I think you need to distinguish the rendering part and the component level part. |
Created this issue for specificity problem. |
I'd be happy to see you both join forces, instead of introducing yet another styling solution. Both projects are just too similar. I'm currently using JSS to style my components and actually I decided to go the opposite way and defined a priority for each stylesheet. So far it went straight forward and wasn't "messy". Let me tell you how I do it, maybe it can give you some inspiration. All my stylesheets are registered into a theme object and receive an implicit priority according to the order of when they were registered. I use a custom DOM renderer that obeys to the priorities when rendering the stylesheets. This is pretty simple and has so far solved all styling issues I encountered with other css based styling solutions. I used |
@cvle how to you define priorities for your sheets? Any code samples? |
@cvle lets move priority discussion to a separate issue in jss project, because this thread should be focused. I think I could add something to jss to allow manual priority definition. |
Regarding descendant, sibling and other kinds of nested selectors: they are all considered harmful. They break components encapsulation. The only way they can be used correctly is defining margins and positions/layouting on a child component, which should not be of concern of the child. But lets not go deeper into this. |
You're 100% correct, although I was having to use it in an unnatural way before to achieve certain goals. Quick note re the descendant selectors: I completely agree they should be used sparingly, but I think there are more use cases where the extra specificity can make something work much more easily. For example, I'm yet to find a better solution for handling use cases where something such as a color needs to be changed to fit in with the background. Let's say you're a user, you create yourself an icon component which has a grey color and you decide to place it on a dark blue toolbar and want to color the icon white instead (or any colour combination for that matter). What would you suggest as the className based solution here? All the obvious methods involve some sort of broken encapsulation because of how CSS is designed.
Yes, but that was to target for server side rendering reconciliation with the sheets, it doesn't help keep component styles grouped together when more sheets start getting rendered. Or are you suggesting to use only 1
I know that here we are mixing the concerns of components + rendering. But this is exactly what I'm talking about when I say that there are design decisions that are catering towards There is an option here to get rid of these mixed concerns and stipulations about className generation: make
Nope, was talking about actual changes to the rule in the component file. This ties in directly with what I was saying above about providing an API that is friendly for users that use CSS in a more basic fashion. It's probably confusing because it isn't the route that makes the most sense outside of this use case. I'm really starting to think that we should completely drop this idea and keep selectors as an implementation detail. For the future I definitely think the group of users that the design decision helps will shrink. Bottom Line@kof The attempt to make selectors a public API is holding the styling solution If we drop that requirement, it's going to be a lot easier to use |
Do you see the ideal way for |
Icon needs to accept a class name which can overwrite props from the system styles on the proper element. However this gives to user ability to overwrite anything else on that element, so yes this is not ideal and can't be controlled, but will certainly work for the user with some caution.
I would rather make renderer place jss styles one after another in some specified order, so that they are grouped. 1 sheet is also a possible way, however I like the 1:1 component-sheet relation, it is easy to see in the dom the entire sheet for one component.
Oh I didn't realize that. I missed the point that you actually want to provide components with global classes. I thought the way you want to allow use regular css with components is to let user pass class names to components, which are then applied. This would fit also css-modules. const theme = {
classes: {
someElement: '.my-very-special-selector'
}
} I don't know if we can convince the material-ui community that global classes are evil. But in any case user doesn't need to access generated classes at all. If you want to provide global classes additionally, you can put them additionally to generated classes on the element. This is even easier to achieve because user doesn't need to pass classnames maps. But this is again ... evil. <button className={`${generatedClass} myButton`}>My button</button>
Totally. I think the component integration you have made will be perfectly able to use jss as a rendering engine. |
I pretty much agree with @kof. I can't see why we would need to expose global class names at all.
That would pretty close to how
I'm not sure it's the best approach, but yes, that's also something we could do 🙈 . |
The more I consider it, the more I think that making the All components must be able to accept Honestly though, it would still be a significant improvements for CSS users over what we have right now: inline styles.
Are you thinking a basic priority system? IIRC you were not in favor of one previously, what has changed your mind?
By component integration -- are you referring to the react/theme/context setup with |
@nathanmarks I'm curious to know what you see as the downsides when using priority based rendering. A components author would have complete control over which styles overrides which. User styles can be given higher priority to allow easy overriding. I don't know |
@cyle at first I wasn't certain but I've changed my mind about it. it's worth it given the nature of CSS and could offer a lot of power. In the end we have to use the features CSS offers to help solve the problems at hand. |
@cvle I atted the wrong person above! Also, I am using sheet ordering currently in this lib too in a basic way, not using a priority system. But I'm sold on the priority system idea for JSS. Would help solve a lot of the problems with ordering and specificity without having to use more complex selectors to begin with. |
Related issue: cssinjs/jss#290 |
@nathanmarks Now you have taken my strongest reasons for writing my own complete set of material based components, given that now we are taking a very similar approach :-) (except that I write in typescript). One thing I'm trying to solve atm is to easily customize the style of deeply nested components. Instead of passing multiple stylesheet references as props, I'm playing around with uniquely prefixing the css classes of every component, so I can pass a single user stylesheet to customize the style of every nested component. Additionally I'll play around with descendants selectors to customize child components of the same class with different styles (thanks for that idea). Hoping to spark some ideas for you too. If you include a good solution for customizing deeply nested components, and include auto- mounting/unmounting of stylesheets, I'm totally sold for the next version of |
lol i don't know what's going on here but i'll just cc @kmck on this guy because this repo looks familiar |
In your opinion, what are the biggest benefits of unmounting stylesheets when components are unmounted? Originally I was a fan of the technique, but I started to wonder if the potential benefits are worth the risk. Sure, we end up with a little less CSS in the DOM once the component is unmounted, but what is the real performance gain from that? On the other hand, the risk is that you have a component that comes in and out frequently and you are repeatedly adding and removing the entire sheet from the DOM, meaning the browser has to rebuild the CSSOM (unless I am misunderstanding something). |
@cvle By the way, there will be less nested components on |
oh hey cool i was randomly tagged into this. neat project! i made a webpack loader that does kind of a similar thing, but i'm optimizing for a different use case (i think). here it is, in case anyone is curious: https://github.com/kmck/style-component-loader |
@nathanmarks You have good arguments, appreciated. |
I didn't have the use case where user wants to overwrite jss styles from css by using global classes.
yes
My original assumption was that more css should slow down elements creation, as logically DOM needs to run every element agains all registered selectors. However I didn't came to prove that by real benchmarks, I don't even know how to accurately measure this. Here is the corresponding issue, any help appreciated.
Do you have a use case? |
I started on a prototype implementation using |
Actually even in this case, sheet will not be rerendered, just attached/detached the style node ... most probably there is almost no overhead. |
@kof I think with either technique, only edge cases are going to have any sort of real performance overhead. |
Hi Nathan, we have been talking a lot about JSS and you seemed to be prototyping a material-ui integration with JSS, however you ended up with an own similar implementation. Can you share the learnings you have done and especially the reasons which stopped you from using jss as is?
The text was updated successfully, but these errors were encountered: