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

Prefixing breaks server rendering #201

Closed
skevy opened this Issue Jun 9, 2015 · 43 comments

Comments

Projects
None yet
@skevy

skevy commented Jun 9, 2015

Not sure what to do on this - but in general, prefixing breaks server rendering, as it returns different markup on the server than it does on the client.

React spits out stuff like this:

Warning: React attempted to reuse markup in a container but the checksum was invalid. This generally means that you are using server rendering and the markup generated on the server was not what the client was expecting. React injected new markup to compensate which works but you have lost many of the benefits of server rendering. Instead, figure out why the markup being generated is different on the client or server:
(client) style="display:flex;-webkit-flex:1;-webk
(server) style="display:flex;flex:1;align-items:c

Ideas?

@awkaiser

This comment has been minimized.

Show comment
Hide comment
@awkaiser

awkaiser Jun 10, 2015

@skevy This is a known issue, and you can search closed issues for "prefix" to read more about it.

To avoid bulking up the size of Radium on the client, one solution would be an optional server-side override of Radium's prefixer module, using something like Autoprefixer to apply the appropriate prefixes based on known browser support (i.e. Can I Use data). I suspect even that may not always result in exactly the same prefix generation as Radium's current client-side method, though. If so, the client may need to share prefix logic with the server after all, and not rely on browser element style detection magic. Tricksy prefixes!

awkaiser commented Jun 10, 2015

@skevy This is a known issue, and you can search closed issues for "prefix" to read more about it.

To avoid bulking up the size of Radium on the client, one solution would be an optional server-side override of Radium's prefixer module, using something like Autoprefixer to apply the appropriate prefixes based on known browser support (i.e. Can I Use data). I suspect even that may not always result in exactly the same prefix generation as Radium's current client-side method, though. If so, the client may need to share prefix logic with the server after all, and not rely on browser element style detection magic. Tricksy prefixes!

@awkaiser

This comment has been minimized.

Show comment
Hide comment
@awkaiser

awkaiser Jun 10, 2015

Neither Radium's public API export or the Style module hung off that provide a direct path for server-side only code to override the Prefixer module or its methods.

@ianobermiller, is there a plan around that? Maybe something along the lines of Config.setMatchMedia, like Config.setPrefixer (expecting module that quacks like prefixer.js)?

awkaiser commented Jun 10, 2015

Neither Radium's public API export or the Style module hung off that provide a direct path for server-side only code to override the Prefixer module or its methods.

@ianobermiller, is there a plan around that? Maybe something along the lines of Config.setMatchMedia, like Config.setPrefixer (expecting module that quacks like prefixer.js)?

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Jun 10, 2015

Collaborator

@awkaiser Yes, we could definitely add such an extension point. The server-side replacement prefixer will have to be smarter than Autoprefixer, unfortunately, since it must only include a single prefixed key.

I think it may be more fruitful to modify Radium's prefixer to work on the client and the server. Two ideas come to mind:

  1. User-agent detection to prefix keys and values - requires a big mapping, which could potentially bloat Radium
  2. Both client and server can add all the prefixed keys (e.g. -webkit-transform -ms-transform and transform) this will not work correctly for values that need prefixes, like display: flex

I'm inclined to go with Option 1.

Collaborator

ianobermiller commented Jun 10, 2015

@awkaiser Yes, we could definitely add such an extension point. The server-side replacement prefixer will have to be smarter than Autoprefixer, unfortunately, since it must only include a single prefixed key.

I think it may be more fruitful to modify Radium's prefixer to work on the client and the server. Two ideas come to mind:

  1. User-agent detection to prefix keys and values - requires a big mapping, which could potentially bloat Radium
  2. Both client and server can add all the prefixed keys (e.g. -webkit-transform -ms-transform and transform) this will not work correctly for values that need prefixes, like display: flex

I'm inclined to go with Option 1.

@alexlande

This comment has been minimized.

Show comment
Hide comment
@alexlande

alexlande Jun 10, 2015

Member

Agree that it would be great for the prefixer to work on the client and the server. Personally I would prefer option 2. I'd love to let the browser handle as much of that stuff as possible.

Member

alexlande commented Jun 10, 2015

Agree that it would be great for the prefixer to work on the client and the server. Personally I would prefer option 2. I'd love to let the browser handle as much of that stuff as possible.

@awkaiser

This comment has been minimized.

Show comment
Hide comment
@awkaiser

awkaiser Jun 10, 2015

