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

ReactDOMServer.renderToStaticMarkup gets stuck while trying to render Portals #11692

Closed
amoshg opened this issue Nov 28, 2017 · 11 comments
Closed

Comments

@amoshg
Copy link

amoshg commented Nov 28, 2017

Calling ReactDOMServer.renderToStaticMarkup(elementWithPortal); gets stuck in a loop while trying to do a static render.

Current behavior:
The behavior can be seen here: https://codepen.io/anon/pen/BmqWOM by un-commenting line 58 (warning: this will make your tab freeze)

Expected behavior:
It should return with results or throw some kind of error if the input is not valid

@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

Want to dig into why this happens? I'd expect an error there.

@amoshg
Copy link
Author

amoshg commented Nov 29, 2017

It seems to enter this while loop while (out.length < bytes) {...} inside ReactDOMServerRenderer.prototype.read and the stack keeps building up elements indefinitely causing an infinite loop. Not familiar with the render loop but could it be caused by a circular reference somewhere?

Is it not possible for it return the markup? What kind of error would you expect?

@aweary
Copy link
Contributor

aweary commented Nov 29, 2017

It looks like it's because React.isValidElement returns false for Portals, so resolve in ReactPartialRenderer keeps returning the same Portal element, which keeps getting pushed into the stack

@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

I think we want to throw when isValidElement is false. We fail for objects, don't we?

@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

Oh. We used to fail in toArray(). But now it actually accepts portals. I guess we need an explicit check somewhere.

@aweary
Copy link
Contributor

aweary commented Nov 29, 2017

So is the expected behavior that we throw an error when trying to render a Portal using ReactDOMServer?

@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

Yes. You can't render them anyway because they throw on a missing DOM node. And you can't have a DOM node on the server. So you're supposed to skip them in the server code.

@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

I have a fix in #11709.

@gaearon gaearon closed this as completed Nov 29, 2017
@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

Okay. We'll include the fix for the hanging loop in the next release (which might take a couple of weeks). It's not common anyway since to reproduce it you'd need to server render on the client.

But still, this use case is currently unsupported, and you'd need to pass data down your components to not render the portal in this case. I would also say it's not super great to use SSR renderer on the client because it brings extra bytes. You might as well ReactDOM.render into a new div and then read its innerHTML.

@amoshg
Copy link
Author

amoshg commented Nov 29, 2017

Thanks!
The way we came across this issue was during our unit testing.... we use Enzyme's render method to generate static snapshots of our components and compare them with previous snapshots. The tests are ran in the browser using Karma.... Enzyme's render calls renderToStaticMarkup under the hood.

@gaearon
Copy link
Collaborator

gaearon commented Nov 29, 2017

Oh I see. Thanks for explaining the context.

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

No branches or pull requests

3 participants