facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.32k stars 46.96k forks source link

Add support for hydrating portals #13097

Open marcusdarmstrong opened 6 years ago

marcusdarmstrong commented 6 years ago

Do you want to request a feature or report a bug?

Probably bug, but arguably a feature request, I suppose.

What is the current behavior?

I've attempted my best effort at a fiddle that shows off the particular issue. Obviously server side rendering is impossible via JSFiddle, but the markup should be equivalent to having rendered Test into a div with id test-1 during server side render.

https://jsfiddle.net/y8o5n2zg/

As seen in the fiddle, an attempt to ReactDOM.hydrate() a portal results in:

Warning: Expected server HTML to contain a matching text node for "Hello World" in <div>.

Additionally, after failing to hydrate, React renders the component and appends it resulting in a duplicated section of DOM:

<div id="test-1">Hello WorldHello World</div>

What is the expected behavior?

In an ideal world, calling hydrate on a component that has portals would allow those DOM containers to hydrate into the components they were rendered with.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

I've only tested this in 16.4.1, but I've confirmed the behavior in Chrome and Firefox. Given that I'm really looking at an edge case here I doubt it worked previously.

Why I'm doing this edge-case-y nonsense:

We're currently using multiple React roots on our pages (as some portions of the pages are not rendered by React yet), most of which are server-side rendered. We'd like to be able to hydrate them into a single React root on page, so that we can share contexts between them without difficulty and without repeating those context components in memory (in some cases we can have a good number of roots on the page—20-30, perhaps?).