I wonder how small the mapping could be if it was created by a build script operating off Can I Use data, and baked down to just enough information for Radium to get the job done? If Radium restricts its auto-prefixing to reasonably recent browsers, there are not many rules that require prefixes. Allowing Radium's default prefix behavior to be overridden would allow for more comprehensive prefixing on projects that require it.

awkaiser commented Jun 10, 2015

I wonder how small the mapping could be if it was created by a build script operating off Can I Use data, and baked down to just enough information for Radium to get the job done? If Radium restricts its auto-prefixing to reasonably recent browsers, there are not many rules that require prefixes. Allowing Radium's default prefix behavior to be overridden would allow for more comprehensive prefixing on projects that require it.

@awkaiser

This comment has been minimized.

Show comment
Hide comment
@awkaiser

awkaiser Jun 10, 2015

Oh, and to keep things simple, there could be something like Radium.Config.setUserAgent() for use by server-side code and tests. Setting the user agent, and any prefixer overrides, would just need to be done before Radium processes any rules.

awkaiser commented Jun 10, 2015

Oh, and to keep things simple, there could be something like Radium.Config.setUserAgent() for use by server-side code and tests. Setting the user agent, and any prefixer overrides, would just need to be done before Radium processes any rules.

@awkaiser

This comment has been minimized.

Show comment
Hide comment
@awkaiser

awkaiser Jun 10, 2015

"Option 2" would require the least amount of code but wouldn't be very efficient over the wire (in terms of HTML delivered by server), e.g. 180 characters:

<div style="-moz-animation:foobar 5s infinite;-ms-animation:foobar 5s infinite;-o-animation:foobar 5s infinite;-webkit-animation:foobar 5s infinite;animation:foobar 5s infinite" />

vs. 82 chars if limited to current browser:

<div style="-webkit-animation:foobar 5s infinite;animation:foobar 5s infinite;" />

