Unhandled Rejection (NotFoundError): Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

[brenthmiras]

I have a component
<ComponentWithVideo /> that contains a list of tracks that are being rendered in a single <video/> element.

That I rendered with <InPortal /> and displayed to <OutPortal />

This error occurs when I add another track and <ComponentWithVideo(s) /> re-renders.

[pimterry]

Hmm, I see, thanks for reporting this. In general that shouldn't happen, and that doesn't happen in the examples here, so there's something unusual in your use case that we haven't been testing.

I'm not sure how to reproduce this from your description though, unfortunately. Can you provide a small example of the issue, e.g. by editing the examples in this repo (https://github.com/httptoolkit/react-reverse-portal/blob/master/stories/html.stories.js) or using codesandbox.io or similar?

If that's not practical, if you could share the relevant code from your application then that might help shine a light on the issue.

[pimterry]

Going to close this for now, but I'm happy to look into it further later if you can provide some more info.

[Mesoptier]

Reproduction
This issue occurs when conditionally rendering a component positioned directly before the <OutPortal> component. I've created a simple CodeSandbox to reproduce the issue:

https://codesandbox.io/s/misty-cookies-jztzjq?file=/src/App.js

import { useState } from "react";
import { createHtmlPortalNode, InPortal, OutPortal } from "react-reverse-portal";

export default function App() {
  const portalNode = createHtmlPortalNode();
  const [crash, setCrash] = useState(false);

  return (
    <div className="App">
      <button onClick={() => setCrash(true)}>Click here to crash!</button>
      <InPortal node={portalNode}>Portal contents</InPortal>
      {crash && <div />}
      <OutPortal node={portalNode} />
    </div>
  );
}

Cause
It looks like this has to do with the internals of React. When rendering a new element, at some point React needs to call parentNode.insertBefore(newNode, referenceNode) to actually insert the new node. However, the referenceNode has been replaced with the node created by createHtmlPortalNode and so isn't actually a child node of parentNode anymore. This causes the following error:

Uncaught DOMException: Failed to execute 'insertBefore' on 'Node': The node before which the new node is to be inserted is not a child of this node.

Solution?
Unfortunately, I don't think it's possible to solve this problem without messing with the internals of React, which is probably not desirable.

One thing that might work, is to render an empty text node directly before the <OutPortal> component. React can happily insertBefore the text node. And the text node doesn't add any additional (wrapping) elements, which would clash with other scripts and styles. Only it does affect some DOM stuff, like node.nextSibling, which may now return the empty text node instead of the rendered portal node.

This "fix" is as simple as this:

import { useState } from "react";
import {
  createHtmlPortalNode,
  InPortal,
  OutPortal
} from "react-reverse-portal";

export default function App() {
  const portalNode = createHtmlPortalNode();
  const [crash, setCrash] = useState(false);

  return (
    <div className="App">
      <button onClick={() => setCrash(true)}>Click here to crash!</button>
      <InPortal node={portalNode}>Portal contents</InPortal>
      {crash && <div />}
      {"" /* <- I only added this empty text node */}
      <OutPortal node={portalNode} />
    </div>
  );
}

[pimterry]

Hi @brenthmiras, thanks for reporting this! That makes sense, although I haven't run into it myself anywhere before, really interesting find.

I think your suggested solution makes perfect sense, that's a great idea. Would you be open to opening a PR to make OutPortals add this extra empty text node automatically?

[j-tai]

Another workaround is to simply wrap the OutPortal in a div:

<div>
  <OutPortal node={node} />
</div>

This is less likely to affect the DOM or the styling.

[pimterry]

That would work, and it's a nice quick workaround you can use to fix this today.

It does affect the DOM structure though in a way that could affect styles (e.g. direct descendent selectors) which I'd like to avoid.

Using an empty text node doesn't have that problem, and should work equally well as a proper fix, so while an extra div is good as a quick workaround, I'd like to avoid implementing that as the real solution.

[Mesoptier]

Disclaimer: I'm not actually using react-reverse-portal. But I am doing something similar to it for a project at work, so I figured I'd share my findings at the time in my previous comment.

---

It does affect the DOM structure though in a way that could affect styles (e.g. direct descendent selectors) which I'd like to avoid.

This is exactly the problem I ran into, I have some existing styles using direct descendent and sibling selectors, which would both break by adding a wrapping <div> around the "out portal".

The problem described in this issue—where React tries to insert a new element before the detached placeholder node—can be fixed by rendering the empty text node before the "out portal".

---

However, I also ran into another problem which was not fixed by the empty text node and forced me to add wrapping <div> elements around the "out portal" after all.

That problem occurs when the order of two "out portals" (including empty text nodes) is changed. Here, React doesn't know that the "placeholder" node was swapped for the "replaced" node. So when the order of the "out portals" changes, React just moves the empty text node and placeholder node in the DOM to the new location, leaving the replaced node behind.

Consider this example (not real code):

<OutPortal key="A" name="A" />
<OutPortal key="B" name="B" />

Then this is what happens in the DOM after the order of these components in swapped:

	What is actually in the DOM	What React thinks is in the DOM
Initial render	Text('') Element(replaced A) Text('') Element(replaced B)	Text('') Element(placeholder A) Text('') Element(placeholder B)
Re-order "out portals"	Element(replaced A) Text('') Element(replaced B) Text('') Element(placeholder A)	Text('') Element(placeholder B) Text('') Element(placeholder A)

At this point I gave up on trying to keep away from wrapping elements, because it felt like any fix would rely too much on React internals and could break at any moment or in weird edge cases.

---

I haven't tried reproducing this other problem with react-reverse-portal, but I expect the same problem will probably occur here.

One of the examples does something similar to this (this example), but there the actual <OutPortal /> components aren't re-ordered, only the node props are.

[pimterry]

Oooh, that's really interesting, and yes that's a complicated case that's not handled well right now.

Any idea what the actual DOM transformations React is doing there are? The insertBefore in the previous case made sense, and seemed easy enough to handle, but it sounds like this is doing something different.

In the meantime, you can probably handle this (and many other similar cases) by using key to force React to re-render the OutPortal during this update, instead of moving it. That way, React will first call componentWillUnmount on the OutPortal (triggering the logic here that resets the placeholders), then it will mutate the DOM with placeholders everywhere as it expects, then it will mount a new OutPortal, and React-Reverse-Portal can then kick in and put the portal node content back. Since the actual DOM content lives inside your portalNode, making the OutPortal effectively stateless, that should be very cheap and totally invisible as a user.

[Minf97]

Another workaround is to simply wrap the OutPortal in a div:另一种解决方法是简单地将 OutPortal 包装在 div 中：

<div>
  <OutPortal node={node} />
</div>

This is less likely to affect the DOM or the styling.这不太可能影响 DOM 或样式。

It does work!! Thanks