React: What should portals do when container has a child managed by React?

[siddharthkp]

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

What is the current behavior?
ReactDOM.unstable_createPortal(<Component/>, target) appends the rendered component in the target instead of replacing the contents of the target

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/ebsrpraL/).

https://codesandbox.io/s/pjx8x9z2o7

What is the expected behavior?

It should replace the contents of the target with the new rendered component

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

[email protected]
[email protected]

Note: I might have completely misunderstood how portals work. @gaearon encouraged me to open this issue 😄

[gaearon]

I don’t quite understand the sandbox, how should I interpret it? How do you tell it’s appending rather than replacing? I only see a single child here (“bye”), not many.

[siddharthkp]

I start by rendering <Hi/> = <div id="hi" style={{border: '1px solid'}}>hi</div>
I'm wrapping this in a border to show the boundary of this component.

After that's done, I render a portal, which picks up document.getElementById("hi") and renders <Bye/> = <div>bye</div>

Now you'll see that <div>bye</div> is rendered inside the div#hi (because it's inside the border/boundary)

<div id="hi" style="border: 1px solid;">
  hi
  <div>bye</div>
</div>

Expectation: It should clear the hi and replace it with bye

<div id="hi" style="border: 1px solid;">
  <div>bye</div>
</div>

[gaearon]

Ooh I see what you mean.

I don’t think we support rendering portals into DOM nodes already having content managed by React. Just like you can’t ReactDOM.render() into a container already having other React children, and expect it to work.

We should probably clear and warn in this case. Not sure.

[siddharthkp]

I'm sure what the expectation is here, to be honest.

I think if should work similar to ReactDOM.render which does replace the content: https://codesandbox.io/s/mxmrxnknp

My slides explore the problems that can be solved by totally misusing portals.

[gaearon]

I don’t think this behavior makes sense to me with portals because there’s still a mounted React application that still “owns” that container div. Unlike the case with top-level render where you essentially tell the old application to stop running.

Consider this component:

const Parent = () => (
  <div id='hello'>
    <blink>Hi</blink>
  </div>
);

Now, if there’s a

function Child() {
  return createPortal(<marquee>Bye</marquee>, document.getElementByID('hello'))
}

somewhere, what do you expect to render?

You’re saying you’d expect <marquee> (portal child) to render alone. But what if then Parent’s parent calls setState(), and Parent re-renders? Its render clearly says: the child is <blink>Hi</blink>. But actually the child is different. Do you expect it to be overwritten again? Which of them should win? What happens to refs?

This sounds like a can of worms. In a single React application, two different components should not interfere with each other’s DOM. This is why it would make sense to me to disallow rendering portals into children managed by React.

The most restrictive version could look like this:

class Parent extends Component {
  portalContainer = document.createElement('div');

  componentDidMount() {
    portalContainer.id = 'hello';
    this.div.appendChild(portalContainer);
  }

  render() {
    return (
      <div>
        <blink>Hi</blink>
        <div id='hello' ref={node => this.div = node} />
      </div>
    );
  }
}

function Child() {
  return createPortal(<marquee>Bye</marquee>, document.getElementByID('hello'))
}

If we didn’t allow you to mount portals into React-managed nodes at all, this would be a minimal possible implementation. To be honest I actually like it: it’s very clear what is happening, and the bounds where the parent “exits” React data flow at the leaf are similar to how you do other DOM manipulations with leaf ref children.

The benefit of a strict approach like this is that render never lies. It’s true down to the leaf component, and as for leaf children, they’re explicitly managed manually. You don’t have this problem where Parent claims to render something but it’s not really in the DOM. And if you really wanted to clear its children, you’d have to do this explicitly by threading relevant state (either via props, or via sideways subscription system).

What do you think?

[gaearon]

A less restrictive version of this is when we let you render into a React node but only if it’s empty:

class Parent extends Component {
  render() {
    return (
      <div>
        <blink>Hi</blink>
        <div id='hello' />
      </div>
    );
  }
}

function Child() {
  return createPortal(<marquee>Bye</marquee>, document.getElementByID('hello'))
}

This is probably less annoying overall. But then we should warn if you attempt to render children in it:

        <div id='hello'>
          <blink>Hi</blink>
        </div>

And let’s not forget this can happen dynamically (first there’s no children, then portal child is created, then a normal child appears). So you might miss this situation until later.

And what if you have multiple portals rendering into the same node? Now that could become really confusing. You have no way to trace which one comes from where.

So I would prefer if we forced you to completely get out of “React-land” when creating a portal container.

[siddharthkp]

You're saying you'd expect <marquee> (portal child) to render alone. But what if then Parent's parent calls setState(), and Parent re-renders? Its render clearly says: the child is Hi. But actually the child is different

What I expected was that, by using a portal, you overwrite what the parent rendered and now even when the parent calls setState(), nothing happens because the parent doesn't control this child anymore! render totally lies now because we overwrote it.

This sounds like a can of worms. In a single React application, two different components should not interfere with each other’s DOM. This is why it would make sense to me to disallow rendering portals into children managed by React.

I absolutely agree with you on that and really like the example that you've given. It solves the problems that I was tackling and looks explicit at the same time.

A less restrictive version of this is when we let you render into a React node but only if it’s empty.
...
And let’s not forget this can happen dynamically (first there’s no children, then portal child is created, then a normal child appears). So you might miss this situation until later.

Nah, that's just very difficult to explain! It works differently on the first time you render and the subsequent times. No good.

So I would prefer if we forced you to completely get out of “React-land” when creating a portal container.

