Skip to content

Commit

Permalink
Validate DOM nesting for hydration before the hydration warns / errors (
Browse files Browse the repository at this point in the history
#28434)

If there's invalid dom nesting, there will be mismatches following but
the nesting is the most important cause of the problem.

Previously we would include the DOM nesting when rerendering thanks to
the new model of throw and recovery. However, the log would come during
the recovery phase which is after we've already logged that there was a
hydration mismatch.

People would consistently miss this log. Which is fair because you
should always look at the first log first as the most probable cause.

This ensures that we log in the hydration phase if there's a dom nesting
issue. This assumes that the consequence of nesting will appear such
that the won't have a mismatch before this. That's typically the case
because the node will move up and to be a later sibling. So as long as
that happens and we keep hydrating depth first, it should hold true.
There might be an issue if there's a suspense boundary between the nodes
we'll find discover the new child in the outer path since suspense
boundaries as breadth first.

Before:

<img width="996" alt="Screenshot 2024-02-23 at 7 34 01 PM"
src="https://github.com/facebook/react/assets/63648/af70cf7f-898b-477f-be39-13b01cfe585f">

After:

<img width="853" alt="Screenshot 2024-02-23 at 7 22 24 PM"
src="https://github.com/facebook/react/assets/63648/896c6348-1620-4f99-881d-b6069263925e">

Cameo: RSC stacks.
  • Loading branch information
sebmarkbage authored Feb 24, 2024
1 parent 16d3f78 commit 118ad2a
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 59 deletions.
27 changes: 27 additions & 0 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,19 @@ export function getFirstHydratableChildWithinSuspenseInstance(
return getNextHydratable(parentInstance.nextSibling);
}

export function validateHydratableInstance(
type: string,
props: Props,
hostContext: HostContext,
): boolean {
if (__DEV__) {
// TODO: take namespace into account when validating.
const hostContextDev: HostContextDev = (hostContext: any);
return validateDOMNesting(type, hostContextDev.ancestorInfo);
}
return true;
}

export function hydrateInstance(
instance: Instance,
type: string,
Expand Down Expand Up @@ -1383,6 +1396,20 @@ export function hydrateInstance(
);
}

export function validateHydratableTextInstance(
text: string,
hostContext: HostContext,
): boolean {
if (__DEV__) {
const hostContextDev = ((hostContext: any): HostContextDev);
const ancestor = hostContextDev.ancestorInfo.current;
if (ancestor != null) {
return validateTextNesting(text, ancestor.tag);
}
}
return true;
}

export function hydrateTextInstance(
textInstance: TextInstance,
text: string,
Expand Down
33 changes: 22 additions & 11 deletions packages/react-dom-bindings/src/client/validateDOMNesting.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ const didWarn: {[string]: boolean} = {};
function validateDOMNesting(
childTag: string,
ancestorInfo: AncestorInfoDev,
): void {
): boolean {
if (__DEV__) {
ancestorInfo = ancestorInfo || emptyAncestorInfoDev;
const parentInfo = ancestorInfo.current;
Expand All @@ -455,7 +455,7 @@ function validateDOMNesting(
: findInvalidAncestorForTag(childTag, ancestorInfo);
const invalidParentOrAncestor = invalidParent || invalidAncestor;
if (!invalidParentOrAncestor) {
return;
return true;
}

const ancestorTag = invalidParentOrAncestor.tag;
Expand All @@ -464,7 +464,7 @@ function validateDOMNesting(
// eslint-disable-next-line react-internal/safe-string-coercion
String(!!invalidParent) + '|' + childTag + '|' + ancestorTag;
if (didWarn[warnKey]) {
return;
return false;
}
didWarn[warnKey] = true;

Expand All @@ -477,45 +477,56 @@ function validateDOMNesting(
'the browser.';
}
console.error(
'%s cannot appear as a child of <%s>.%s',
'In HTML, %s cannot be a child of <%s>.%s\n' +
'This will cause a hydration error.',
tagDisplayName,
ancestorTag,
info,
);
} else {
console.error(
'%s cannot appear as a descendant of ' + '<%s>.',
'In HTML, %s cannot be a descendant of <%s>.\n' +
'This will cause a hydration error.',
tagDisplayName,
ancestorTag,
);
}
return false;
}
return true;
}

function validateTextNesting(childText: string, parentTag: string): void {
function validateTextNesting(childText: string, parentTag: string): boolean {
if (__DEV__) {
if (isTagValidWithParent('#text', parentTag)) {
return;
return true;
}

// eslint-disable-next-line react-internal/safe-string-coercion
const warnKey = '#text|' + parentTag;
if (didWarn[warnKey]) {
return;
return false;
}
didWarn[warnKey] = true;

if (/\S/.test(childText)) {
console.error('Text nodes cannot appear as a child of <%s>.', parentTag);
console.error(
'In HTML, text nodes cannot be a child of <%s>.\n' +
'This will cause a hydration error.',
parentTag,
);
} else {
console.error(
'Whitespace text nodes cannot appear as a child of <%s>. ' +
'In HTML, whitespace text nodes cannot be a child of <%s>. ' +
"Make sure you don't have any extra whitespace between tags on " +
'each line of your source code.',
'each line of your source code.\n' +
'This will cause a hydration error.',
parentTag,
);
}
return false;
}
return true;
}

export {updatedAncestorInfoDev, validateDOMNesting, validateTextNesting};
39 changes: 23 additions & 16 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2188,8 +2188,9 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev([
'Warning: <tr> cannot appear as a child of ' +
'<div>.' +
'Warning: In HTML, <tr> cannot be a child of ' +
'<div>.\n' +
'This will cause a hydration error.' +
'\n in tr (at **)' +
'\n in div (at **)',
]);
Expand All @@ -2208,8 +2209,9 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev(
'Warning: <p> cannot appear as a descendant ' +
'of <p>.' +
'Warning: In HTML, <p> cannot be a descendant ' +
'of <p>.\n' +
'This will cause a hydration error.' +
// There is no outer `p` here because root container is not part of the stack.
'\n in p (at **)' +
'\n in span (at **)',
Expand Down Expand Up @@ -2241,22 +2243,25 @@ describe('ReactDOMComponent', () => {
root.render(<Foo />);
});
}).toErrorDev([
'Warning: <tr> cannot appear as a child of ' +
'Warning: In HTML, <tr> cannot be a child of ' +
'<table>. Add a <tbody>, <thead> or <tfoot> to your code to match the DOM tree generated ' +
'by the browser.' +
'by the browser.\n' +
'This will cause a hydration error.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
'Warning: Text nodes cannot appear as a ' +
'child of <tr>.' +
'Warning: In HTML, text nodes cannot be a ' +
'child of <tr>.\n' +
'This will cause a hydration error.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
'Warning: Whitespace text nodes cannot ' +
"appear as a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.' +
'Warning: In HTML, whitespace text nodes cannot ' +
"be a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.\n' +
'This will cause a hydration error.' +
'\n in table (at **)' +
'\n in Foo (at **)',
]);
Expand All @@ -2283,9 +2288,10 @@ describe('ReactDOMComponent', () => {
root.render(<Foo> </Foo>);
});
}).toErrorDev([
'Warning: Whitespace text nodes cannot ' +
"appear as a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.' +
'Warning: In HTML, whitespace text nodes cannot ' +
"be a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.\n' +
'This will cause a hydration error.' +
'\n in table (at **)' +
'\n in Foo (at **)',
]);
Expand All @@ -2311,8 +2317,9 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev([
'Warning: Text nodes cannot appear as a ' +
'child of <tr>.' +
'Warning: In HTML, text nodes cannot be a ' +
'child of <tr>.\n' +
'This will cause a hydration error.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in tbody (at **)' +
Expand Down
20 changes: 10 additions & 10 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ describe('ReactDOMFloat', () => {
}).toErrorDev(
[
'Cannot render <noscript> outside the main document. Try moving it into the root <head> tag.',
'Warning: <noscript> cannot appear as a child of <#document>.',
'Warning: In HTML, <noscript> cannot be a child of <#document>.',
],
{withoutStack: 1},
);
Expand All @@ -538,7 +538,7 @@ describe('ReactDOMFloat', () => {
await waitForAll([]);
}).toErrorDev([
'Cannot render <template> outside the main document. Try moving it into the root <head> tag.',
'Warning: <template> cannot appear as a child of <html>.',
'Warning: In HTML, <template> cannot be a child of <html>.',
]);

await expect(async () => {
Expand All @@ -551,7 +551,7 @@ describe('ReactDOMFloat', () => {
await waitForAll([]);
}).toErrorDev([
'Cannot render a <style> outside the main document without knowing its precedence and a unique href key. React can hoist and deduplicate <style> tags if you provide a `precedence` prop along with an `href` prop that does not conflic with the `href` values used in any other hoisted <style> or <link rel="stylesheet" ...> tags. Note that hoisting <style> tags is considered an advanced feature that most will not use directly. Consider moving the <style> tag to the <head> or consider adding a `precedence="default"` and `href="some unique resource identifier"`, or move the <style> to the <style> tag.',
'Warning: <style> cannot appear as a child of <html>.',
'Warning: In HTML, <style> cannot be a child of <html>.',
]);

await expect(async () => {
Expand All @@ -574,7 +574,7 @@ describe('ReactDOMFloat', () => {
}).toErrorDev(
[
'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence. Consider adding precedence="default" or moving it into the root <head> tag.',
'Warning: <link> cannot appear as a child of <#document>.',
'Warning: In HTML, <link> cannot be a child of <#document>.',
],
{withoutStack: 1},
);
Expand All @@ -591,7 +591,7 @@ describe('ReactDOMFloat', () => {
await waitForAll([]);
}).toErrorDev([
'Cannot render a sync or defer <script> outside the main document without knowing its order. Try adding async="" or moving it into the root <head> tag.',
'Warning: <script> cannot appear as a child of <html>.',
'Warning: In HTML, <script> cannot be a child of <html>.',
]);

await expect(async () => {
Expand Down Expand Up @@ -2552,11 +2552,11 @@ body {
'Cannot render a <style> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <style> remove the `itemProp` prop. Otherwise, try moving this tag into the <head> or <body> of the Document.',
'Cannot render a <link> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <link> remove the `itemProp` prop. Otherwise, try moving this tag into the <head> or <body> of the Document.',
'Cannot render a <script> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <script> remove the `itemProp` prop. Otherwise, try moving this tag into the <head> or <body> of the Document.',
'<meta> cannot appear as a child of <html>',
'<title> cannot appear as a child of <html>',
'<style> cannot appear as a child of <html>',
'<link> cannot appear as a child of <html>',
'<script> cannot appear as a child of <html>',
'In HTML, <meta> cannot be a child of <html>',
'In HTML, <title> cannot be a child of <html>',
'In HTML, <style> cannot be a child of <html>',
'In HTML, <link> cannot be a child of <html>',
'In HTML, <script> cannot be a child of <html>',
]);
});

Expand Down
3 changes: 2 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ describe('ReactDOMForm', () => {
);
});
}).toErrorDev([
'Warning: <form> cannot appear as a descendant of <form>.' +
'Warning: In HTML, <form> cannot be a descendant of <form>.\n' +
'This will cause a hydration error.' +
'\n in form (at **)' +
'\n in form (at **)',
]);
Expand Down
5 changes: 3 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMOption-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ describe('ReactDOMOption', () => {
expect(() => {
node = ReactTestUtils.renderIntoDocument(el);
}).toErrorDev(
'<div> cannot appear as a child of <option>.\n' +
'In HTML, <div> cannot be a child of <option>.\n' +
'This will cause a hydration error.\n' +
' in div (at **)\n' +
' in option (at **)',
);
Expand Down Expand Up @@ -263,7 +264,7 @@ describe('ReactDOMOption', () => {
[
'Warning: Text content did not match. Server: "FooBaz" Client: "Foo"',
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>',
'Warning: <div> cannot appear as a child of <option>',
'Warning: In HTML, <div> cannot be a child of <option>',
],
{withoutStack: 1},
);
Expand Down
37 changes: 29 additions & 8 deletions packages/react-dom/src/__tests__/validateDOMNesting-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,31 +60,42 @@ describe('validateDOMNesting', () => {
it('prevents problematic nestings', () => {
expectWarnings(
['a', 'a'],
['<a> cannot appear as a descendant of <a>.\n' + ' in a (at **)'],
[
'In HTML, <a> cannot be a descendant of <a>.\n' +
'This will cause a hydration error.\n' +
' in a (at **)',
],
);
expectWarnings(
['form', 'form'],
[
'<form> cannot appear as a descendant of <form>.\n' +
'In HTML, <form> cannot be a descendant of <form>.\n' +
'This will cause a hydration error.\n' +
' in form (at **)',
],
);
expectWarnings(
['p', 'p'],
['<p> cannot appear as a descendant of <p>.\n' + ' in p (at **)'],
[
'In HTML, <p> cannot be a descendant of <p>.\n' +
'This will cause a hydration error.\n' +
' in p (at **)',
],
);
expectWarnings(
['table', 'tr'],
[
'<tr> cannot appear as a child of <table>. ' +
'In HTML, <tr> cannot be a child of <table>. ' +
'Add a <tbody>, <thead> or <tfoot> to your code to match the DOM tree generated by the browser.\n' +
'This will cause a hydration error.\n' +
' in tr (at **)',
],
);
expectWarnings(
['div', 'ul', 'li', 'div', 'li'],
[
'<li> cannot appear as a descendant of <li>.\n' +
'In HTML, <li> cannot be a descendant of <li>.\n' +
'This will cause a hydration error.\n' +
' in li (at **)\n' +
' in div (at **)\n' +
' in li (at **)\n' +
Expand All @@ -93,16 +104,26 @@ describe('validateDOMNesting', () => {
);
expectWarnings(
['div', 'html'],
['<html> cannot appear as a child of <div>.\n' + ' in html (at **)'],
[
'In HTML, <html> cannot be a child of <div>.\n' +
'This will cause a hydration error.\n' +
' in html (at **)',
],
);
expectWarnings(
['body', 'body'],
['<body> cannot appear as a child of <body>.\n' + ' in body (at **)'],
[
'In HTML, <body> cannot be a child of <body>.\n' +
'This will cause a hydration error.\n' +
' in body (at **)',
],
);
expectWarnings(
['svg', 'foreignObject', 'body', 'p'],
[
'<body> cannot appear as a child of <foreignObject>.\n' +
// TODO, this should say "In SVG",
'In HTML, <body> cannot be a child of <foreignObject>.\n' +
'This will cause a hydration error.\n' +
' in body (at **)\n' +
' in foreignObject (at **)',
'Warning: You are mounting a new body component when a previous one has not first unmounted. It is an error to render more than one body component at a time and attributes and children of these components will likely fail in unpredictable ways. Please only render a single instance of <body> and if you need to mount a new one, ensure any previous ones have unmounted first.\n' +
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1512,12 +1512,12 @@ function updateHostComponent(
workInProgress: Fiber,
renderLanes: Lanes,
) {
pushHostContext(workInProgress);

if (current === null) {
tryToClaimNextHydratableInstance(workInProgress);
}

pushHostContext(workInProgress);

const type = workInProgress.type;
const nextProps = workInProgress.pendingProps;
const prevProps = current !== null ? current.memoizedProps : null;
Expand Down
Loading

0 comments on commit 118ad2a

Please sign in to comment.