Ignore whitespace in the container element when reusing markup #996
Description
At the moment, if there's any whitespace around a pre-rendered component inside it's container, the HTML won't be reused. This is because getReactRootElementInContainer() uses Node.firstChild which will return a TEXT_NODE instead of an ELEMENT_NODE in this case.
For example, this HTML won't be reused:
<div class="wrapper">
<div class="component" data-reactid=".r[1wtmm]" data-react-checksum="490228448">...</div>
</div>If you want, I can send through a pull request that changes getReactRootElementInContainer() to look for the first ELEMENT_NODE instead of just any Node.
Activity
sophiebits commentedon Jan 30, 2014
My inclination would be to ignore only text nodes comprised solely of whitespace.
Rowno commentedon Jan 30, 2014
True, that will work too.
syranide commentedon Jan 30, 2014
This is fixed in master (my whitespace PR took care of spaces appearing literally everywhere).
Rowno commentedon Jan 30, 2014
Sweet, looking forward to the next release then. 😀
sophiebits commentedon Jan 30, 2014
@syranide I thought your PR only modified jsx -- this is about adopting server-rendered markup.
syranide commentedon Jan 30, 2014
@spicyj Yes it's only JSX. It seemed to me like white-space (arbitrarily) introduced by JSX was the issue. But perhaps I'm mistaken? Proof-reading it again, it may be the case actually (not 100% sure what @Rowno means :)).
Rowno commentedon Jan 30, 2014
Maybe this is clearer.
This markup won't be reused:
But this markup will:
The
.componentelement is the output generated byReact.renderComponentToString(). I'm not using React to render the entire page, only the dynamic parts. So this problem occurs when combining the React output with other server-side templates.syranide commentedon Jan 30, 2014
@Rowno Aha, yeah my mistake then. Not fixed in master.
sophiebits commentedon Feb 13, 2014
(petehunt says: "We just need to change our use of
firstChildin src/browser/getReactRootElementInContainer.js to bechildren[0].")JannesMeyer commentedon Jun 18, 2014
I experimented with a possible fix a little bit. As @spicyj said in #1050 it might be worth it to not ignore prepended non-whitespace text.
This gist would be the necessary code if you want to make sure you only ignore text nodes that consist purely of whitespace. But this would get rather more complex if you wanted to make sure you account for the edge case of alternating comment nodes and text nodes until you actually find an element node or a text node with non-whitespace content.
Therefore I think it would be much easier to implement @petehunt's proposal and just use
container.children[0].In my opinion it makes no sense to do this non-whitespace check, because React doesn't actually make sure there is no appended non-whitespace text neither when re-using DOM nodes that came from the server.
aweary commentedon Jul 12, 2017
Closing this out since #1912 implemented a warning that would cover this situation (exampe)