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

Print attributes in hydration errors #24167

Closed
wants to merge 2 commits into from
Closed

Print attributes in hydration errors #24167

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b285b71
Print attributes in hydration errors
gaearon Mar 25, 2022
c5232b2
Tighten up the types
gaearon Mar 25, 2022
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
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,7 @@ describe('ReactDOMFizzServer', () => {
}).toErrorDev(
[
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
'Warning: Expected server HTML to contain a matching <div> in <div>.\n' +
'Warning: Expected server HTML to contain a matching <div> in <div id="container">.\n' +
' in div (at **)\n' +
' in App (at **)',
],
Expand Down Expand Up @@ -1947,7 +1947,7 @@ describe('ReactDOMFizzServer', () => {
}).toErrorDev(
[
'Warning: An error occurred during hydration. The server HTML was replaced with client content',
'Warning: Expected server HTML to contain a matching <div> in <div>.\n' +
'Warning: Expected server HTML to contain a matching <div> in <div id="container">.\n' +
' in div (at **)\n' +
' in App (at **)',
],
Expand Down
10 changes: 5 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ describe('ReactDOMServerPartialHydration', () => {

if (__DEV__) {
expect(mockError.mock.calls[0]).toEqual([
'Warning: Expected server HTML to contain a matching <%s> in <%s>.%s',
'div',
'Warning: Expected server HTML to contain a matching <%s> in %s.%s',
'div',
'<div>',
'\n' +
' in div (at **)\n' +
' in Component (at **)\n' +
Expand Down Expand Up @@ -610,9 +610,9 @@ describe('ReactDOMServerPartialHydration', () => {
}
if (__DEV__) {
expect(mockError).toHaveBeenCalledWith(
'Warning: Did not expect server HTML to contain a <%s> in <%s>.%s',
'span',
'div',
'Warning: Did not expect server HTML to contain a %s in %s.%s',
'<span>',
'<div>',
'\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
Expand Down
4 changes: 3 additions & 1 deletion packages/react-dom/src/__tests__/ReactRenderDocument-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,9 @@ describe('rendering React components at document', () => {
// getTestDocument() has an extra <meta> that we didn't render.
expect(() =>
ReactDOM.hydrate(<Component text="Hello world" />, testDocument),
).toErrorDev('Did not expect server HTML to contain a <meta> in <head>.');
).toErrorDev(
'Did not expect server HTML to contain a <meta charset="utf-8"> in <head>.',
);
expect(testDocument.body.innerHTML).toBe('Hello world');
});

Expand Down
39 changes: 30 additions & 9 deletions packages/react-dom/src/client/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ let warnForInvalidEventListener;
let canDiffStyleForHydrationWarning;

let normalizeHTML;
let formatTagDEV;

if (__DEV__) {
warnedUnknownTags = {
Expand Down Expand Up @@ -206,6 +207,26 @@ if (__DEV__) {
testElement.innerHTML = html;
return testElement.innerHTML;
};

formatTagDEV = function(node: Element): string {
let str = '<' + node.nodeName.toLowerCase();
const attributeNames = node.getAttributeNames();
for (let i = 0; i < attributeNames.length; i++) {
if (i > 30) {
str += ' ...';
break;
}
const attributeName = attributeNames[i];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could get a React name for this but seems confusing. E.g. it would still print style as a string. So I figured it should just use HTML in the message.

const value = node.getAttribute(attributeName);
let trimmedValue = value;
if (value.length > 30) {
trimmedValue = value.substr(0, 30) + '...';
}
str += ' ' + attributeName + '="' + trimmedValue + '"';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This doesn't escape trimmedValue but I think that's fine. If we wanted to escape we'd need to move the escaping utility from the server folder or copypaste here.

}
str += '>';
return str;
};
}

// HTML parsing normalizes CR and CRLF to LF.
Expand Down Expand Up @@ -1209,9 +1230,9 @@ export function warnForDeletedHydratableElement(
}
didWarnInvalidHydration = true;
console.error(
'Did not expect server HTML to contain a <%s> in <%s>.',
child.nodeName.toLowerCase(),
parentNode.nodeName.toLowerCase(),
'Did not expect server HTML to contain a %s in %s.',
formatTagDEV(child),
formatTagDEV(parentNode),
);
}
}
Expand All @@ -1226,9 +1247,9 @@ export function warnForDeletedHydratableText(
}
didWarnInvalidHydration = true;
console.error(
'Did not expect server HTML to contain the text node "%s" in <%s>.',
'Did not expect server HTML to contain the text node "%s" in %s.',
child.nodeValue,
parentNode.nodeName.toLowerCase(),
formatTagDEV(parentNode),
);
}
}
Expand All @@ -1244,9 +1265,9 @@ export function warnForInsertedHydratedElement(
}
didWarnInvalidHydration = true;
console.error(
'Expected server HTML to contain a matching <%s> in <%s>.',
'Expected server HTML to contain a matching <%s> in %s.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it clear what the "in" really means here? That's the thing that I always found vague. Like are these the mismatches, or is one parent and one child?

I feel like the message you use some formatting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I'd propose changing it more significantly. Something like

There is a mismatch between the client and server render output.
The mismatch occurred inside of this parent element:

  <div class="blabla parent">

The last successfully matched child was:

  <h1 class="blabla">

After it, the server rendered:

  <h3 class="blabla">

But the client rendered:

  <h2 class="blabla">

Fix the error by making the client and the server output the same.

Thoughts?

Copy link
Collaborator

@sebmarkbage sebmarkbage Mar 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This has the same data but is easier to parse for me at least. More visual:

There is a mismatch between the client and server render output.
The mismatch occurred inside of this element:

  <div class="blabla parent">
    <h1 class="blabla">...</h1>
-   <h3 class="blabla">  <--- server
+   <h2 class="blabla">  <--- client
    ...
  </div>

Copy link
Collaborator

Choose a reason for hiding this comment

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

The same format translates to missing and extra nodes too.

tag,
parentNode.nodeName.toLowerCase(),
formatTagDEV(parentNode),
);
}
}
Expand All @@ -1268,9 +1289,9 @@ export function warnForInsertedHydratedText(
}
didWarnInvalidHydration = true;
console.error(
'Expected server HTML to contain a matching text node for "%s" in <%s>.',
'Expected server HTML to contain a matching text node for "%s" in %s.',
text,
parentNode.nodeName.toLowerCase(),
formatTagDEV(parentNode),
);
}
}
Expand Down