[autoprefixer] Fix server side rendering

[oliviertassinari]

Should fix #2159 and #2119

[oliviertassinari]

I wondering if we shouldn't remove this method. It's no longer used.

[justinmc]

Confirmed that checking this branch out fixed the problem for me (I was having the problem in #2119). 👍

[bradparks]

Note that this branch doesn't solve a similar issue I'm having with the prefixer, that being that when I run it in a WKWebView in my OSX app, it fails with the following error, though it runs fine in Chrome and Safari:

[oliviertassinari]

@bradparks You should probably report this issue to the inline-style-prefixer repository.

[ghost]

@oliviertassinari Hello, I don't know your process, when do you think a new version with this fix can be released ?

[mfrye-intuit]

Same here. When do expect the next release to go out?

[mfrye-intuit]

@sutat I forked the repo and have a compiled version available https://github.com/mfrye-intuit/material-ui.

@oliviertassinari You guys have done a great job with this repo, but it would be nice if you practiced CI / CD. I've had to switch and pull from master so many times now because your latest code isn't pushed to npm.

[raydemandforce]

@oliviertassinari Do you have unit test for this fix?

[oliviertassinari]

@raydemandforce Nop, but it would be great to have unit test for the server side rendering. We broke it up two times lately.

[vedraan]

Tried the fix myself and while it does render the page, I'm still getting this in my node console (note that I did also update to inline-style-prefixer@0.5.1):

Material-UI expects the global navigator.userAgent to be defined for server-side rendering. Set this property when receiving the request headers.
Either the global navigator was undefined or an invalid userAgent was provided. Using a valid userAgent? Please let us know and create an issue at https://github.com/rofrischmann/inline-style-prefixer/issues
Material-UI expects the global navigator.userAgent to be defined for server-side rendering. Set this property when receiving the request headers.

And this in the browser console (I'm using the material-ui example code):

[bdsabian]

Same issue as @vedraan. Solved the navigator.userAgent problem by defining it, no problem. I'm still getting the "React attempted to reuse markup in a container but the checksum was invalid" warning in-browser though.

[ghost]

@mfrye-intuit Thank you I will try that this evening
@bdsabian How do you set this parameter please ? My question about this parameter is : does-it work with councurrent access ?

[vedraan]

@oliviertassinari I think this PR should not be pushed to a release, you should optionally only update the package.json to use the latest "inline-style-prefixer": "^0.5.1"

The solution to the problem presented in this PR is a hack and doesn't work too good as you can see in my earlier post. It creates rendering differences between the server and client and throws errors client-side.

The server _MUST_ know the user agent visiting the app, otherwise there is no guarantee that it will serve identical CSS as the one that will be generated client side.

The node console log message was actually very correct saying:

Material-UI expects the global navigator.userAgent to be defined for server-side rendering. Set this property when receiving the request headers.

It would just be good if you could post server side rendering instructions somewhere. Here is what I did and it works perfectly.

If you already installed from master, uninstall and install the latest NPM release instead:
npm install material-ui --save

Then, _in your server setup, for example if you are using Express to serve as I do, add this middleware before the route that is serving your application or before any other middleware you might have_:

app.use(function(req, res, next) {
    GLOBAL.navigator = {
        userAgent: req.headers['user-agent']
    }
    next();
});

And that's it! Perfect rendering, no errors in either node or browser console. I have tested using Chrome, FF, IE and simulating Googlebot and Facebook bot user agents.

[oliviertassinari]

The server MUST know the user agent visiting the app, otherwise there is no guarantee that it will serve identical CSS as the one that will be generated client side.

Yes, that's the original intent of #2007.
I agree, a note regarding the server side rendering in the documentation would be great.

I think this PR should not be pushed to a release

I'm not convinced, I think it can. However we could make three different improvement:

• Add what you said to the doc
• Use the React Context instead of a global variable to propagate the userAgent
• Allow the prefixAll mode in the browser and not only in the server

[vedraan]

I'm not a fan of globals either. I'm not sure if you're thinking of React context as an alternative. I'm fairly new to react, but have already seen a large number of different approaches to server side rendering so I'm not sure if there is a reliable and an easy to understand (to beginners at least) way of passing the req.header to React context. I've never used context prior to material-ui, didn't need to.

Perhaps instead of using globals something like this could be used in the middleware:

app.use(materialUiRendering());

Which would then handle the rest.

Regarding allowing the prefixAll in browser, I didn't really dig into how it all works (had no time) so I have no opinion about that part. As long as the server renders it the same as the client, it's good enough for me.

[bdsabian]

@sutat I wrote a simple middleware pretty much exactly as @vedraan describes above.

[ghost]

@bdsabian @vedraan This middleware is not thread-safe according to me.
As I understand NodeJS, there is only one thread, but 2 concurrents caller can overwrite globals variables cause of async.

_Exemple :_

• First caller, set the global variable "userAgent" for Firefox, then wait IO
• Second caller, set the global variable "userAgent" for IE, then generates the render (read userAgent === IE), return the response, end the transaction
• First caller is waking up, it generates the render (read userAgent === IE), return the response, end the transaction

⚠️ the First caller is generated with userAgent === IE but was called with userAgent === Firefox.

If I miss something, feel free to tell me RTFM, but please give me the manual I have to read !

[bdsabian]

@sutat, you're right. It should probably be moved to context as suggested
by @oliviertassinari https://github.com/oliviertassinari.

On Tue, Nov 17, 2015 at 12:36 PM, Sutat notifications@github.com wrote:

@bdsabian https://github.com/bdsabian @vedraan
https://github.com/vedraan This middleware is not thread-safe according
to me.
As I understand NodeJS, there is only one thread, but 2 concurrents caller
can overwrite globals variables cause of async.

Exemple :

• First caller, set the global variable "userAgent" for Firefox,
  then wait IO
• Second caller, set the global variable "userAgent" for IE, then
  generates the render (read userAgent === IE), return the response, end the
  transaction
• First caller is waking up, it generates the render (read userAgent
  === IE), return the response, end the transaction

[image: ⚠️] the First caller is generated with userAgent === IE
but was called with userAgent === Firefox.

If I miss something, feel free to tell me RTFM, but please give me the
manual I have to read !

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

[vedraan]

@sutat that's correct, node globals which are not constant are bad and should be avoided. The fact that such high concurrency will rarely happen doesn't mean this is OK. It fixes problems for my prototype app, but creating a middleware material-ui function which is not using globals is the right answer for production apps.

[Plumpernickel]

@vedraan awesome solution, dude. Thank you!

[alefherrera]

thank you very much for the solution, it worked perfectly

[adieuadieu]

So.. how do you create a middleware material-ui function to provide material-ui with GLOBAL.navigator, if Material UI is looking for the userAgent in GLOBAL.navigator? Does anyone have an example of this in code?

[adieuadieu]

@oliviertassinari sorry, i missed the last bit in your previous comment.

If I understand this correctly, I think this is the way to go on the server side:

Allow the prefixAll mode in the browser and not only in the server

E.g. On the server, use prefixAll. That kind of makes sense, given a "normal" situation, you'd likely have all the vendor prefixes in your CSS.

[oliviertassinari]

Allow the prefixAll mode in the browser and not only in the server

I wouldn't say that is the way to go on the server side. I proposed this to be more flexible.
I'm assuming that in some cases you don't know the userAgent.

If you know the userAgent, it's more efficient to only prefix for this userAgent.
With CSS, it's different, you have to prefix for all browsers since it's static.

[mattkrick]

If you know the userAgent, it's more efficient to only prefix for this userAgent.

Could you briefly mention why it's more efficient? In terms of size or processing? From my naive investigation, it looks like it's only used in 10 files. The auto-prefix function is called for every instance of every property, meaning that it has to be called 9 times for every TextField, 4 times for every RaisedButton etc. I'd assume just about everybody would be willing to trade that processing for a couple extra bytes of static inline styles. If my assumption is wrong, please let me know!

The bigger picture is that to make this work properly (via browser prefixAll or user-decision), we'd have to include a userAgent in our app's root component & material-ui would have to write context receivers on each of those 10 components (either that or turn the requires into functions, bleh). Complicating the API and adding the extra text to receive the context would IMO be a bad trade for saving a couple bytes.

[oliviertassinari]

Could you briefly mention why it's more efficient?

It's more efficient because less data are transfered down the wires. So, it's in terme of size and network.

[mattkrick]

Thanks for clarifying!
Regarding size down the wire, cutting out inline-style-prefixer and auto-prefixer would result in a large net savings & solve this problem fairly easily 😃

[vedraan]

@mattkrick I'm not sure if cutting out inline-style-prefixer and auto-prefixer would do more good than harm. That would mean having to rewrite every material-ui component to include the missing CSS properties to support all mobile and other browsers. Out of the box, a lot of them do not work well on all modern devices (for example late-gen Android phones) and they require CSS prefixes.
Rewriting each of them to support all devices (in addition to taking forever to complete) would probably mean introducing a lot of new bugs.

[mattkrick]

Agreed, making sure we gather all the css, even older browsers, would be a pain. If you were willing to explore it, we could do a 2 part PR where part 1 includes the static text & additionally runs AutoPrefixer.prefixAll and throws an error if the two objects fail a deep equal. Part 2 removes the AutoPrefixer sanity check.

The only other alternatives I see are not supporting SSR (current) or having each prefixed component look for a context.userAgent and have devs set that in their root component. I'll be happy to invest a couple hours in whatever option you guys think is best.