Spread across all DOM elements in a React application, extraneous rules can add up quickly. If it can be done without a disagreeable amount of library bloat, I think that predetermining the single necessary vendor prefix would be worthwhile. Keep in mind that HTML responses are more frequently cache busted (if they're cached at all, hah) than JavaScript or CSS assets, so extraneous style rules would arguably have a greater network effect over time than increasing Radium's minified library size a bit.

Logic to handle value prefixes for stuff like flexbox would be necessary for both options. "Option 1" could be smart about it and, unless this info is bogus, "Option 2" could shotgun blast all the value variations (again, wasteful over the wire).

awkaiser commented Jun 10, 2015

"Option 2" would require the least amount of code but wouldn't be very efficient over the wire (in terms of HTML delivered by server), e.g. 180 characters:

<div style="-moz-animation:foobar 5s infinite;-ms-animation:foobar 5s infinite;-o-animation:foobar 5s infinite;-webkit-animation:foobar 5s infinite;animation:foobar 5s infinite" />

vs. 82 chars if limited to current browser:

<div style="-webkit-animation:foobar 5s infinite;animation:foobar 5s infinite;" />

Spread across all DOM elements in a React application, extraneous rules can add up quickly. If it can be done without a disagreeable amount of library bloat, I think that predetermining the single necessary vendor prefix would be worthwhile. Keep in mind that HTML responses are more frequently cache busted (if they're cached at all, hah) than JavaScript or CSS assets, so extraneous style rules would arguably have a greater network effect over time than increasing Radium's minified library size a bit.

Logic to handle value prefixes for stuff like flexbox would be necessary for both options. "Option 1" could be smart about it and, unless this info is bogus, "Option 2" could shotgun blast all the value variations (again, wasteful over the wire).

@skevy

This comment has been minimized.

Show comment
Hide comment
@skevy

skevy Jun 11, 2015

Hey guys -

Sorry for not searching better before I flagged the issue. Usually I'm better about that.

I'm stoked about the discussion that has ensued, however. To chime in, I'm definitely inclined to prefer option 1. I think setting the user agent is perfectly reasonable.

skevy commented Jun 11, 2015

Hey guys -

Sorry for not searching better before I flagged the issue. Usually I'm better about that.

I'm stoked about the discussion that has ensued, however. To chime in, I'm definitely inclined to prefer option 1. I think setting the user agent is perfectly reasonable.

@moret

This comment has been minimized.

Show comment
Hide comment
@moret

moret Jul 24, 2015

Hello guys.

👍 for thinking about this issue - which I would not label as a bug but as enhancement, btw.

Let me chime in the discussion too: I agree that option 1 of server-side UA detection and prefixing only what is needed is more efficient. It will force changes to existing setups that don't create caches for different UA's, though - e.g. nginx with path as a cache_key serving upstream node servers. Do you guys think this would be a problem for adoption?

About option 2, would that mean adding a build step to avoid bundling autoprefixer or someone similar?

moret commented Jul 24, 2015

Hello guys.

👍 for thinking about this issue - which I would not label as a bug but as enhancement, btw.

Let me chime in the discussion too: I agree that option 1 of server-side UA detection and prefixing only what is needed is more efficient. It will force changes to existing setups that don't create caches for different UA's, though - e.g. nginx with path as a cache_key serving upstream node servers. Do you guys think this would be a problem for adoption?

About option 2, would that mean adding a build step to avoid bundling autoprefixer or someone similar?

@DiThi

This comment has been minimized.

Show comment
Hide comment
@DiThi

DiThi Jul 24, 2015

Here's another idea: Serve the client a JS script at the end of <body> that prefixes every prefixeable property in the HTML among the ones used.

The drawback is that the content wouldn't be properly styled until the body has loaded. I would add it as a complement of option 1, in case the user agent sniffing was incorrect, replacing every instance of the incorrect prefix by the correct one.

DiThi commented Jul 24, 2015

Here's another idea: Serve the client a JS script at the end of <body> that prefixes every prefixeable property in the HTML among the ones used.

The drawback is that the content wouldn't be properly styled until the body has loaded. I would add it as a complement of option 1, in case the user agent sniffing was incorrect, replacing every instance of the incorrect prefix by the correct one.

@moret

This comment has been minimized.

Show comment
Hide comment
@moret

moret Jul 25, 2015

@DiThi Currently that's what Radium does, it automatically prefixes on the browser using the thin client-side prefixer.js instead of a heavyweight - for the client - like autoprefixer. But that means accepting some FOUC if the prefixes are not delivered from the server too. React will complain that the client rendering expectations are not matching the server rendering, which is the source of this issue.

moret commented Jul 25, 2015

@DiThi Currently that's what Radium does, it automatically prefixes on the browser using the thin client-side prefixer.js instead of a heavyweight - for the client - like autoprefixer. But that means accepting some FOUC if the prefixes are not delivered from the server too. React will complain that the client rendering expectations are not matching the server rendering, which is the source of this issue.

@DiThi

This comment has been minimized.

Show comment
Hide comment
@DiThi

DiThi Jul 25, 2015

Ah, I see! Here's another idea, then: have radium render client side using
DOM's style attributes instead of their own. Next time it renders, react
won't complain.

Even better it would be to have a flag in React that ignores such
differences, overwriting all the style.

On Sat, Jul 25, 2015 at 2:23 AM, Danilo Moret notifications@github.com
wrote:

@DiThi https://github.com/DiThi Currently that's what Radium does, it
automatically prefixes on the browser using the thin client-side
prefixer.js
https://github.com/FormidableLabs/radium/blob/master/modules/prefixer.js
instead of a heavyweight - for the client - like autoprefixer. But that
means accepting some FOUC
https://en.wikipedia.org/wiki/Flash_of_unstyled_content if the prefixes
are not delivered from the server too. This means that React will complain
that the client rendering is not matching the server rendering, which is
the source of this issue.


Reply to this email directly or view it on GitHub
#201 (comment)
.

DiThi commented Jul 25, 2015

Ah, I see! Here's another idea, then: have radium render client side using
DOM's style attributes instead of their own. Next time it renders, react
won't complain.

Even better it would be to have a flag in React that ignores such
differences, overwriting all the style.

On Sat, Jul 25, 2015 at 2:23 AM, Danilo Moret notifications@github.com
wrote:

@DiThi https://github.com/DiThi Currently that's what Radium does, it
automatically prefixes on the browser using the thin client-side
prefixer.js
https://github.com/FormidableLabs/radium/blob/master/modules/prefixer.js
instead of a heavyweight - for the client - like autoprefixer. But that
means accepting some FOUC
https://en.wikipedia.org/wiki/Flash_of_unstyled_content if the prefixes
are not delivered from the server too. This means that React will complain
that the client rendering is not matching the server rendering, which is
the source of this issue.


Reply to this email directly or view it on GitHub
#201 (comment)
.

@moret

This comment has been minimized.

Show comment
Hide comment
@moret

moret Jul 25, 2015

@DiThi I think that would beat the purpose of the server-side rendering. When only client-side rendering is concerned Radium already does an excellent job with the vendor prefixes.

moret commented Jul 25, 2015

@DiThi I think that would beat the purpose of the server-side rendering. When only client-side rendering is concerned Radium already does an excellent job with the vendor prefixes.

@DiThi

This comment has been minimized.

Show comment
Hide comment
@DiThi

DiThi Jul 25, 2015

@moret what would? If I understood the problem correctly, server side rendering yields good results because it ships with a simple prefixer.js, but it differs from the client side rendering when you want to attach events, etc, after it has fully loaded. So that warning should be silenced somehow, telling react "I know there are differences, ignore them and overwrite the styles please".

And, if it's not possible to add such feature to react, as a workaround I would render the "correct" styles the first client render pass by taking the style attribute given by the server+prefixer.

DiThi commented Jul 25, 2015

@moret what would? If I understood the problem correctly, server side rendering yields good results because it ships with a simple prefixer.js, but it differs from the client side rendering when you want to attach events, etc, after it has fully loaded. So that warning should be silenced somehow, telling react "I know there are differences, ignore them and overwrite the styles please".

And, if it's not possible to add such feature to react, as a workaround I would render the "correct" styles the first client render pass by taking the style attribute given by the server+prefixer.

@moret

This comment has been minimized.

Show comment
Hide comment
@moret

moret Jul 25, 2015

@DiThi indeed many trade-offs are possible. My preference would be in the direction of full isomorphic rendering, and React warning helps in this sense. 8)

moret commented Jul 25, 2015

@DiThi indeed many trade-offs are possible. My preference would be in the direction of full isomorphic rendering, and React warning helps in this sense. 8)

