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

Add support for hydrating portals #13097

Open
marcusdarmstrong opened this issue Jun 22, 2018 · 35 comments
Open

Add support for hydrating portals #13097

marcusdarmstrong opened this issue Jun 22, 2018 · 35 comments

Comments

@marcusdarmstrong
Copy link

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
Copy link

MaxMotovilov commented Jul 17, 2018

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
Copy link
Collaborator

gaearon commented Jul 17, 2018

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
Copy link
Author

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
Copy link

MaxMotovilov commented Jul 17, 2018

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
Copy link
Collaborator

gaearon commented Jul 17, 2018

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
Copy link

MaxMotovilov commented Jul 17, 2018

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
Copy link
Collaborator

gaearon commented Jul 17, 2018

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

  • it will likely increase the code size
  • the current SSR implementation doesn’t support it so this adds more disparity
  • you can work around it with multiple hydrate calls (which wouldn’t preserve the context — but you wouldn’t have it on the server either)

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:

  • Renderer-independent code that tracks hydration state: https://github.com/facebook/react/blob/21ac62c77ac68c66b2a1f0adcc60f7760b884a0e/packages/react-reconciler/src/ReactFiberHydrationContext.js
  • Renderer-independent code that enters hydration code path:
    if (
    (current === null || current.child === null) &&
    root.hydrate &&
    enterHydrationState(workInProgress)
    ) {
  • DOM-specific implementation of hydration traversal
    export const supportsHydration = true;
    export function canHydrateInstance(
    instance: Instance | TextInstance,
    type: string,
    props: Props,
    ): null | Instance {
    if (
    instance.nodeType !== ELEMENT_NODE ||
    type.toLowerCase() !== instance.nodeName.toLowerCase()
    ) {
    return null;
    }
    // This has now been refined to an element node.
    return ((instance: any): Instance);
    }
    export function canHydrateTextInstance(
    instance: Instance | TextInstance,
    text: string,
    ): null | TextInstance {
    if (text === '' || instance.nodeType !== TEXT_NODE) {
    // Empty strings are not parsed by HTML so there won't be a correct match here.
    return null;
    }
    // This has now been refined to a text node.
    return ((instance: any): TextInstance);
    }
    export function getNextHydratableSibling(
    instance: Instance | TextInstance,
    ): null | Instance | TextInstance {
    let node = instance.nextSibling;
    // Skip non-hydratable nodes.
    while (
    node &&
    node.nodeType !== ELEMENT_NODE &&
    node.nodeType !== TEXT_NODE
    ) {
    node = node.nextSibling;
    }
    return (node: any);
    }
    export function getFirstHydratableChild(
    parentInstance: Container | Instance,
    ): null | Instance | TextInstance {
    let next = parentInstance.firstChild;
    // Skip non-hydratable nodes.
    while (
    next &&
    next.nodeType !== ELEMENT_NODE &&
    next.nodeType !== TEXT_NODE
    ) {
    next = next.nextSibling;
    }
    return (next: any);
    }
    export function hydrateInstance(
    instance: Instance,
    type: string,
    props: Props,
    rootContainerInstance: Container,
    hostContext: HostContext,
    internalInstanceHandle: Object,
    ): null | Array<mixed> {
    precacheFiberNode(internalInstanceHandle, instance);
    // TODO: Possibly defer this until the commit phase where all the events
    // get attached.
    updateFiberProps(instance, props);
    let parentNamespace: string;
    if (__DEV__) {
    const hostContextDev = ((hostContext: any): HostContextDev);
    parentNamespace = hostContextDev.namespace;
    } else {
    parentNamespace = ((hostContext: any): HostContextProd);
    }
    return diffHydratedProperties(
    instance,
    type,
    props,
    parentNamespace,
    rootContainerInstance,
    );
    }
    export function hydrateTextInstance(
    textInstance: TextInstance,
    text: string,
    internalInstanceHandle: Object,
    ): boolean {
    precacheFiberNode(internalInstanceHandle, textInstance);
    return diffHydratedText(textInstance, text);
    }

@MaxMotovilov
Copy link

@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
Copy link
Collaborator

gaearon commented Jul 17, 2018

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

@marcusdarmstrong
Copy link
Author

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
Copy link

MaxMotovilov commented Jul 17, 2018

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
Copy link
Author

@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
Copy link
Collaborator

gaearon commented Jul 17, 2018

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
Copy link
Author

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

@MaxMotovilov
Copy link

MaxMotovilov commented Jul 17, 2018

@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
Copy link
Author

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
Copy link

@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!

@gaearon gaearon changed the title Hydrating a Portal errors and duplicates contents Add support for hydrating portals Aug 2, 2018
enjoylife added a commit to enjoylife/baseui that referenced this issue Mar 3, 2019
We have to check for mount as the call to createPortal will error with `Portals are not currently supported by the server renderer...` if rehydrating after ssr. See, facebook/react#13097 (comment),  for rationale of the given changes.
enjoylife added a commit to enjoylife/baseui that referenced this issue Mar 3, 2019
We have to check for mount as the call to createPortal will error with `Portals are not currently supported by the server renderer...` if rehydrating after ssr. See, facebook/react#13097 (comment),  for rationale of the given changes.
@stale
Copy link

stale bot commented Jan 10, 2020

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.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@ljharb
Copy link
Contributor

ljharb commented Jan 10, 2020

Still relevant (stale bots are user hostile)

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Apr 9, 2020

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!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2020
@mrasoahaingo
Copy link

Is the support of SSR portals in the pipelines?

@stale
Copy link

stale bot commented Jul 11, 2020

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!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jul 11, 2020
@ljharb
Copy link
Contributor

ljharb commented Jul 11, 2020

bump

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jul 11, 2020
@svobik7
Copy link

svobik7 commented Aug 5, 2020

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

@stale
Copy link

stale bot commented Dec 25, 2020

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!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
@shwao
Copy link

shwao commented Dec 25, 2020

Still relevant

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
@gianjohansen
Copy link

bump

@LukasBombach
Copy link

@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
Copy link

LukasBombach commented Sep 5, 2021

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
Copy link

waiting for this too

@gabemeola
Copy link

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
Copy link

To my understanding server components are the answer to that

@gabemeola
Copy link

gabemeola commented Dec 21, 2022

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
Copy link

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
Copy link

mayank99 commented Feb 8, 2024

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).

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