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

[jss-ssr-vendor-prefixer] make it work on server side #279

Open
kof opened this Issue Jul 31, 2016 · 27 comments

Comments

Projects
None yet
8 participants
@kof
Copy link
Member

kof commented Jul 31, 2016

I think the best way would be to use autoprefixer serverside. Basically a user can do it already now by passing the css generated by jss to autoprefixer, but this requires an additional build step and setup.

Lets create a separate plugin for ssr vendor prefixing to avoid unneeded dependences for people who don't need it.

@kof kof added the task label Jul 31, 2016

@kof kof changed the title [jss-vendor-prefixer] make it work in SSR [jss-vendor-prefixer] make it work with SSR Jul 31, 2016

@typical000

This comment has been minimized.

Copy link
Contributor

typical000 commented Jul 31, 2016

Maybe, as a solution - create a flag when initialize jss-vendor-prefixer that describes 2 modes:

  1. SSR (server-side rendering) - add all prefixes for all supported browsers. It will work like postcss-autoprefixes. But, in this case, we need additional variable on initialization - supported browsers (like in postcss-autoprefixes - object with versions of browsers)
  2. CSR (client-side rendering) - works, like now - adds prefixes only for current browser
    And after, in some way, detect if we create styles on server, or on client - and switch true/false this flag
@kof

This comment has been minimized.

Copy link
Member Author

kof commented Aug 5, 2016

A good alternative to autoprefixer, may be not as feature complete - inline-style-prefixer

@kof kof changed the title [jss-vendor-prefixer] make it work with SSR [jss-vendor-prefixer] make it work on server side Aug 10, 2016

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Aug 10, 2016

As we support ie10+ we may not need all features of postcss autoprefixer so inline-style-prefixer may be a good alternative cc @rofrischmann

@rofrischmann

This comment has been minimized.

Copy link
Member

rofrischmann commented Aug 10, 2016

(y) I will soon implement the Autoprefixer test suite (at least all relevant browser versions), but sadly I spill beer over my Macbook. It's still working, but I will wait for some days to let it dry completely. Feel free to add feature requests or bugs if the prefixer is missing something.

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Aug 10, 2016

We need a platform that buys open source contributors a new notebook once they loose theirs!!!

@rofrischmann

This comment has been minimized.

Copy link
Member

rofrischmann commented Aug 10, 2016

Big companies could donate old notebooks :P But as far as I know a lot are already doing so, e.g. to open coding schools for kids, homeless people, etc.

But well, it's still working, so no worries haha :D It even is quite relaxing to have some days forced not to code. I'll ping you on Gitter as soon as I am back^^

@nathanmarks

This comment has been minimized.

Copy link
Member

nathanmarks commented Aug 20, 2016

@kof

Are you still thinking of using inline-style-prefixer? This issue definitely a feature that material-ui would need to have a good solution for.

@umidbekkarimov

This comment has been minimized.

Copy link
Contributor

umidbekkarimov commented Aug 21, 2016

So as i get, there are two solutions for it:

  1. On server side build stylesheet for all browsers and rewrite on client side for current browser
  2. On server side build for current browser and do same on client side

Problem with first approach:
Currently class names get names from stylesheet hash, so if stylesheets would be different on client and server side we will get errors from react based on wrong classnames

Problems with second approach:
Common pattern with jss - is to use singleton, so inline-style-prefixer or autoprefixer plugin will be applied once to singleton, so there should be tool that will allow to render css stylesheets based on user-agent, e.g.: jss.sheets.toString(userAgent)

@nathanmarks

This comment has been minimized.

Copy link
Member

nathanmarks commented Aug 21, 2016

@umidbekkarimov

Best bet for server side compatibility with changing styles is to use the factory method instead of using jss as a singleton.

I'm developing a quick prototype solution using jss right now for another project, and I've abstracted away all of the jss methods, even createStyleSheet.

@umidbekkarimov

This comment has been minimized.

Copy link
Contributor

umidbekkarimov commented Aug 21, 2016

@nathanmarks agree, I'm thinking about rewriting useSheet method from react-jss to work with react context and create JssProvider component to provide custom jss instance.

@kof what are you think about it?


Also I have some ideas for attaching jss sheets on server side based on https://github.com/nfl/react-helmet

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Aug 21, 2016

@nathanmarks have you looked into inline-style-prefixer, do you see any issues there?

@nathanmarks

This comment has been minimized.

Copy link
Member

nathanmarks commented Aug 21, 2016

@kof the main downside is that it's a fairly large lib if you use the dynamic version. For a jss integration, It would be good for users if they could decide whether to use the dynamic or static version.

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Aug 21, 2016

I only consider using it for server-side, where size doesn't matter that much. Dynamic version will be used on the client as it is now.

