React: unmountComponentAtNode works async ?

[qhxin]

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

bug

What is the current behavior?

i tried to use ReactDOM.unmountComponentAtNode and ReactDOM.render to manage my multi roots. i found that if i use them in sync code, i get an error just like this when i change route pages:

Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by React and is not a top-level container. Instead, have the parent component update its state and rerender in order to remove this component.

Warning: render(...): Replacing React-rendered children with a new root component. If you intended to update the children of this node, you should instead have the existing children update their state and render the new components instead of calling ReactDOM.render.

i use browserHistory to change route.
my code is written in componentDidMount method of a root React component:

Page1.js

  componentDidMount() {
    const a = document.getElementById('root2');
    ReactDOM.unmountComponentAtNode(a);
    ReactDOM.render(
      <A />,
      a
    );
    ReactDOM.render(
      <B />,
      document.getElementById('root3')
    );
  }

Page2.js

  componentDidMount() {
    const a = document.getElementById('root2');
    ReactDOM.unmountComponentAtNode(a);
    ReactDOM.render(
      <C />,
      a
    );
    ReactDOM.render(
      <B />,
      document.getElementById('root3')
    );
  }

And then, if i use setTimeout to wrap ReactDOM.render after ReactDOM.unmountComponentAtNode(a);, the result can be success.

What is the expected behavior?

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

react 16.5.2
react-dom 16.5.2

[gaearon]

Can you please provide a single runnable complete example that reproduces the issue. For example as a fiddle.

[qhxin]

@gaearon this is my demo. thanks.

[qhxin]

@gaearon if i don't use unmountComponentAtNode, React Root can clear the old Component's events/doms/state and sub components by its self? or the old component will unmount automatically ？

[AyWa]

Hello,

I tried to investigate with the example that you gave. (but note that It is the first time I am checking reactDOM source and just want to try to contribute in react 😃)

So first, to do what you want to achieve:
you do not need to call unmountComponentAtNode if you just plan to render/rerender a react component.

• In case of rendering the same component => it will just rerender properly (without unmount / mount a new component)
• In case of rendering an other component => it will properly unmount the previous component and render the new one.

You can easily test those 2 behavior by:

• removing ReactDOM.unmountComponentAtNode(ele)
• adding: componentWillUnmount() {console.log("unmount")} in component 2/1 and check that the behavior is correct ;)

However I think the code that you share should work correctly and this my investigation:

bug behavior

• first button click -> ReactDOM.unmountComponentAtNode(ele) unmount something empty -> it return false which is the correct behavior (from the doc): false if there was no component to unmount.
• component 1 is render correctly and _reactRootContainer is set correctly
• second button click -> ReactDOM.unmountComponentAtNode(ele) unmount a react component -> it return true which is the correct behavior (from the doc): Returns true if a component was unmounted
• component 2 is render -> component 1 is unMount ->_reactRootContainer will be set to null for the container (which is the bug)
• next click, ReactDOM.unmountComponentAtNode(ele) will have some error and return false, and will not unmount correctly component 2. The root cause of that is _reactRootContainer which is null.

I guess the root cause might come from https://github.com/facebook/react/blob/144328fe81719e916b946e22660479e31561bb0b/packages/react-dom/src/client/ReactDOM.js#L684 + the uncorrect order of unmount / mount ?

If we wrap all the code in async, it will works because the order will be:
component 1 is unMount -> _reactRootContainer will be set to null -> component 2 is render -> _reactRootContainer is correctly set

I used https://codesandbox.io/s/7wvwlnorn6 to investigate.

Let me know If it needs a fix or not, or if the investigation is wrong.

[gaearon]

If you can send a fix I’d be happy to verify it.

[qhxin]

@gaearon Thank u! And, yes, you are right. i have checked the source code of unmountComponentAtNode, and i think it works relates to fiber in react.16 because my demo works right in react 15 ... maybe its a new changed feature that should be noticed to all.

[AyWa]

Little investigate more.
I think the problem come from this behavior:

  if (isRendering) {
    // Prevent reentrancy. Remaining work will be scheduled at the end of
    // the currently rendering batch.
    return;
}

at https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberScheduler.js#L2001

That is why when we are testing with timeout it is working -> because we are not in render.
So I believe somehow, when we re-schedule the job after the rendering, order might be inverted ?
For now still don't have clue about that, but will continue to investigate before making a fix :)

[AyWa]

@gaearon
I could reproduce with unit test and tried to find a correct way to fix. (I found multiple way, but not really confident about the quality of those, seems quite hacky)

PR is there https://github.com/facebook/react/pull/13744 any feedback / opinion about the proposal to make fix would be appreciated 😃

[rcalimlim]

There hasn't been any activity here for ~9 months, can we close this issue for now and reopen if this pops up again?

[ipip2005]

Is unmountComponentAtNode supposed to be asycn?
I have a legacy code that is integrated with DOM manipulation after react dom is unmounted. (I know it's not recommended, but I want to know when is unmountComponentAtNode is finished before I can execute DOM manipulation to quickly fix this legacy code)

The following order of code will have React uncaught exceptions:

1. invoke unmountComponentAtNode on DOM-A
2. clean DOM-A by javascript
3. React throw errors because when it tries to remove node, it's already being removed by step '#2'
   

[stale[bot]]

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[bot]]

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!