In searching, I found a few potentially related bugs (#12615, #10713, #11169), but it seemed like these really didn't line up with my (hopefully valid?) use case.

Thanks!

MaxMotovilov commented 6 years ago

Nearly identical use case here. For now, resolving with a kludge in the component rendered into a portal (render self with display:none, detect duplicate, delete, show self).

gaearon commented 6 years ago

When you hydrate, the initial render should match your server render. But portals are currently not supported on the server. Therefore, hydrating a portal doesn't make sense with current limitations.

I think you want to do something like:

state = {mounted: false};

componentDidMount() {
  this.setState({ mounted: true });
}

render() {
  return <div>{this.state.mounted && ReactDOM.createPortal(...)}</div>
}

Does that make sense? Same workaround as you need to use when your client render doesn't match.

marcusdarmstrong commented 6 years ago

Thanks for following up, Dan!

I can't speak for Max here, but in my use case, we have no intention of rendering portals on the server (obviously that concept makes very little sense without a real dom to portal into). We'd like to use a multi-root approach on the server, but then when it comes time to hydrate on the client, declare each of those server-side-rendered roots to a special root component that can hydrate each of those into a Portal on the client, so that we end up with a single React root in-browser.

In the case described by the Fiddle linked above, an initial render does match the server render (both want to have "Hello World" in the test div)—it's just that the mechanism on the server for creating the Portal divs is external to React (just as it is on the client in the case of a Portal).

That is: https://jsfiddle.net/7y3kcnbh/ Rendering these components on the client yields the same thing as my hypothetical server-side-rendered markup in the original fiddle: https://jsfiddle.net/y8o5n2zg/

Rendering the portal on mount doesn't really help here, because it misses out on the opportunity to hydrate the various server-side rendered components that are in their respective DOM roots.

MaxMotovilov commented 6 years ago

Well, my use case is somewhat more convoluted than that. It is not a true hydration: I am loading a "foreign" page (JSP, WordPress -- that sort of thing) that wants to instantiate multiple React sub-applications aware of each other. To smooth out the load experience (and let google see all of content), the page contains a copy of the initial DOM recorded from the browser (call it a poor man's SSR :-) ). Because the final decision on which sub-applications to instantiate at all belongs to the page, they implement a handshake protocol to build a single vDOM tree (and Redux store etc) and decide which of them will go into portals. Thus I am not really re-rendering the container node for the portal at all and have to stick to cleaning up siblings after render.

Edit: now that I read @marcusdarmstrong's comment above, I think our use cases are indeed very similar, except for the top-level embedding mechanism.

gaearon commented 6 years ago

In the case described by the Fiddle linked above, an initial render does match the server render (both want to have "Hello World" in the test div)

I see where you're coming from, I just explain why the current behavior isn't so much a bug but a missing feature. From React's point of view, the initial render does not match the server render because portals are not supported on the server. Therefore, the portal encountered on the client is considered a new thing that needs to be inserted (rather than hydrated).

I agree that hydrating portals could be useful even before React SSR supports it.

MaxMotovilov commented 6 years ago

What if createPortal() could be explicitly told that yes, portal element does already contain pre-rendered DOM we need to diff against? Isn't this sort of what hydrate() does now -- passes in a flag that overrides the check for existing copy of the DOM? I understand it would be a bit of a kludge -- and place the responsibility squarely on user's shoulders as yet another __dangerously__ feature -- but probably much easier to implement than server-side portals in their entirety?

gaearon commented 6 years ago

I think if somebody implements this we can take a look at the PR. It's not a priority for us because:

There are plans for a different SSR implementation that would support “modern” features like error boundaries and Suspense. I think it would make sense to add full support for portals at the same time, with client and server parity.

But again, if somebody sends a PR we can take a look. Here's a few interesting places in code:

MaxMotovilov commented 6 years ago

@gaearon - fair enough, and thanks a lot for the pointers! I will probably stick with the simple kludge I have for now, unless synchronous replacement of the tree (as opposed to true hydration) proves to be an issue.

gaearon commented 6 years ago

You can still hydrate if you don't use portals and instead perform several hydrate calls when mounting the app.

marcusdarmstrong commented 6 years ago

Thanks for the hints! I suspected this might be more of a "by design" type thing. I'll go ahead and add all this to our internal ticket on the subject. It's possible somebody on my team might go ahead and take a look at this. The biggest motivations for us here are that we can use local state rather than module state for our context objects, so it's really an optimization more than anything else, so we'll see how the prioritization goes.

MaxMotovilov commented 6 years ago

You can still hydrate if you don't use portals and instead perform several hydrate calls when mounting the app.

I guess I don't quite understand what will I get in this case. Wouldn't this result in multiple independent vDOMs instead of a single common one I build now? Clearly the contexts will be independent as well, so no common instances of <Provider>, <BrowserRouter> and such; every sub-app would have to be wrapped separately and proper sharing of global resources (location, Redux store et. al.) can only be ensured by these wrapper components. Sounds a bit scary as I can't be sure offhand if this use case is supported by all of the service libs currently in use...

marcusdarmstrong commented 6 years ago

@MaxMotovilov That's what we're doing, for what it's worth. Our on-page "runtime" handles wrapping everything in Providers and whatnot all pointing to the same store instance before hydrating.

gaearon commented 6 years ago

Clearly the contexts will be independent as well, so no common instances of , and such

Right, but don't you have the same problem on the server anyway? Since SSR doesn't support portals.

marcusdarmstrong commented 6 years ago

It's generally less of a problem on the server because (at least our) contexts don't mutate on the server.

MaxMotovilov commented 6 years ago

@marcusdarmstrong - what about the router lib and i18next? Do you use those; any problems with this use case?

@gaearon - we don't use SSR at all, too many (HTML-producing) legacy backends to take care of. My concern related to not having a single page-wide context in the frontend code while managing common/global resources.

marcusdarmstrong commented 6 years ago

Our routing and internationalization approaches are quite custom, but fundamentally they all work via the same mechanism of a shared store provided to multiple roots by our "runtime", that coordinates all the roots on the page.

MaxMotovilov commented 6 years ago

@marcusdarmstrong - Makes sense. It still appears to me that our current approach of building a common vDOM as part of the handshake should suffice for the time being; as you said, hydrating vs. replacing subtrees is mostly a matter of optimization. Thanks a lot for the feedback too!

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

ljharb commented 4 years ago

Still relevant (stale bots are user hostile)

stale[bot] commented 4 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any additional information, please include with in your comment!

mrasoahaingo commented 4 years ago

Is the support of SSR portals in the pipelines?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

ljharb commented 4 years ago

bump

svobik7 commented 4 years ago

Pretty old issue 🙂 is this in React roadplan or it has no priority?

stale[bot] commented 3 years ago

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

shwao commented 3 years ago

Still relevant

gianjohansen commented 3 years ago

bump

LukasBombach commented 3 years ago

@gaearon will calling hydrate multiple times run multiple reconcilers concurrently? I am wondering, if that is the case, that optimizations to reduce browser repaints are lost to some extend.

LukasBombach commented 3 years ago

Because I just wanted to be really smart and only partially hydrate my page with a single render root and portals instead of using multiple render roots and now I am rendering a button inside a button instead of hydrating it XD

https://github.com/LukasBombach/next-hyper-performance

But that won't be necessary I guess if using hydrate mutliple times has no impact on repaint performance [or possibly concurrency optimizations in the reconciler]

miguelcaravantes commented 2 years ago

waiting for this too

gabemeola commented 1 year ago

Would be great to add for partially hydrated SSR apps. The proof is working well but the flash of content when re-rendering is a pain.

LukasBombach commented 1 year ago

To my understanding server components are the answer to that

gabemeola commented 1 year ago

To my understanding server components are the answer to that

Agreed, more or less if you're using a framework which controls the full page (e.g. Next / Remix). Portal hydration could work alongside RSC and better for "island" microfrontend architectures that you see in Astro and Qwik

stephan-noel commented 11 months ago

Now that React Server Components are here, just wanted to check if the story around this has changed or if it's still relevant?

Seems like it could still help avoid a useLayoutEffect on first render.

mayank99 commented 9 months ago

Just wanted to share a workaround I'm currently using:

  1. Since createPortal doesn't work during SSR, I use a user-land server portal implementation to render the markup in the correct place.
  2. Since the server-generated markup cannot be "hydrated", I blow away whatever was rendered on the server, and then call createPortal.

Simplified code:

useEffect(() => {
  if (container && !ssrMarkupRemoved) {
    container.replaceChildren();
    setSsrMarkupRemoved(true);
  }
}, [container]);

return container && ssrMarkupRemoved && createPortal(<Stuff />, container);

It's not perfect, as the node will be recreated from scratch (DOM state such as focus could be lost), but it's the best solution I've found if SSR is a necessity. It's better than manually calling hydrate, which would create a completely separate client tree detached from the main app (breaking context).