@alexlande

This comment has been minimized.

Show comment
Hide comment
@alexlande

alexlande Aug 3, 2015

Member

@ianobermiller: Curious about your thoughts on https://github.com/UXtemple/babel-plugin-react-autoprefix

I haven't tried it yet, and it might need some updates (arrays for property fallbacks), but it would be a nice option to use something like that and make the prefixer optional if you're handling it in another way.

Member

alexlande commented Aug 3, 2015

@ianobermiller: Curious about your thoughts on https://github.com/UXtemple/babel-plugin-react-autoprefix

I haven't tried it yet, and it might need some updates (arrays for property fallbacks), but it would be a nice option to use something like that and make the prefixer optional if you're handling it in another way.

@stephanos

This comment has been minimized.

Show comment
Hide comment
@stephanos

stephanos Aug 3, 2015

Regarding option 2 (aka 'add all the prefixed keys'): Maybe gzip can reduce the burden significantly? I mean if you increase the payload by, let's say, less than 5% by prefixing everything - wouldn't that be worth it?

Bottom line is we don't really know how much prefixing actually adds to the (compressed) payload.

stephanos commented Aug 3, 2015

Regarding option 2 (aka 'add all the prefixed keys'): Maybe gzip can reduce the burden significantly? I mean if you increase the payload by, let's say, less than 5% by prefixing everything - wouldn't that be worth it?

Bottom line is we don't really know how much prefixing actually adds to the (compressed) payload.

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Aug 3, 2015

Collaborator

@alexlande Thanks for the link, that is pretty amazing. I worry a bit that it won't capture all usages (styles defined in a var styles = {} block at the bottom, or styles defined in other files. But those can probably be fixed. Very cool approach, though! Fallbacks could probably be solved by doing, e.g. height: '100vh;height:800px yeah? Would much prefer to stand on the shoulders of autoprefixer!

@stephanos I'm not terribly worried about increasing the payload size, since, as you say, gzip will make most of that irrelevant.

Collaborator

ianobermiller commented Aug 3, 2015

@alexlande Thanks for the link, that is pretty amazing. I worry a bit that it won't capture all usages (styles defined in a var styles = {} block at the bottom, or styles defined in other files. But those can probably be fixed. Very cool approach, though! Fallbacks could probably be solved by doing, e.g. height: '100vh;height:800px yeah? Would much prefer to stand on the shoulders of autoprefixer!

@stephanos I'm not terribly worried about increasing the payload size, since, as you say, gzip will make most of that irrelevant.

@alexlande

This comment has been minimized.

Show comment
Hide comment
@alexlande

alexlande Aug 3, 2015

Member

@ianobermiller Something like height: '100vh;height:800px would probably work fine, yeah. Array-based fallbacks would be really nice to have built into React though.

Member

alexlande commented Aug 3, 2015

@ianobermiller Something like height: '100vh;height:800px would probably work fine, yeah. Array-based fallbacks would be really nice to have built into React though.

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Aug 3, 2015

Collaborator

Sorry, that should be {height: '100%; height: 100vh'}. But yeah in Radium we should definitely do:

{height: ['100%', '100vh']}

We should probably also reverse the order, as radium currently requires the fallback to be last, which is the opposite of CSS.

Collaborator

ianobermiller commented Aug 3, 2015