@nathanmarks

This comment has been minimized.

Copy link
Member

nathanmarks commented Aug 21, 2016

Would be good to test both libraries against each other for the browsers to make sure output is consistent.

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Aug 21, 2016

Yep, it should be done at some point, because otherwise someone will wonder why ssr page is displayed differently than same page rendered on the client. There will be a need to run the same test suite, but adapted for the case that client library only adds prefix for the current runtime if at all.

@nathanmarks

This comment has been minimized.

Copy link
Member

nathanmarks commented Aug 21, 2016

@kof If they use the dynamic version of inline-style-prefixer server side and pass the UA string, the output should be fairly close... in an ideal world 😄

@kof kof added the important label Nov 5, 2016

@kof kof added the plugin label Dec 2, 2016

@kof kof changed the title [jss-vendor-prefixer] make it work on server side [jss-ssr-vendor-prefixer] make it work on server side Aug 20, 2017

@jedwards1211

This comment has been minimized.

Copy link
Contributor

jedwards1211 commented Jan 17, 2018

@kof seems to me like it would be cleanest if jss-vendor-prefixer and css-prefix are both able to accept a UA string, and default to trying to get one from the browser if none is given. That way you know it's the same code paths doing to prefixing for a given UA on client and server side.

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Jan 17, 2018

Using user agent string for vendor prefixing means no feature detection but agent sniffing. That is exactly what we are trying to avoid in runtime.

@jedwards1211

This comment has been minimized.

Copy link
Contributor

jedwards1211 commented Jan 17, 2018

@kof okay, then what about using agent sniffing on the server side and feature detection on the client side? Or are you saying that the only robust approach is to apply all prefixes on the server side?

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Jan 17, 2018

UA can be used during SSR, but can't if its a build time rendered CSS, so we need to find the best route. I was thinking of static prefixing for both SSR and build time for simplicity.

@eps1lon

This comment has been minimized.

Copy link
Contributor

eps1lon commented Jan 10, 2019

IMO a strict mode that uses autoprefixer (and therefore uses the .browserslistrc) for purely static rendering and a loose mode that can accept UA strings for dynamic rendering would be nice. At least make the choice for the user explicit if one wants to serve a little bit more CSS to be sure or a minimal version that might be wrong (not sure what the state of UA strings is).

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Jan 10, 2019

@eps1lon that would be another interesting thing to do, but caching would be even harder.

@eps1lon

This comment has been minimized.

Copy link
Contributor

eps1lon commented Jan 10, 2019

@kof Is this actually an issue for server-side-rendering? Isn't the hole component tree thrown away between requests anyway which reduces the usefulness of a cache? Could you specify what should be cached in jss-vendor-prefixer?

Come to think of it it sounds like the simplest solution would be to advice users to pipe the output from jss into e.g. autprefixer. Or let them inject their own "vendor" implementation. I'll see if I can write an adapter for jss-vendor-prefixer that replaces css-vendor on the server with the help of autoprefixer.

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Jan 10, 2019

It might depend on consumer. If consumer can cache the responses, they will also have to include UA in that case and cache for each UA string separately, basically still possible, but will increase the size of the cache. We can allow it if it doesn't make maintaining it too complex.

@oliviertassinari

This comment has been minimized.

Copy link
Member

oliviertassinari commented Jan 10, 2019

IMHO it would be great to have two modes, exactly like we had with Material-UI v0.x.

  1. A full prefixing strategy using the project .browserslistrc, agnostic of the actual browser that requests the resource. It comes at a cost, the generated CSS is larger. But it also comes with a massive advantage. When caching, you don't have to take the browser into account. I believe it's what styled-components and emotion are doing. It's what we do with the Material-UI documentation website.
  2. A per browser caching. Basically having the opposite pros and cons of the 1st solution.

These two modes are fundamentally using a different tradeoff. I think that they both of their use cases.

@kof

This comment has been minimized.

Copy link
Member Author

kof commented Jan 10, 2019

@oliviertassinari its possible. The most important thing for SSR vendor prefixing is performance, many cases won't be able to cache the final result. I don't think autoprefixer over postcss will perform well in that case.

So ideally, we get something that works well without caching. If caching is possible it won't hurt. But autoprefixer without being able to cache will be probably really slow.

@eps1lon

This comment has been minimized.

Copy link
Contributor

eps1lon commented Jan 10, 2019

It's unfortunately not as simple as writing a server side implementation of css-vendor since jss-plugin-vendor-prefixer works under the assumption that an existing property is only changed. If we would pass a browserslist it might however be necessary to expand a given property. So it might be viable for a single browser but not for a list of browsers. Since I don't think UA sniffing is a viable option I will just use autoprefixer on the server and jss-plugin-vendor-prefixer on the client.

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