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

NavigationDrawer with mobileDrawerType or drawerType results in invalid checksum with Server-Side Rendering #236

Closed
frol opened this issue Feb 2, 2017 · 7 comments
Labels
Milestone

Comments

@frol
Copy link
Contributor

frol commented Feb 2, 2017

Description

I am using Next.js, which offers SSR (server-side rendering) out of the box. Recently, I bumped into an issue when passing mobileDrawerType or drawerType I get:

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) ton></div></header><!-- react-empty: 15 
 (server) ton></div></header><span data-reactid="1

Everything works fine, but React throws everything away instead of just connecting to existing DOM.

Using only tabletDrawerType and desktopDrawerType doesn't raise the warning. Also, drawerType of non *_MINI work fine. I guess that SSR may get detected and rendered as a mobile version, but later on the client side, it turns out to be a desktop.

Link to a gist or code sample where the issue can be reproduced

https://github.com/zeit/next.js/tree/master/examples/with-react-md

Version

  • React 15.4.2
  • React-MD 1.0.2
  • Next.JS 2.0.0.beta20
@mlaursen
Copy link
Owner

mlaursen commented Feb 3, 2017

Just to remind myself when I fix this.. This is a bug because of the simple implementation of SSR for the Portal component. When a Portal is visible from the server, it automatically renders it inline, but when it makes it to the client, it isn't expecting it to be rendered inline and creates a new portal of the child. Either need to update the portal to have some way to detect if it was initially visible from the server and render inline again, or add a way to make the portal optional. It is seeming more and more worthless as a SSR component. Can't remember why I implemented it...

@mlaursen
Copy link
Owner

mlaursen commented Feb 3, 2017

Actually, I think this is a bit more of a problem with not setting the defaultMedia correctly on the NavigationDrawer. Is there any way to do something like this with next.js? When the defaultMedia is set correctly from SSR, that message doesn't appear.

@frol
Copy link
Contributor Author

frol commented Feb 3, 2017

I have tried setting it to 'desktop' before I submitted this issue. It didn't help :(

@mlaursen
Copy link
Owner

mlaursen commented Feb 3, 2017

Darn, ok. I'll look into it deeper then. I am still thinking I should make the Portal optional. Might solve a few problems :)

@mlaursen mlaursen added this to the v1.1.0 milestone Feb 7, 2017
mlaursen added a commit that referenced this issue Mar 7, 2017
Since the Server Side Rendering with the Portal ended up being a giant
mess, most components do not use the Portal by default anymore. The
Portal can still be enabled if this is preferrred behavior by providing
`portal` to the components.

This should close #230 and theoretically #236
@mlaursen
Copy link
Owner

This has been fixed with #230

@frol
Copy link
Contributor Author

frol commented Mar 15, 2017

@mlaursen Great news! Thank you!

By the way, what is the current state of 1.1.x? Is it stable enough to be published as "beta"?

@mlaursen
Copy link
Owner

Sadly, not yet. It might be a week before it is in a more stable state for a "beta". I believe there is currently an alpha out that has this fix in it, but upgrading to alpha might be a bad idea :)

I'm about 2-3 days away from finishing #145 which will be one of the last more "app-changer" tickets since it modified menus and components that used menus so much. I'll do some more issue pruning/thoughts and figure out if anything might be terrible for migrating and publish a beta then. The SVGIcon (#253) and Fixed Table Header/Footer (#197) might be considered an app changer which might delay the beta. Otherwise, it seems like the other issues are more behind-the-scenes/no-changes-required/new-features sort of things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants