paypal / react-engine

a composite render engine for universal (isomorphic) express apps to render both plain react views and react-router views
Apache License 2.0
1.45k stars 130 forks source link

Using docType: "" causes a React warning #113

Open dglozic opened 8 years ago

dglozic commented 8 years ago

We are now using the option to mount the root React component into a DIV instead of the document. When we do that, we make React render a component only into a buffer (letting Dust.js render the page outline). In order for this to work, we must specify docType: "" so that server.js does not try to inject the script tag with settings before because body does not exist. This causes the script tag to be injected right after the result of the React component output.

However, this is making React unhappy because now there is a mix of React-generated and other nodes inside the mount node, so React issues the following warning:

:warning: Warning: 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.

I am not sure what a proper fix for this could be. What we are doing now is locating the script in the rendered output and stripping it out, and placing before the closing 'body' tag in Dust.js template.

Adding @pklicnik mention - he is affected as well.

pklicnik commented 8 years ago

+1

samsel commented 8 years ago

would it help if we somehow move the script tags to the end (before body), just before we do a react render on the client side https://github.com/paypal/react-engine/blob/master/lib/client.js#L91 ?

dglozic commented 8 years ago

I think it would, and I think that is what @pklicnik is doing now after the fact. I would help if you do it on your end, since you can do it better and with more precision (knowing which script node to move).

pklicnik commented 8 years ago

Yes, that should work. Although, that does seem a bit magical - moving around nodes in the DOM after the fact isn't the ideal solution.

I'm using a slightly different approach right now to work around the problem. I can reliably identify the correct script tag because it always has id "react-engine-props". However, I'm currently stripping it out using a regex and I fear that might be fragile moving forward.

Is there a way to return it as part of the call to render() on the server side?

For example, in lib/server.js provide a way to opt-out of having the script automatically injected and instead return in as part of the callback

    // state (script) injection
    var script = format(TEMPLATE, Config.client.markupId, JSON.stringify(data));

    if (createOptions.docType === '') {
        // User is overriding the `docType`, so lets not add the script tag and instead return in via callback
    }
    else {
        html = html.replace('</body>', script + '</body>');
    }

    return done(null, html, script);

and change the implementation of done

    function done(err, html, script) {
        ...
        callback(err, html, script);
    }

Express might not like having the third parameter, but it's a different approach.