Skip to content

Print attributes in hydration errors #24167

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

Closed
wants to merge 2 commits into from
+46 −18
Closed

Print attributes in hydration errors #24167

Changes from all commits
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
Original file line number Diff line number Diff line change
@@ -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 **)',
],
@@ -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 **)',
],
10 changes: 5 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Original file line number Diff line number Diff line change
@@ -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' +
@@ -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' +
4 changes: 3 additions & 1 deletion packages/react-dom/src/__tests__/ReactRenderDocument-test.js
View file
Original file line number Diff line number Diff line change
@@ -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');
});

46 changes: 36 additions & 10 deletions packages/react-dom/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
@@ -62,7 +62,7 @@ import {
shouldRemoveAttribute,
} from '../shared/DOMProperty';
import assertValidProps from '../shared/assertValidProps';
import {DOCUMENT_NODE} from '../shared/HTMLNodeType';
import {DOCUMENT_NODE, ELEMENT_NODE} from '../shared/HTMLNodeType';
import isCustomComponent from '../shared/isCustomComponent';
import possibleStandardNames from '../shared/possibleStandardNames';
import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook';
@@ -100,6 +100,7 @@ let warnForInvalidEventListener;
let canDiffStyleForHydrationWarning;

let normalizeHTML;
let formatTagDEV;

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

formatTagDEV = function(node: Element | Document | DocumentFragment): string {
let str = '<' + node.nodeName.toLowerCase();
if (node.nodeType === ELEMENT_NODE) {
const element = ((node: any): Element);
const attributes = element.attributes;
for (let i = 0; i < attributes.length; i++) {
if (i > 30) {
str += ' ...';
break;
}
const attributeName = attributes[i].name;
const value = attributes[i].value;
if (value != null) {
let trimmedValue = value;
if (value.length > 30) {
trimmedValue = value.substr(0, 30) + '...';
}
str += ' ' + attributeName + '="' + trimmedValue + '"';
}
}
}
str += '>';
return str;
};
}

// HTML parsing normalizes CR and CRLF to LF.
@@ -1209,9 +1235,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),
);
}
}
@@ -1226,9 +1252,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),
);
}
}
@@ -1244,9 +1270,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),
);
}
}
@@ -1268,9 +1294,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),
);
}
}