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.

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

Ignore CR/LF differences when warning about markup mismatch #11119

Merged
merged 15 commits into from Oct 5, 2017
Merged
38 changes: 35 additions & 3 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Expand Up @@ -59,6 +59,29 @@ var HTML = '__html';

var {Namespaces: {html: HTML_NAMESPACE}, getIntrinsicNamespace} = DOMNamespaces;

var NORMALIZE_NEWLINES_REGEX = /\r\n?/g;
function isServerTextDifferentFromClientText(
serverText: string,
clientText: string,
): boolean {
if (serverText === clientText) {
return false;
}
// HTML parsing normalizes CR and CRLF to LF.
// https://www.w3.org/TR/html5/single-page.html#preprocessing-the-input-stream
// If we have a mismatch, it might be caused by that.
// We won't be patching up in this case as that matches our past behavior.
// TODO: verify that it doesn't have any observable difference.
const normalizedClientText = clientText.replace(
NORMALIZE_NEWLINES_REGEX,
'\n',
);
if (normalizedClientText === serverText) {
return false;
}
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This return value is meant to indicate that it should patch it up. So this says that we will patch up the DOM if it differs, which is not what your comment says.

}

if (__DEV__) {
var warnedUnknownTags = {
// Chrome is the only major browser not shipping <time>. But as of July
Expand Down Expand Up @@ -951,14 +974,20 @@ var ReactDOMFiberComponent = {
// TODO: Warn if there is more than a single textNode as a child.
// TODO: Should we use domElement.firstChild.nodeValue to compare?
if (typeof nextProp === 'string') {
if (domElement.textContent !== nextProp) {
const isDifferent = isServerTextDifferentFromClientText(
domElement.textContent,
nextProp,
);
if (isDifferent) {
if (__DEV__) {
warnForTextDifference(domElement.textContent, nextProp);
}
updatePayload = [CHILDREN, nextProp];
}
} else if (typeof nextProp === 'number') {
if (domElement.textContent !== '' + nextProp) {
// It's just a number so a simple check is enough
const isDifferent = domElement.textContent !== '' + nextProp;
if (isDifferent) {
if (__DEV__) {
warnForTextDifference(domElement.textContent, nextProp);
}
Expand Down Expand Up @@ -1090,7 +1119,10 @@ var ReactDOMFiberComponent = {
},

diffHydratedText(textNode: Text, text: string): boolean {
const isDifferent = textNode.nodeValue !== text;
const isDifferent = isServerTextDifferentFromClientText(
textNode.nodeValue,
text,
);
if (__DEV__) {
if (isDifferent) {
warnForTextDifference(textNode.nodeValue, text);
Expand Down
Expand Up @@ -1117,6 +1117,52 @@ describe('ReactDOMServerIntegration', () => {
expectTextNode(e.childNodes[1], 'bar');
}
});

describe('with carriage returns', () => {
// HTML parsing normalizes CR and CRLF to LF.
// https://www.w3.org/TR/html5/single-page.html#preprocessing-the-input-stream
// If we have a mismatch, it might be caused by that (and should not be reported).
// We won't be patching up in this case as that matches our past behavior.

itRenders('a div with one text CR child', async render => {
const e = await render(<div>{'foo\rbar\r\nbaz\nqux'}</div>);
if (
render === serverRender ||
render === clientRenderOnServerString ||
render === streamRender
) {
expect(e.childNodes.length).toBe(1);
// Both CR and CRLF are replaced with LF.
expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar\nbaz\nqux');
} else {
expect(e.childNodes.length).toBe(1);
// Client rendering leaves CRs as they are.
// TODO: verify that it doesn't cause any observable difference.
expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\rbar\r\nbaz\nqux');
}
});

itRenders('a div with two text CR children', async render => {
const e = await render(<div>{'foo\rbar'}{'\r\nbaz\nqux'}</div>);
if (
render === serverRender ||
render === clientRenderOnServerString ||
render === streamRender
) {
// Both CR and CRLF are replaced with LF.
// We have three nodes because there is a comment between them.
expect(e.childNodes.length).toBe(3);
expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');
expectNode(e.childNodes[2], TEXT_NODE_TYPE, '\nbaz\nqux');
} else {
expect(e.childNodes.length).toBe(2);
// Client rendering leaves CRs as they are.
// TODO: verify that it doesn't cause any observable difference.
expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\rbar');
expectNode(e.childNodes[1], TEXT_NODE_TYPE, '\r\nbaz\nqux');
}
});
});
});

describe('number children', function() {
Expand Down