Sorry, that should be {height: '100%; height: 100vh'}. But yeah in Radium we should definitely do:

{height: ['100%', '100vh']}

We should probably also reverse the order, as radium currently requires the fallback to be last, which is the opposite of CSS.

@boyswan

This comment has been minimized.

Show comment
Hide comment
@boyswan

boyswan Aug 17, 2015

any news on this?

boyswan commented Aug 17, 2015

any news on this?

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Aug 19, 2015

Collaborator

@rofrischmann is making nice progress on inline-style-prefixer. I've not had time to tackle this issue, and as of yet, server rendering is not very important for my use cases.

Collaborator

ianobermiller commented Aug 19, 2015

@rofrischmann is making nice progress on inline-style-prefixer. I've not had time to tackle this issue, and as of yet, server rendering is not very important for my use cases.

@andytompkins

This comment has been minimized.

Show comment
Hide comment
@andytompkins

andytompkins Aug 20, 2015

keyframes.js has a problem too. It creates a style element and adds it to head to determine if prefixing is needed for keyframe animations. I end up with <style>@keyframes {}</style> in my dom even though I'm not using keyframes at all. (I think it should be creating and adding the element in the keyframes function instead, and calling removeChild on the test tag that was added prior). I get Invariant Violation first because of this, but even if I "fix" it, I get them because of prefixes. I really want to use Radium, but I also really want to build a universal/isomorphic app. Server side rendering is quite important for some of us.

andytompkins commented Aug 20, 2015

keyframes.js has a problem too. It creates a style element and adds it to head to determine if prefixing is needed for keyframe animations. I end up with <style>@keyframes {}</style> in my dom even though I'm not using keyframes at all. (I think it should be creating and adding the element in the keyframes function instead, and calling removeChild on the test tag that was added prior). I get Invariant Violation first because of this, but even if I "fix" it, I get them because of prefixes. I really want to use Radium, but I also really want to build a universal/isomorphic app. Server side rendering is quite important for some of us.

@doctyper

This comment has been minimized.

Show comment
Hide comment
@doctyper

doctyper Aug 20, 2015

Contributor

👍 Just chiming in on importance. The NFL is interested in switching to Radium but we are universal, so server-side rendering is a blocker.

Contributor

doctyper commented Aug 20, 2015

👍 Just chiming in on importance. The NFL is interested in switching to Radium but we are universal, so server-side rendering is a blocker.

@moret

This comment has been minimized.

Show comment
Hide comment
@moret

moret Aug 20, 2015

@ianobermiller I'm setting up a universal example and trying to use @rofrischmann inline-style-prefixer. Even in it's initial stage it seems to be a nice approach to render the same prefixes both on the client and on the server, as in option 1 above.

