facebook / react

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

Make React more friendly with other DOM-Manipulation libraries #24720

Closed erkebek closed 6 months ago

erkebek commented 2 years ago

Currently I am working on integrating React into project with existing UI-Framework. The problem is that both of them can modify the DOM. For the old framework that's not a problem, but for React it's not. Every time something get changed in DOM React has to sync the actual DOM with it's Virtual DOM. Adding and updating of elements are solved by manual rerendering. But removing of elements lead to the exceptions "Uncaught DOMException: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node."

We tried to implement our custom renderer with react-reconciler. But later we found out that we only need to extend the current react-dom package. So the problem was solved by adding if statements to the ReactDOMHostConfig.js:

export function removeChild(
  parentInstance: Instance,
  child: Instance | TextInstance | SuspenseInstance,
): void {
  // remove only if an element exists
  if (parentInstance.contains(child)) {
    parentInstance.removeChild(child);
  }
}

export function removeChildFromContainer(
  container: Container,
  child: Instance | TextInstance | SuspenseInstance,
): void {
  if (container.nodeType === COMMENT_NODE) {
    (container.parentNode: any).removeChild(child);
  } else {
    // remove only if an element exists
    if (container.contains(child)) {
      container.removeChild(child);
    }
  }
}

Would it be possible to apply this update to the official version?

We understand that not all users use React like this. But keeping in mind that one of the strengths of react is that it can be embedded in any web application and can be used with other DOM-Manipulation libraries.

This change would make the integration process of React with other libraries even more simpler.

gaearon commented 2 years ago

What exactly does your library do and how do you integrate it? Can you give some examples that break?

erkebek commented 2 years ago

We have a UI-Framework (used only in our company) which is similar to React has it's own components called widgets for UI. One of them is ReactWidget, if all other widgets are rendered completely by old framework, for ReactWidgets only container HTML-Element is rendered with some attributes (component name, id) and the rest must be rendered by React.

Integration is done by using React.Lazy (to load components by given name), React Portals (to render components in the given container from old framework).

So sometimes old framework must unmount the ReactWidget (container) which is also removes all it's children rendered by React. In this case we try to remove them from React's Virtual DOM as well to avoid memory leaks and to keep the Virtual DOM and actual DOM consistent. This is done by storing rendered React Components in an array, when old framework changes the DOM an event is fired, React part must remove unmounted components from Virtual DOM and render again. In this case React tries to remove an HTML-Element which was already removed by old framework, and the exception gets thrown. That's why we added an if check before removing an HTML-Element, which has solved our problem.

DinarSharipov commented 2 years ago

Same problem. when using google browser translator. Error: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node. at removeChild

paztis commented 1 year ago

same for me with react-apexcharts library. I create a portal inside on of their nodes are it crashes when the portal tries to unmount.

the problem is it totally crashes my app now instead of just displaying an error in the logs Can't this be fixed as said in 1st comment ? You alway remove it form Virtual DOM and remove it from dom only if it is still under the container for example

paztis commented 1 year ago

By the way until a fix is done, how can I prevent my app to crash because of it ?

paztis commented 1 year ago

This bug is causing me more and more issues. Can't you just add a try/catch over container.parentNode.removeChild(child); ?

There's no way on our side to avoid this defect. It appears more and more when we're creating a Portal over another React component node

Please give a status to this

github-actions[bot] commented 6 months 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!

github-actions[bot] commented 6 months ago

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!