React.renderToStaticMarkup should not add <noscript> tags

[ByScripts]

I think the placeholders should not be inserted when rendering with renderToStaticMarkup:

var React = require('react');

var Demo = React.createClass({
    propTypes: {
        render: React.PropTypes.bool.isRequired
    },
    render: function() {
        return this.props.render && <div>Im a demo</div>;
    }
});

var Container = React.createClass({
    propTypes: {
        render: React.PropTypes.bool.isRequired
    },
    render: function() {
        return (
            <div>
                <Demo render={this.props.render}/>
            </div>
        );
    }
});

console.log(React.renderToStaticMarkup(<Container render={true}/>));
// <div><div>Im a demo</div></div>

console.log(React.renderToStaticMarkup(<Container render={false}/>));
// <div><noscript></noscript></div>

For example, in a more complex structure, it becomes hard to use CSS selectors like :first-child, :nth-child() etc.

[zpao]

We render <noscript> elements in the DOM too, so you'll have issues with those pseudo selectors client-side too. This is how we make it possible for render to return false and null. Right now we need each component to map to a node. While this restriction is in place, we'll continue to insert <noscript> elements. There isn't really a fix in sight for that and there are couple other issues so I'm going to close out.

[zpao]

A little bit more clarification since I didn't initially notice this was specifically for renderToStaticMarkup - we're trying to keep that code path and output the same as renderToString as much as possible. This makes it so that somebody could switch and mount on the client without having to revisit they're stylesheets and other assumptions like DOM structure.

[ByScripts]

Ok I understand. Thank you for the quick reply.

In fact, I have a webpage generator on the backend side which does not use the same CSS than the frontend side (which displays the rendered HTML stored in a DB).

But I worked around by adding a different class in the root element.

Thanks again.

[quantizor]

@zpao Could we use a comment node instead of noscript?

[zpao]

IIRC there are issues with that though I don't know off the top of my head. Perhaps @chenglou does?

[chenglou]

Eh, I think comment would accidentally disappear in older browsers? @syranide knew

[quantizor]

If that behavior doesn't exist in IE9, it might serve as a useful alternative.

[jimfb]

We still support IE8, but yeah.

[sophiebits]

I don't recall us considering comment nodes (which sounds like a good idea!), though I would also believe @syranide if he says we did.

[syranide]

@chenglou IIRC the IE8 whitespace bug affects comments too (making them disappear), but that should not be an issue for us. Again IIRC, IE8-9 also has a bug where adjacent comments are merged into a single comment node (not sure under which conditions). All of these affect(ed) knockoutjs so perhaps there's more info to be found there. Other than that... comments (like text nodes) don't appear in children, but I don't think that's an obstacle for us.

@spicyj I think it was mentioned but never discussed, you had a PR for using comments instead of spans for text nodes though.

[quantizor]

@syranide I'm having trouble finding any information about a comment coalescence bug in IE 8/9. I did find a DOM method called node.normalize() which could explain adjacent text nodes being merged if it was being used under the hood, but nothing specifically about comment nodes.

[syranide]

@yaycmyk This is a long time ago, memory is really fuzzy, but I remember knockoutjs being affected by some comment quirks.

[sophiebits]

knockout/knockout@6648861 is the closest I can find.