But I still have the nagging feeling that option 2 of including all known prefixes via autoprefixer would be easier to general use, despite being less efficient. 8( Do you guys have an opinion on that?

moret commented Aug 20, 2015

@ianobermiller I'm setting up a universal example and trying to use @rofrischmann inline-style-prefixer. Even in it's initial stage it seems to be a nice approach to render the same prefixes both on the client and on the server, as in option 1 above.

But I still have the nagging feeling that option 2 of including all known prefixes via autoprefixer would be easier to general use, despite being less efficient. 8( Do you guys have an opinion on that?

@moret

This comment has been minimized.

Show comment
Hide comment
@moret

moret Aug 20, 2015

I couldn't get option 2 out of my head. 😄 I've switched the experiment to use autoprefix and it's looks promising. I can work on that if you guys agree with this approach.

moret commented Aug 20, 2015

I couldn't get option 2 out of my head. 😄 I've switched the experiment to use autoprefix and it's looks promising. I can work on that if you guys agree with this approach.

@rofrischmann

This comment has been minimized.

Show comment
Hide comment
@rofrischmann

rofrischmann Aug 20, 2015

Contributor

@moret FYI: My prefixer now is kind of alpha :p i am supporting all properties, some values and some browser issues e.g. Flexbox specifications.
Yet I have never used autoprefixer or even checked their code in detail, but I always thought their parsing CSS to some handy JSON, process it and compose back to CSS which for me does not sound performant on runtime (client-side mainly), but fix me if I am wrong.
If it works fine I'd love to me know about that too :)

Sadly I am at my iPad right know so I can't test / check it properly..
Also it'd be nice to just tell me the final file size of autoprefix, because that also matters sometimes.

Contributor

rofrischmann commented Aug 20, 2015

@moret FYI: My prefixer now is kind of alpha :p i am supporting all properties, some values and some browser issues e.g. Flexbox specifications.
Yet I have never used autoprefixer or even checked their code in detail, but I always thought their parsing CSS to some handy JSON, process it and compose back to CSS which for me does not sound performant on runtime (client-side mainly), but fix me if I am wrong.
If it works fine I'd love to me know about that too :)

Sadly I am at my iPad right know so I can't test / check it properly..
Also it'd be nice to just tell me the final file size of autoprefix, because that also matters sometimes.

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Aug 21, 2015

Collaborator

I did a quick test to add autoprefix to Radium to test the final size. After minify + gzip, size is 43k, vs 5k. autoprefix pulls in autoprefixer-core, which has postcss and some other things that we really don't need on the client side, so I'll bet it could be smaller. I think this underscores the need to get a plugin system working first, since it would be easy to try different prefixers then.

Collaborator

ianobermiller commented Aug 21, 2015

I did a quick test to add autoprefix to Radium to test the final size. After minify + gzip, size is 43k, vs 5k. autoprefix pulls in autoprefixer-core, which has postcss and some other things that we really don't need on the client side, so I'll bet it could be smaller. I think this underscores the need to get a plugin system working first, since it would be easy to try different prefixers then.

@rofrischmann

This comment has been minimized.

Show comment
Hide comment
@rofrischmann

rofrischmann Aug 21, 2015

Contributor

Could be done by adding something like Config.setPrefixer which should just be a func that takes an obj and outputs/modifies it.
43kb is quite a mess though, but that'd be users choice then :p

Contributor

rofrischmann commented Aug 21, 2015

Could be done by adding something like Config.setPrefixer which should just be a func that takes an obj and outputs/modifies it.
43kb is quite a mess though, but that'd be users choice then :p

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Aug 21, 2015

Collaborator

I think I'll have a working plugin system soon. Stayed up way too late working on it yesterday :S

Collaborator

ianobermiller commented Aug 21, 2015

I think I'll have a working plugin system soon. Stayed up way too late working on it yesterday :S

@ronag

This comment has been minimized.

Show comment
Hide comment
@ronag

ronag Sep 22, 2015

The plugin system seems to be available now. For anyone that is having the same issue and are alright with using auto prefix until a better solution is found here is how you can do it:

Based on @moret example.

import Radium from 'radium'
import { Plugins, PluginConfig, PluginResult } from 'radium'
import autoPrefix from 'autoprefix'

const autoPrefixPlugin = ({componentName, style}) => {
  const newStyle = {}
  const prefixedStyle = autoPrefix(style)
  Object
    .keys(prefixedStyle)
    .forEach((key) => {
      const prefixedValue = prefixedStyle[key]

      if (prefixedValue instanceof Array) {
        newStyle[key] = prefixedValue.join('; ' + key)
      } else {
        newStyle[key] = prefixedValue
      }
    })

  return {style: newStyle}
}

export default function style(component) {
  return Radium({
    plugins: [
      Plugins.mergeStyleArray,
      Plugins.checkProps,
      Plugins.resolveMediaQueries,
      Plugins.resolveInteractionStyles,
      autoPrefixPlugin,
      Plugins.checkProps
    ]
  })(component)
}

ronag commented Sep 22, 2015

The plugin system seems to be available now. For anyone that is having the same issue and are alright with using auto prefix until a better solution is found here is how you can do it:

Based on @moret example.

import Radium from 'radium'
import { Plugins, PluginConfig, PluginResult } from 'radium'
import autoPrefix from 'autoprefix'

const autoPrefixPlugin = ({componentName, style}) => {
  const newStyle = {}
  const prefixedStyle = autoPrefix(style)
  Object
    .keys(prefixedStyle)
    .forEach((key) => {
      const prefixedValue = prefixedStyle[key]

      if (prefixedValue instanceof Array) {
        newStyle[key] = prefixedValue.join('; ' + key)
      } else {
        newStyle[key] = prefixedValue
      }
    })

  return {style: newStyle}
}

export default function style(component) {
  return Radium({
    plugins: [
      Plugins.mergeStyleArray,
      Plugins.checkProps,
      Plugins.resolveMediaQueries,
      Plugins.resolveInteractionStyles,
      autoPrefixPlugin,
      Plugins.checkProps
    ]
  })(component)
}

@ronag

This comment has been minimized.

Show comment
Hide comment
@ronag

ronag Sep 22, 2015

I've noticed that this solution is quite bad because it significantly slows things down. We've decided to simply not use auto prefixing in any form.

ronag commented Sep 22, 2015

I've noticed that this solution is quite bad because it significantly slows things down. We've decided to simply not use auto prefixing in any form.

@alexlande

This comment has been minimized.

Show comment
Hide comment
@alexlande

alexlande Sep 22, 2015

Member

@ronag: Yeah, autoprefixer is way too large to include on the client-side. You might look into a Babel plugin like this that could preprocess your styles to add vendor prefixes: https://github.com/UXtemple/babel-plugin-react-autoprefix

(That plugin might not work out of the box for Radium, but could be a good starting point.)

Member

alexlande commented Sep 22, 2015

@ronag: Yeah, autoprefixer is way too large to include on the client-side. You might look into a Babel plugin like this that could preprocess your styles to add vendor prefixes: https://github.com/UXtemple/babel-plugin-react-autoprefix

(That plugin might not work out of the box for Radium, but could be a good starting point.)

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Sep 22, 2015

Collaborator

I've been tracking the progress of inline-style-prefixer, which seeks to do prefixing based on user agent.

Collaborator

ianobermiller commented Sep 22, 2015

I've been tracking the progress of inline-style-prefixer, which seeks to do prefixing based on user agent.

@toddw

This comment has been minimized.

Show comment
Hide comment
@toddw

toddw Oct 7, 2015

I vote for Option 2, 180 extra characters doesn't seem like a big deal at all. I know it's not a 1-1 mapping but an extra 180 bytes to better support server side rendering. Even if you had to do it 100 times on the page, after gzipping it, that's not really much to worry about. I'm a little ignorant to exactly how gzip works but in my experiments, when you have a string repeated several times, the compression is much greater. There will be a lot of repeated strings with the prefixes so I think it would be pretty compressed.

I wanted to educate myself on what this would actually amount to with a "more realistic" data set so I created a quick script to generate the example with an extra 180 characters from above.

My Script:

require "securerandom"

100.times do |number|
  name = SecureRandom.base64(15)
  puts "<div style='-moz-animation:#{name} #{number}s infinite;-ms-animation:#{name} #{number}s infinite;-o-animation:#{name} #{number}s infinite;-webkit-animation:#{name} #{number}s infinite;animation:#{name} #{number}s infinite' />"
end

This outputs something like:

<div style='-moz-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;-ms-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;-o-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;-webkit-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;animation:j7iEN8Idp2sqSbK4LL7y 0s infinite' />
<div style='-moz-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;-ms-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;-o-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;-webkit-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;animation:lkkeP/z/wcROPaQsZnxm 1s infinite' />
…
…

Then I created a text file with the output and then gzipped it.

$ ruby test.rb > test.text
$ tar czfv test.text.tar.gz test.text
> a test.text
$ ls -alh
> 25K test.text
> 3.5K test.text.tar.gz

Then I modified the script to render the more browser specific version:

require "securerandom"

100.times do |number|
  name = SecureRandom.base64(15)
  puts "<div style='-webkit-animation:#{name} #{number}s infinite;' />"
end

and I these commands again:

$ ruby test.rb > test.text
$ tar czfv test.text.tar.gz test.text
> a test.text
$ ls -alh
> 6.7K test.text
> 2.2K test.text.tar.gz

I realize this might not be exactly what would happen in a real life situation but it is as close as I could get with the amount of time I felt like spending on this. So if my test isn't totally bogus and I'm understanding the results correctly. There is only a 0.3K difference between the two versions. This is with it on the screen 100 times. Also, I realize there are more tags so this is going to very depending on how many different vendor prefixed properties you use.

Please let me know if I'm way off here… but if this has any merit then I vote for Option 2.

toddw commented Oct 7, 2015

I vote for Option 2, 180 extra characters doesn't seem like a big deal at all. I know it's not a 1-1 mapping but an extra 180 bytes to better support server side rendering. Even if you had to do it 100 times on the page, after gzipping it, that's not really much to worry about. I'm a little ignorant to exactly how gzip works but in my experiments, when you have a string repeated several times, the compression is much greater. There will be a lot of repeated strings with the prefixes so I think it would be pretty compressed.

I wanted to educate myself on what this would actually amount to with a "more realistic" data set so I created a quick script to generate the example with an extra 180 characters from above.

My Script:

require "securerandom"

100.times do |number|
  name = SecureRandom.base64(15)
  puts "<div style='-moz-animation:#{name} #{number}s infinite;-ms-animation:#{name} #{number}s infinite;-o-animation:#{name} #{number}s infinite;-webkit-animation:#{name} #{number}s infinite;animation:#{name} #{number}s infinite' />"
end

This outputs something like:

<div style='-moz-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;-ms-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;-o-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;-webkit-animation:j7iEN8Idp2sqSbK4LL7y 0s infinite;animation:j7iEN8Idp2sqSbK4LL7y 0s infinite' />
<div style='-moz-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;-ms-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;-o-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;-webkit-animation:lkkeP/z/wcROPaQsZnxm 1s infinite;animation:lkkeP/z/wcROPaQsZnxm 1s infinite' />
…
…

Then I created a text file with the output and then gzipped it.

$ ruby test.rb > test.text
$ tar czfv test.text.tar.gz test.text
> a test.text
$ ls -alh
> 25K test.text
> 3.5K test.text.tar.gz

Then I modified the script to render the more browser specific version:

require "securerandom"

100.times do |number|
  name = SecureRandom.base64(15)
  puts "<div style='-webkit-animation:#{name} #{number}s infinite;' />"
end

and I these commands again:

$ ruby test.rb > test.text
$ tar czfv test.text.tar.gz test.text
> a test.text
$ ls -alh
> 6.7K test.text
> 2.2K test.text.tar.gz

I realize this might not be exactly what would happen in a real life situation but it is as close as I could get with the amount of time I felt like spending on this. So if my test isn't totally bogus and I'm understanding the results correctly. There is only a 0.3K difference between the two versions. This is with it on the screen 100 times. Also, I realize there are more tags so this is going to very depending on how many different vendor prefixed properties you use.

Please let me know if I'm way off here… but if this has any merit then I vote for Option 2.

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Oct 7, 2015

Collaborator

The problem with Option 2 is that it either requires: a) using autoprefixer on the client or b) preprocessing your style objects.

