Fix mount and unmount warnings in Fiber#10056
Conversation
sebmarkbage
requested review from
acdlite,
bvaughn,
flarnie,
gaearon and
sophiebits
|
Flow failing. |
sebmarkbage
force-pushed
the
fixmountwarnings
branch
2 times, most recently
from
c765469 to
37a0139
Compare
|
Fixed. The missing brand check for nodeType is annoying. |
| false, | ||
| 'render(): Target node has markup rendered by React, but there ' + | ||
| 'are unrelated nodes as well. This is most commonly caused by ' + | ||
| 'white-space inserted around server-rendered markup.', |
There was a problem hiding this comment.
Nit: whitespace is usually written as a single word?
There was a problem hiding this comment.
I guess it's just an old message.
| let warned = false; | ||
| while (container.lastChild) { | ||
| container.removeChild(container.lastChild); | ||
| let rootSibling = container.lastChild; |
There was a problem hiding this comment.
Nit: we read it twice here, can we avoid that?
Supernit: I see two spaces before =
There was a problem hiding this comment.
Using the variable to avoid reading it twice yields slightly larger code in prod. :)
Fixed the nit. Forgot prettier.
There was a problem hiding this comment.
Oh wait. I figured out another way to write it.
sebmarkbage
force-pushed
the
fixmountwarnings
branch
from
37a0139 to
bac9e3c
Compare
This triggers our non-reuse mode. This is covered by ReactMount already but the test doesn't pass yet without also landing facebook#10026.
sebmarkbage
force-pushed
the
fixmountwarnings
branch
from
bac9e3c to
bbd97bd
Compare
| // Check if the container itself is a React root node. | ||
| const isContainerReactRoot = | ||
| container.nodeType === 1 && | ||
| isValidContainer(container.parentNode) && |
There was a problem hiding this comment.
@spicyj Should we check for a previous sibling that is a container too? To support this warning properly for comment mount points.
4 participants
Warn if unmounting a non-container. Warn if mounting into a container where the first child isn't the "root".