Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

React 16 fails to rehydrate noscripts #10993

Closed
wchargin opened this issue Sep 30, 2017 · 8 comments · Fixed by #11157
Closed

React 16 fails to rehydrate noscripts #10993

wchargin opened this issue Sep 30, 2017 · 8 comments · Fixed by #11157
Labels
Component: Server Rendering Type: Bug

Comments

@wchargin
Copy link

Do you want to request a feature or report a bug?
Bug.

What is the current behavior?
As of React 16, <noscript dangerouslySetInnerHTML={...} />s are not properly rehydrated. Equivalent code works in React 15.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/ebsrpraL/).
Consider the following server-side code:

const React = require('react');
const ReactDOMServer = require('react-dom/server');

class Main extends React.Component {

    render() {
        const component = React.createElement('strong', {}, 'Hello!');
        const contents = ReactDOMServer.renderToStaticMarkup(component);
        console.log("--- Contents: " + contents);
        return React.createElement('noscript', {
            dangerouslySetInnerHTML: {__html: contents},
        }, null);
    }

}

function createApp() {
    return React.createElement(Main, {}, null);
}

console.log(ReactDOMServer.renderToString(createApp()));

This code works fine in Reacts 15 and 16 alike; the outputs are similar:

// React 15
--- Contents: <strong>Hello!</strong>
<noscript data-reactroot="" data-reactid="1" data-react-checksum="1795300394"><strong>Hello!</strong></noscript>

// React 16
--- Contents: <strong>Hello!</strong>
<noscript data-reactroot=""><strong>Hello!</strong></noscript>

Now, consider the following application (jsbin):

<!DOCTYPE html>
<html>
<body>

<!-- React 16 (exhibits bug) -->
<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>

<!-- React 15 (does not exhibit bug) -->
<!--
<script crossorigin src="https://cdnjs.cloudflare.com/ajax/libs/react/15.6.1/react.js"></script>
<script crossorigin src="https://cdnjs.cloudflare.com/ajax/libs/react/15.6.1/react-dom.js"></script>
-->


<div id="target"><noscript data-reactroot=""><strong>Hello!</strong></noscript></div>

<script>
class Main extends React.Component {

    render() {
        const contents = '<strong>Hello!</strong>';
        console.log("--- Contents: " + contents);
        return React.createElement('noscript', {
            dangerouslySetInnerHTML: {__html: contents},
        }, null);
    }

}

function createApp() {
    return React.createElement(Main, {}, null);
}

(ReactDOM.hydrate || ReactDOM.render)(
    createApp(), document.getElementById('target'));

console.log("Initialized.");
</script>

</body>
</html>

In React 16, this displays the following in the console:

[log] --- Contents: <strong>Hello!</strong>
[err] Warning: Prop `dangerouslySetInnerHTML` did not match. Server: "&lt;strong&gt;Hello!&lt;/strong&gt;" Client: "<strong>Hello!</strong>"
[log] Initialized.

If you swap the comment blocks in the HTML so that React 15 is used, the code works just fine: there are no warnings. (This is true regardless of whether you change the contents of div#target from the React 16 output to the React 15 output.)

The error message confuses me; it states that the server output is

&lt;strong&gt;Hello!&lt;/strong&gt;

but this is not consistent with the output of the server-side code (neither the "Contents" nor the final markup emitted has HTML entities).

Please note that this is important because it breaks the only known workaround to #1252.

I believe that this may be specific to noscripts, because if one replaces all occurrences of noscript with div then the example works fine in React 16.

What is the expected behavior?
Rendering a component with renderToString and then rehydrating that component verbatim should not yield an error.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
Regression from 15 to 16. Tested in Chrome v61.
@Merri Merri mentioned this issue Oct 2, 2017
Closed
@Merri
Copy link

Merri commented Oct 2, 2017 •
edited
Loading

As an additional detail I noticed some of the attributes are rendered using the DOM property name instead of HTML attribute name.

ReactDOMServer.renderToStaticMarkup(
    React.createElement(
        'noscript',
        null,
        React.createElement('img', { className: 'className', srcSet: 'srcSet' })
    )
)

React 15: <noscript><img class="className" srcset="srcSet"/></noscript>
React 16: <noscript><img class="className" srcSet="srcSet"/></noscript>

This is incorrect both client and server side in React 16.0.0

Reported here as I think this seems very related to this particular bug.

@gaearon
Copy link
Collaborator

gaearon commented Oct 2, 2017

I don't think it is technically incorrect because HTML is case-insensitive. Although it doesn't hurt to lowercase them and we can do that.

@Merri
Copy link

Merri commented Oct 2, 2017

Ah yes, I had already forgotten that. Used XHTML for so many years that it stuck :)

Then the only reason to do that would be for full backwards compatibility with 15, unless XHTML compatibility is considered a thing in 2017.

@wchargin
Copy link
Author

wchargin commented Oct 2, 2017

Just a note: the usual caveats about downcasing with SVG apply (SVG has case-sensitive attributes). For instance, <svg viewBox="0 0 1 1" /> needs to use the attribute name viewBox and not viewbox.

From recent PRs (#11033), it looks like similar issues are on @gaearon's mind; posting here just for posterity, as naively "downcasing everything" is not sound.

@Merri Merri mentioned this issue Oct 3, 2017
Closed
@gaearon
Copy link
Collaborator

gaearon commented Oct 3, 2017

Regarding case sensitivity, there is an existing issue: #10863.

@gaearon gaearon mentioned this issue Oct 3, 2017
Closed
19 tasks
@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

OK, what seems to be happening is that assigning innerHTML to a <noscript> attribute isn’t actually enough because the content is parsed without knowing it’s “inside of” <noscript>. Maybe we should always wrap into an extra tag when normalizing.

@gaearon gaearon mentioned this issue Oct 9, 2017
Closed
@gaearon gaearon mentioned this issue Oct 9, 2017
Merged
@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

I have a fix in #11157.

@wchargin
Copy link
Author

Thanks very much @gaearon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: Server Rendering Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix false positive <noscript> rehydration text difference warning in React 16 gaearon/react
3 participants
@gaearon @Merri @wchargin