Open pristas-peter opened 8 years ago
It is an interesting thought, but goes against much of what Radium was designed to do, which is to be a pure inline solution. StyleKeeper is a necessary evil, and should only be used when absolutely necessary. There are already subtle bugs with precedence and overriding once you introduce actual CSS.
I meant to still support only the hover, fpcus and active, the purpose was to let the browser do its magic behind this and remove the delay when the site bootsraps from SSR.
Yes, that would make sense. I'm also unsure of the performance implications of addCSS
and the StyleKeeper
modifying a style sheet. The good news is that with Radium's modular architecture, it should be trivial to create a version of the resolveInteractiveStyles
plugin that renders to CSS.
I am actually coming around to this idea, slowly, since it would fix many bugs similar to #524 and #527. I have two concerns, though:
As you can see there are a bunch of issues at play here. If we can do this correctly, though, this would open up just about any CSS selector to be easily used from within radium (e.g. :disabled). It also significantly changes the original architecture of Radium (pure inline styles, which has already been compromised), but really does stay true to the dynamic runtime-only awesomeness.
@alexlande, @coopy, @rofrischmann, @knowbody, any thoughts?
I've not checked all the StyleKeeper code right now, but within Look I am removing media query CSS immediately after the app was fully mounted (after window.matchMedia
is available) and use the inline method again while font-faces
, keyframes
stick to the CSS solution as there is no pure javascript one.
The only reason why media queries get rendered into CSS is for sure: initial rendering on server-side.
As hover
, focus
and active
do not work until the JS code is loaded completely, this might make sense as well (but I'd still remove them immediately after the app was mounted as I'd like to keep global CSS rules as small as possible).
I even heard once that there is some kind of performance issue if you're having tons of (unused) CSS selectors as e.g. when hovering an element the browser compares every single selector over and over again. Yet I do not know if this is/was or ever was a real issue and if it is true at all.
Another benefit actually is that you're able to use CSS rules to add support for all the other pseudo classes that are (not yet) supported by Radium or will never be as there is no pure javascript solution e.g. :visited. I also use this nice tiny tool to mimic some special pseudo classes e.g -webkit-input-placeholder. See Pseudo to CSS mixin for more information. You could e.g. have a plugin that takes some kind of pseudo class mapping from your config and translates those rules to CSS. Then users can choose for themselves which pseudo classes they'd like to use.
Still I am not sure wether I like the idea of converting everything to CSS as it somehow brings back all the problems with CSS which are (at least one of the main) reasons we even started building CSS in JS solutions, right?
For polyfilling unsupported pseudo classes as well as for initial server-side rendering this is great. Also especially hover
, active
and focus
could benefit at is uses the browsers built-in solution (and fixes many bugs). But always keep in mind where the initial idea behind CSS in JS solutions came from ;)
@ianobermiller I think you meant to mention @alexlande ;]
I actually think that using StyleKeeper
for states is a pretty great idea. I'd even go a little (OK, a lot) further and say that I'm sort of interested in treating CSS as a render target for everything instead of actual inline styles.
If you still get the benefits of encapsulation, specificity equalization, and runtime computation without having to duplicate CSS features (and without losing any potential browser optimizations for CSS vs inline), what's the downside? :trollface:
I'm sort of interested in treating CSS as a render target for everything
Heresy! We should definitely prototype this :).
I've done several performance tests and experiments. Some stuff I noticed:
style
) does not improve performance (at least not noticeable):hover
, :active
, :focus
are goneState
API (or why would you else need the event listener?)className
s are only added once and then can be reusedprops
, state
or context
will create new classes over and over again (if values differ) (Note: this also means you need to resolve them every time if they include dynamic values)Also I do not know if there are performance improvements using insertRule
in favor of rerendering the whole <style>
every time, but with the second approach you could perhaps do some kind of carbage collection better. Also the CSS styles get cleared automatically if you route to another root Component, which might help big applications.
As I mentioned above, one blogger once posted that every CSS selector slows down the rendering process (sure we're talking about really small values).
I once built an experimental module that does some kind of diffing to only update those CSS rules, but I don't think it does any improvement at all. Still it might be helpful: Dynamic Style Sheets (CSSSheet and DOM Interface did this job)
Awesome write-up, thanks @rofrischmann.
And so is the State API (or why would you else need the event listener?)
Very good point about the getState
API, I had forgotten about that. Users would have to manually add event listeners as a replacement, but I don't think getState
is used that much anyway.
Presort pseudo classes to avoid LVHA problems. *(Note: Leave the rest in the exact order or you'll have no option to order those)
LVHA is a good point too, might mean you will still want to merge such styles before application to preserve the order, the exact opposite of what I just did with media queries.
Pseudo classes (dynamically added) do not show up in the Chrome DevTools by default (? strange behaviour as those added with CSS files directly do)
Unfortunate, maybe we can file a task to get that one fixed.
@ianobermiller @alexlande Have you done any prototyping yet regarding css as render target? I'm wondering if this would allow us to somehow build a babel plugin which can traverse the whole build and figure out which media queries are needed and then render them to a separate file which can be instantly rendered on the client to prevent flickering on the first render of components which mobile-first styles and media queries.
@johanneslumpe: No one has, to my knowledge. That said, media queries are already rendered to CSS in a style tag, so in theory I would expect that one could already try to make such a babel plugin, without having to render all CSS that way (I think?).
You could make a radium plugin to test this out. We haven't yet prototyped it. On Wed, Mar 16, 2016 at 9:12 AM Alex Lande notifications@github.com wrote:
@johanneslumpe https://github.com/johanneslumpe: No one has, to my knowledge. That said, media queries are already rendered to CSS in a style tag, so in theory I would expect that one could already try to make such a babel plugin, without having to render all CSS that way (I think?).
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/FormidableLabs/radium/issues/510#issuecomment-197402848
Hi, I wrote the prototype as a seperate plugin 'no-inline', check this fork: https://github.com/pristas-peter/radium
@pristas-peter: I tried testing your fork over the weekend but there were build errors and it wouldn't run. Is there something I need to do to get it in a working state?
Hi, sorry I didnt try to build it, I'm using lodash in pkugin and I havent add it to package.json. If this would not be the case let me know and I will try to run the build by myself
hi, it's fixed, there was a typo in plugins/index.js
I think this would be great for all the browser specific pseudo selectors... Like ::-moz-focus-inner
. 😅
Would not it be better to support pseudo classes with style keeper just like mediaqueries and keyframes are?