Totally makes sense to me!

P.S. Thanks for taking the time to correct my silly experiments! I'm going to close this issue.

[gaearon]

Let’s keep it open—we should at least add a warning when there’s a conflict. Or maybe get more restrictive and throw. I’d like to hear what @sebmarkbage thinks.

[siddharthkp]

Sure!

[jamiewinder]

I'm a little surprised this is an issue unless I'm misunderstanding something. Should I have to worry if more than one Portal is rendering to the same DOM node? I had assumed it'd just append (and it currently does). I'm not sure why that's a problem?

It'd be a pain to have to track whether a DOM node is already a portal host which you'd probably have to do for the very common portal scenario of rendering to document.body. A workaround would be to append a new container (document.createElement) and render to that (urgh).

[gaearon]

This issue is not about two portals rendering to one node, it is about a portal rendering to a node already managed by a regular React component.

[jamiewinder]

Ah, gotcha. Thanks

[ghost]

Well, the question about rendering portals in the same node is still actual. Scenario when the replace of the content is prefered is not something unusual. Actually I have one in almost every project I've ever had, so It would be nice to have such opportunity.

In my opinion, current behavior with appending of new nodes is quite confusing. If I want to be sure component will be rendered in separate node I can append new node manually for each component as it done in tutorial example

[ngryman]

The current implementation makes perfect sense conceptually and I understand the motivations behind this. But I wonder if the real world usage is coherent with the concept? 🤔

I'm pretty sure that most people use portals as placeholders and not living containers. They implicitly think that it will behave like React.render.

ReactDOM.unstable_renderSubtreeIntoContainer had this behavior back then, a good part of the code that was using it now partially breaks in React 16.

It might be a good idea to let people have the choice here so every use case is covered 🎉

@gaearon Would you consider to implement either an opt-in or another API for this? Are there any technical constraints in Fiber not to implement this?

[joetidee]

Just to clarify, if you were to do this:

const insertToast  = (ToastComponent ) => {
    const toastContainer = document.getElementById('toast-container');

    return ReactDOM.createPortal(
      <ToastComponent />,
      toastContainer,
    );
};

const ToastFoo = () => "foo";
const ToastBar = () => "bar";

insertToast(ToastFoo);
insertToast(ToastBar);

1) would toast-container contain both ToastFoo and ToastBar, or
2) would ToastBar replace ToastFoo, or
3) would React throw an error saying that toast-container already exists?

[OZZlE]

Just to clarify, if you were to do this:

const insertToast  = (ToastComponent ) => {
    const toastContainer = document.getElementById('toast-container');

    return ReactDOM.createPortal(
      <ToastComponent />,
      toastContainer,
    );
};

const ToastFoo = () => "foo";
const ToastBar = () => "bar";

insertToast(ToastFoo);
insertToast(ToastBar);

1. would toast-container contain both ToastFoo and ToastBar, or
2. would ToastBar replace ToastFoo, or
3. would React throw an error saying that toast-container already exists?

I expected 1 or 3 but it seems that right now it's 2

[rpellerin]

If you know what you're doing, here is a <Portal /> component that under the hoods creates a portal, empties the target DOM node and mounts any component with any props:

const Portal = ({ Component, container, ...props }) => {
  const [innerHtmlEmptied, setInnerHtmlEmptied] = React.useState(false)
  React.useEffect(() => {
    if (!innerHtmlEmptied) {
      container.innerHTML = ''
      setInnerHtmlEmptied(true)
    }
  }, [innerHtmlEmptied])
  if (!innerHtmlEmptied) return null
  return ReactDOM.createPortal(<Component {...props} />, container)
}

Usage:

<Portal Component={MyComponent} container={document.body} {...otherProps} />

This empties the content of document.body, then mounts MyComponent while passing down otherProps. Hope that helps. Any feedback appreciated.

[OZZlE]

Ooh I see what you mean.

I don’t think we support rendering portals into DOM nodes _already having content managed by React_. Just like you can’t ReactDOM.render() into a container already having other React children, and expect it to work.

We should probably clear and warn in this case. Not sure.

@gaearon I still don't see this mentioned in the documentation? Could it be added plz, maybe a Warning or Gotchas section or so.. before the actual instruction how to use it :) My new collegue just did the same misstake as I did way back and tried to do it like this.

I think it kind of works in certain scenarious but I can imagine that it's like playing with fire :D

[boltcoder]

I'm having similar use case. I currently have a 10000x10000 excel like UI where each cell has some interaction capabilities eg: opening a drawer with CTAs, drag, move, perform cell functions etc.
Instead of bloating each cell with functionality and context of already applied formulaes from my state container, I've created an interaction provider which catches events from cell and renders a portal into the {data-id:"id",data-colId:"col-id"} cell. using data-attributes in my selector.

On changing the container dynamically it fails to re-render the same component into the new one. Don't know if I'm over engineering here.

[boltcoder]

I'm having similar use case. I currently have a 10000x10000 excel like UI where each cell has some interaction capabilities eg: opening a drawer with CTAs, drag, move, perform cell functions etc.
Instead of bloating each cell with functionality and context of already applied formulaes from my state container, I've created an interaction provider which catches events from cell and renders a portal into the {data-id:"id",data-colId:"col-id"} cell. using data-attributes in my selector.

On changing the container dynamically it fails to re-render the same component into the new one. Don't know if I'm over engineering here.

I've found a way to do it. I'm now keeping a reference of the previous cell's dom element as well and rendering null in that container when the interaction switches to the next container. this seems to work for me.