ReactRootContainer should not be set to null if root has been rerender

[AyWa]

Follow up to #13690

• add unit test to reproduce the issues
• implement quick/dirty fix in order to have a first iteration
• implement proper fix

Detail about the issue:

• When we call render/unmountComponentAtNode inside an other life cycle that accept side effect like ComponentDidMount, it is queue in order to prevent reentrancy from https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberScheduler.js#L1982
• if for the same DOMContainer, we have, an unmountComponentAtNode follow by a render, they will be queue correctly (as explain just before)
• when we process the queue, it will be done in correct order and so render will be correctly executed after the unmount. However because callback are executed at the end of the commitLifeCycles we will execute the unmountComponentAtNode even if a render has been executed, and so we will set the _reactRootContainer to null. Obviously there is multiple side effect, like next unmountComponentAtNode might fail or we will not execute correctly all the lifecycle etc.

Proposal of fix:

As far as I know, the life cycle seems to be fine, the only problem is because callback are call in the end, we need a way to "invalidate" the unmountComponentAtNode callback

• solution 1 2ee1335 -> just a big hack -> because we are just checking if the container has a child element -> just made for fix purpose and can be easily broken (if component return null)

• solution 2 current commit -> one way to do could be to mutate _internalRoot with a boolean like rootShouldBeUnmount(need bettername) and then if a render happen after the unmount call we set the boolean to false again -> still feel hacky

• solution 3 could be just better to remove the callback if unmount root ? but for now we don't track if the function is the unMount one or not. (we can add property isUnmountRoot etc but doesn't know if it is better)

• any better idea to experiment ?

[AyWa]

Flow will fail there, but waiting to get Proposal of fix: opinion

[sizebot]

Details of bundled changes.

Comparing: 2c7b78f...4bdf2e7

scheduler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
scheduler.development.js	n/a	n/a	0 B	19.17 KB	0 B	5.74 KB	UMD_DEV
scheduler.production.min.js	n/a	n/a	0 B	3.16 KB	0 B	1.53 KB	UMD_PROD

Generated by 🚫 dangerJS

[gaearon]

I wouldn't say this is a supported way to use React. There shouldn't be side effects in the rendering phase.

Can you come up with a repro case that is more valid? e.g. maybe you can reproduce this with a class component when calling render and unmountComponentAtNode inside componentDidMount or another lifecycle?

[AyWa]

Got it, the original issue was with componentDidMount so I updated the test with it.

Also I don't know if it should be supported or just throw a warnings? (because in the original issue, the unmountComponentAtNode was unnecessary).