Skip to content

Improve warning in react mount #4324

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

Merged
merged 5 commits into from
Jul 10, 2015
Merged

Improve warning in react mount #4324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0249adf
Handle escaping when comparing server/client markup
Jul 9, 2015
1316160
Properly handle rendering into document when comparing markup
Jul 9, 2015
902b607
Fix some lint
Jul 9, 2015
0941cc9
Use a test case that demonstrates the fix.
Jul 9, 2015
d2e8449
Only perform expensive checks in __DEV__, clean up after inserted iframe
Jul 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions src/renderers/dom/client/ReactMount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,9 +880,29 @@ var ReactMount = {
checksum
);

var diffIndex = firstDifferenceIndex(markup, rootMarkup);
var normalizedMarkup = markup;
if (__DEV__) {
// because rootMarkup is retrieved from the DOM, various normalizations
// will have occurred which will not be present in `markup`. Here,
// insert markup into a <div> or <iframe> depending on the container
// type to perform the same normalizations before comparing.
var normalizer;
if (container.nodeType === ELEMENT_NODE_TYPE) {
normalizer = document.createElement('div');
normalizer.innerHTML = markup;
normalizedMarkup = normalizer.innerHTML;
} else {
normalizer = document.createElement('iframe');
document.body.appendChild(normalizer);
normalizer.contentDocument.write(markup);
normalizedMarkup = normalizer.contentDocument.documentElement.outerHTML;
document.body.removeChild(normalizer);
}
}

var diffIndex = firstDifferenceIndex(normalizedMarkup, rootMarkup);
var difference = ' (client) ' +
markup.substring(diffIndex - 20, diffIndex + 20) +
normalizedMarkup.substring(diffIndex - 20, diffIndex + 20) +
'\n (server) ' + rootMarkup.substring(diffIndex - 20, diffIndex + 20);

invariant(
Copy link
Contributor

Choose a reason for hiding this comment

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

Invariant shouldn't be in devmode block. @zpao should this even be an invariant? Seems like this could just be a warning?

Copy link
Author

Choose a reason for hiding this comment

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

the devmode block ends on line 901.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't in a dev block and yes it should be an invariant. The message makes it pretty clear that React probably breaks when rendering to the document element with content that doesn't checksum.

We have a dev block lower for the warning. We could perhaps have a more concise difference there.

Expand Down
18 changes: 18 additions & 0 deletions src/renderers/dom/client/__tests__/ReactMount-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,24 @@ describe('ReactMount', function() {
);
});

it('should account for escaping on a checksum mismatch', function () {
var div = document.createElement('div');
var markup = React.renderToString(
<div>This markup contains an nbsp entity: &nbsp; server text</div>);
div.innerHTML = markup;

spyOn(console, 'error');
React.render(
<div>This markup contains an nbsp entity: &nbsp; client text</div>,
div
);
expect(console.error.calls.length).toBe(1);
expect(console.error.calls[0].args[0]).toContain(
' (client) nbsp entity: &nbsp; client text</div>\n' +
' (server) nbsp entity: &nbsp; server text</div>'
);
});

if (WebComponents !== undefined) {
it('should allow mounting/unmounting to document fragment container',
function() {
Expand Down