a) is untenable because autoprefixer is just too big to ship down
b) is already a solution, and there are libs that do this. It is not perfect, though, as you either have to have some marker for the preprocessor or always structure your code in such as way that every inline style gets preprocessed. This hampers usability and makes it very easy to have cases where your styles are no longer prefixed, e.g. hard to get right.

Collaborator

ianobermiller commented Oct 7, 2015

The problem with Option 2 is that it either requires: a) using autoprefixer on the client or b) preprocessing your style objects.

a) is untenable because autoprefixer is just too big to ship down
b) is already a solution, and there are libs that do this. It is not perfect, though, as you either have to have some marker for the preprocessor or always structure your code in such as way that every inline style gets preprocessed. This hampers usability and makes it very easy to have cases where your styles are no longer prefixed, e.g. hard to get right.

@toddw

This comment has been minimized.

Show comment
Hide comment
@toddw

toddw Oct 7, 2015

Thanks for the response @ianobermiller

I wasn't aware of these additional issues, I thought the primary concern was the extra payload size from using all of the prefixes.

toddw commented Oct 7, 2015

Thanks for the response @ianobermiller

I wasn't aware of these additional issues, I thought the primary concern was the extra payload size from using all of the prefixes.

@cly

This comment has been minimized.

Show comment
Hide comment
@cly

