React: unmount an empty component is breaking with ReactDOM portals

[KhodorAmmar]

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

What is the current behavior? When unmounting a component that has a child being rendered under a different parent (with portals), react is throwing an error

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem:

https://codesandbox.io/s/73n31lwpjx

What is the expected behavior?

Component should unmount normally

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
16.8.1
Issue also happens with 16.7.0 (https://codesandbox.io/s/oxmpxmllvy)

The issue is only happening under very strict conditions:

• The component being rendered with ReactDOM Portals (Modal) should not render any HTML
• The parent component (Panel) should render Modal as the first component under

Avoiding this is as simple as moving Modal under some other HTML. I'm not entirely sure this is an issue or I'm just doing something wrong with Fragment and portals.

The actual error being thrown is:
react-dom.development.js:9254 Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

[gaearon]

Seems like the same as https://github.com/facebook/react/issues/14434.

Want to look into it?

[KhodorAmmar]

Sure, I might need guidance into where exactly to start looking though

[gaearon]

I'm guessing it's somewhere here

https://github.com/facebook/react/blob/c11015ff4f610ac2924d1fc6d569a17657a404fd/packages/react-reconciler/src/ReactFiberCommitWork.js#L741-L775

Or here:

https://github.com/facebook/react/blob/c11015ff4f610ac2924d1fc6d569a17657a404fd/packages/react-reconciler/src/ReactFiberCommitWork.js#L989-L1077

[KhodorAmmar]

Okay then I’ll take a look

[gaearon]

You might also want to start by making a failing test. Here's some existing tests for portals.

https://github.com/facebook/react/blob/c11015ff4f610ac2924d1fc6d569a17657a404fd/packages/react-dom/src/__tests__/ReactDOMFiber-test.js#L418-L462

[KhodorAmmar]

Would this work? the test creates a component with the same criteria as the demo above and removes the child component via a state update. It currently breaks

https://github.com/KhodorAmmar/react/blob/1aa3167b15b2eaa74f9afeac16d498a781b8c09b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js#L464-L496

Also is ReactDOMFiber-test the correct place for the test?

[KhodorAmmar]

Ok I was debugging the code and I believe I can pinpoint where is the issue coming from:

https://github.com/facebook/react/blob/c11015ff4f610ac2924d1fc6d569a17657a404fd/packages/react-reconciler/src/ReactFiberCommitWork.js#L1040-L1073

When the node is a HostPortal, and it is rendering nothing, and it is the first child under a Fragment, the currentParentIsValid is not being reset to false correctly in line 1071

Adding currentParentIsValid = false; in line 1044 is fixing the issue, all other tests are still passing.

I just feel that it's a too general solution? Am I on the right path at least?

[gaearon]

Thanks, this was very helpful. I sent a fix in https://github.com/facebook/react/pull/14820.

[gaearon]

Canary 0.0.0-0e4135e8c should have the fix. This will make it into 16.8.2.

[KhodorAmmar]

Can confirm it works in demo and my real life project

[gaearon]

Fixed in 16.8.2

[278kunal]

Still facing this in 16.8.6

[dcporter44]

@278kunal I was also receiving this same error in 16.8.6. However, I just discovered that I was receiving this error for a different reason than mentioned in this thread. I was using createPortal to append my component to a sibling container (Bad practice, but I had to do this as a workaround with a 3rd party library I'm using). My code looked something like this:

render() { return ( <div> <div id="container"></div> {ReactDOM.createPortal(<div>Append Me</div>, document.getElementById("container")) </div> ) }

When the page unmounted, React removed the container before it removed the actual portal. I fixed this by simply reordering the createPortal _before_ the sibling container I was appending to. That way React destroys the portal prior to the container. So it looks like this:

render() { return ( <div> {ReactDOM.createPortal(<div>Append Me</div>, document.getElementById("container")) <div id="container"></div> </div> ) }

Just thought this might help someone else :)

[ecfy]

@dcporter44 You're an absolute life-saver. Had exactly the same issue (injecting into thirdy-party sibling) - switching the rendering order fixed it for me too. Thanks!

[jsphstls]

I still see this in 16.12.

Repro: https://codesandbox.io/s/portal-remove-child-error-sbnj2

[tommedema]

Some of our users are getting this error sporadically in production (NotFoundError: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.), but I've been unable to reproduce it. We don't use portal directly but perhaps one of our dependencies does.

[talolard]

Some of our users are getting this error sporadically in production (NotFoundError: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.), but I've been unable to reproduce it. We don't use portal directly but perhaps one of our dependencies does.

Same here

[tayan-serna]

I did a little validation to avoid the crash.

 useLayoutEffect(() => {
    modalRoot.appendChild(el);
    return () => {
      // check if modalRoot have children
      if (modalRoot.children.length) {
        modalRoot.removeChild(el);
      }
    };
  }, [visible]);

In this way, my app is not crashing.

[hgeldenhuys]

I also started getting this, but in the strangest place. At first I thought it was in react-sortablejs, since it does DOM manipulation, but in React 16.12.0 it worked, and gave this error in 16.13.1.

Turns out I was doing something wrong with useSelector in Redux. I basically returned a copy of my redux state, so the selector subscription was changed every time a rerender was triggered. Wasn't an obvious fix, I really had to backtrack and go through a lot of things and fixed my useSelectors with proper selector function.

Think in my case it was zombie children:
https://react-redux.js.org/api/hooks#usage-warnings

[jsphstls]

Validation cannot actually prevent the crash. As @gaearon says here, React:

repeats what you’ve already done, causing failures

React does not repeat the validation, only the removal.

[xianghongai]

• Node.removeChild() - Web APIs | MDN

Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.

fix:

   unmountContainer() {
      // macro task
      setTimeout(() => {
        // refs
        if (containerEle) {
          // receive removed node
          // eslint-disable-next-line no-unused-vars
          let removedChild = document
            .querySelector('.page__wrapper')
            .removeChild(containerEle);
        }
      });
    }