Skip to content
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

Server side rendering #2336

Closed
chadrien opened this issue Dec 2, 2015 · 9 comments
Closed

Server side rendering #2336

chadrien opened this issue Dec 2, 2015 · 9 comments
Labels
package: system Specific to @mui/system

Comments

@chadrien
Copy link

chadrien commented Dec 2, 2015

Hi,

I just wanted to share a feedback I just encountered using material-ui.
When using server-side rendering, I had the now famous "React attempted to reuse markup in a container but the checksum was invalid" error, the diff being the following:

(client) <div style="did-flip:true;height
 (server) <div style="height:100%;backgrou

After some research, I found the source of the error: https://github.com/callemall/material-ui/blob/ae6147ca75dbe4b7229ca775bf95c1b0210e0875/src/utils/styles.js#L18-L25

Thing is, the server is running on production mode, so it doesn't go in the if, therefore don't add the did-filp:true to the element's styles, but client-side, as the variable is undefined, I go into the if and the did-flip:true is added, thus the mismatch.

I use webpack and have been able to use the following trick:

new webpack.DefinePlugin({
    process: {
        env: {
            NODE_ENV: JSON.stringify(process.env.NODE_ENV)
        }
    }
})

As material-ui/react is also something client-side, maybe the use of process should be avoided, isn't it?

Hope this can at least help others in my situation.

@chadrien chadrien changed the title Server side rendering in production mode Server side rendering Dec 2, 2015
@chadrien
Copy link
Author

chadrien commented Dec 2, 2015

I change the subject of the issue to server side rendering in general as I encountered yet another issue related to server-side rending.

Due to this line, there is again a mismatch error, because server-side left will be 0 (server = !isBrowser) and client side left will be x.
Removing the ternary to simply assign x solve this issue. So I'm wondering: what's the point in checking here if we're in a browser or not? If we don't really care, let's just assign x and avoid a mismatch error from React.

@oliviertassinari
Copy link
Member

Due to this line

You are not the first one to raise this issue.
I completly agree with you. That's not right.
I think that the left property shouldn't be changed and we should use transform2d or transform3d depending on the browser support.

@kand617
Copy link

kand617 commented Dec 3, 2015

@chadrien Thank you for the webpack fix.
I am so glad you posted this.

Its quite odd though, I am unable to produce the react warning running locally. however when running on heroku I get the warning....

@chadrien
Copy link
Author

chadrien commented Dec 3, 2015

@kand617: because locally your NODE_ENV is not set to production (probably not set at all btw) and on heroku it's set to production (check yourself with heroku run env | grep NODE_ENV), that's why.

@hourliert
Copy link

Same issue as @chadrien
Is there a pull request somewhere to fix that ?

@oliviertassinari
Copy link
Member

@hourliert Not I'm aware of. I'm implementing the server-side rendering on my project. I need and I plan to fix all the issue related to this. Still, if you want to help, that's welcomed 👍.

@hourliert
Copy link

I am not really aware of how style.js works. But if you could give me an hint on what I need to modify in ensureDirection, I could investigate and do the PR.

@oliviertassinari
Copy link
Member

Actually, I'm not sure it's an issue with material-ui. I would see this more like a feature.
In ordre to solve this warning on your project, you need to use the same process.env.environment on the client and on the server.

@igl
Copy link

igl commented Jan 22, 2016

Don't you need to do exactly this for React already? It's a big performance gain and results in a smaller bundle.

It's odd to have unisoversal libs depend on process.env.NODE_ENV but so it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: system Specific to @mui/system
Projects
None yet
Development

No branches or pull requests

6 participants