cly Nov 7, 2015

So if prefixer was turned off, would the two renderers return same result? In other words, if prefixing was done manually?

cly commented Nov 7, 2015

So if prefixer was turned off, would the two renderers return same result? In other words, if prefixing was done manually?

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Nov 7, 2015

Collaborator

Yes. I am working on landing the new prefixer as soon as possible.

Collaborator

ianobermiller commented Nov 7, 2015

Yes. I am working on landing the new prefixer as soon as possible.

@cly

This comment has been minimized.

Show comment
Hide comment
@cly

cly Nov 7, 2015

Sweet thanks!

cly commented Nov 7, 2015

Sweet thanks!

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Nov 12, 2015

Collaborator

Fixed with use of inline-style-prefixer.

Collaborator

ianobermiller commented Nov 12, 2015

Fixed with use of inline-style-prefixer.

@bsdo64

This comment has been minimized.

Show comment
Hide comment
@bsdo64

bsdo64 Dec 8, 2015

@ianobermiller I tried to use inline-style-prefixer. Every css property works fine but, Radium's '@media (max-width: 1145px)'(e.g) it also throw warning. How can I handle this?

bsdo64 commented Dec 8, 2015

@ianobermiller I tried to use inline-style-prefixer. Every css property works fine but, Radium's '@media (max-width: 1145px)'(e.g) it also throw warning. How can I handle this?

@ianobermiller

This comment has been minimized.

Show comment
Hide comment
@ianobermiller

ianobermiller Dec 9, 2015

Collaborator

Radium already uses inline-style-prefixer internally, you don't have to use it yourself.

Collaborator

ianobermiller commented Dec 9, 2015

Radium already uses inline-style-prefixer internally, you don't have to use it yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment