Skip to content

Commit

Permalink
Build a "hydration diff" highlighting the two nodes involved
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebmarkbage
sebmarkbage committed Jul 10, 2024
1 parent cd6656b commit fd61f14
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 31 deletions.
83 changes: 71 additions & 12 deletions packages/react-dom-bindings/src/client/validateDOMNesting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
* @flow
*/

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import type {HydrationDiffNode} from 'react-reconciler/src/ReactFiberHydrationDiffs';

import {enableOwnerStacks} from 'shared/ReactFeatureFlags';

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';
import {
current,
runWithFiberInDEV,
Expand All @@ -18,8 +20,42 @@ import {
HostComponent,
HostHoistable,
HostSingleton,
HostText,
} from 'react-reconciler/src/ReactWorkTags';

import {describeDiff} from 'react-reconciler/src/ReactFiberHydrationDiffs';

function describeAncestors(
ancestor: Fiber,
child: Fiber,
props: null | {children: null},
): string {
let fiber: null | Fiber = child;
let node: null | HydrationDiffNode = null;
let distanceFromLeaf = 0;
while (fiber) {
if (fiber === ancestor) {
distanceFromLeaf = 0;
}
node = {
fiber: fiber,
children: node !== null ? [node] : [],
serverProps:
fiber === child ? props : fiber === ancestor ? null : undefined,
serverTail: [],
distanceFromLeaf: distanceFromLeaf,
};
distanceFromLeaf++;
fiber = fiber.return;
}
if (node !== null) {
// Describe the node using the hydration diff logic.
// Replace + with - to mark ancestor and child. It's kind of arbitrary.
return describeDiff(node).replace(/\n\+/g, '\n-');
}
return '';
}

type Info = {tag: string};
export type AncestorInfoDev = {
current: ?Info,
Expand Down Expand Up @@ -451,7 +487,7 @@ function findInvalidAncestorForTag(

const didWarn: {[string]: boolean} = {};

function findAncestor(parent: Fiber, tagName: string): null | Fiber {
function findAncestor(parent: null | Fiber, tagName: string): null | Fiber {
while (parent) {
switch (parent.tag) {
case HostComponent:
Expand Down Expand Up @@ -496,6 +532,14 @@ function validateDOMNesting(
}
didWarn[warnKey] = true;

const child = current;
const ancestor = child ? findAncestor(child.return, ancestorTag) : null;

const ancestorDescription =
child !== null && ancestor !== null
? describeAncestors(ancestor, child, null)
: '';

const tagDisplayName = '<' + childTag + '>';
if (invalidParent) {
let info = '';
Expand All @@ -511,10 +555,11 @@ function validateDOMNesting(
// a stack trace since the stack trace format is now for owner stacks.
console.error(
'In HTML, %s cannot be a child of <%s>.%s\n' +
'This will cause a hydration error.',
'This will cause a hydration error.%s',
tagDisplayName,
ancestorTag,
info,
ancestorDescription,
);
} else {
// Don't transform into consoleWithStackDev here because we add a manual stack.
Expand All @@ -524,21 +569,21 @@ function validateDOMNesting(
// a stack trace since the stack trace format is now for owner stacks.
console.error(
'In HTML, %s cannot be a descendant of <%s>.\n' +
'This will cause a hydration error.',
'This will cause a hydration error.%s',
tagDisplayName,
ancestorTag,
ancestorDescription,
);
}
if (enableOwnerStacks && current !== null) {
if (enableOwnerStacks && child) {
// For debugging purposes find the nearest ancestor that caused the issue.
// The stack trace of this ancestor can be useful to find the cause.
// If the parent is a direct parent in the same owner, we don't bother.
const currentFiber = current;
const parent = current.return;
const ancestor = findAncestor(parent, ancestorTag);
const parent = child.return;
if (
ancestor &&
(ancestor !== parent || parent._debugOwner !== currentFiber._debugOwner)
ancestor !== null &&
parent !== null &&
(ancestor !== parent || parent._debugOwner !== child._debugOwner)
) {
runWithFiberInDEV(ancestor, () => {
console.error(
Expand Down Expand Up @@ -569,13 +614,26 @@ function validateTextNesting(childText: string, parentTag: string): boolean {
}
didWarn[warnKey] = true;

const child = current;
const ancestor = child ? findAncestor(child, parentTag) : null;

const ancestorDescription =
child !== null && ancestor !== null
? describeAncestors(
ancestor,
child,
child.tag !== HostText ? {children: null} : null,
)
: '';

if (/\S/.test(childText)) {
// TODO: Format this as a linkified "diff view" with props instead of
// a stack trace since the stack trace format is now for owner stacks.
console.error(
'In HTML, text nodes cannot be a child of <%s>.\n' +
'This will cause a hydration error.',
'This will cause a hydration error.%s',
parentTag,
ancestorDescription,
);
} else {
// TODO: Format this as a linkified "diff view" with props instead of
Expand All @@ -584,8 +642,9 @@ function validateTextNesting(childText: string, parentTag: string): boolean {
'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.\n' +
'This will cause a hydration error.',
'This will cause a hydration error.%s',
parentTag,
ancestorDescription,
);
}
return false;
Expand Down
72 changes: 61 additions & 11 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2194,9 +2194,12 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev(
'In HTML, <tr> cannot be a child of ' +
'<div>.\n' +
'This will cause a hydration error.' +
'In HTML, <tr> cannot be a child of <div>.\n' +
'This will cause a hydration error.\n' +
'\n' +
'- <div>\n' +
'- <tr>\n' +
' ...\n' +
'\n in tr (at **)' +
(gate(flags => flags.enableOwnerStacks)
? ''
Expand Down Expand Up @@ -2257,44 +2260,80 @@ describe('ReactDOMComponent', () => {
'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.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
'- <table>\n' +
' <Row>\n' +
'- <tr>\n' +
' ...\n' +
'\n in tr (at **)' +
'\n in Row (at **)',
'<table> cannot contain a nested <tr>.\nSee this log for the ancestor stack trace.' +
'\n in table (at **)' +
'\n in Foo (at **)',
'In HTML, text nodes cannot be a ' +
'child of <tr>.\n' +
'This will cause a hydration error.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
' <table>\n' +
' <Row>\n' +
' <tr>\n' +
'- x\n' +
' ...\n' +
'\n in tr (at **)' +
'\n in Row (at **)',
'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.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
'- <table>\n' +
' <Row>\n' +
'- {" "}\n' +
'\n in table (at **)' +
'\n in Foo (at **)',
]
: [
'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.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
'- <table>\n' +
' <Row>\n' +
'- <tr>\n' +
' ...\n' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
'In HTML, text nodes cannot be a ' +
'child of <tr>.\n' +
'This will cause a hydration error.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
' <table>\n' +
' <Row>\n' +
' <tr>\n' +
'- x\n' +
' ...\n' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
'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.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
'- <table>\n' +
' <Row>\n' +
'- {" "}\n' +
'\n in table (at **)' +
'\n in Foo (at **)',
],
Expand Down Expand Up @@ -2325,7 +2364,11 @@ describe('ReactDOMComponent', () => {
'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.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
' <table>\n' +
'- {" "}\n' +
'\n in table (at **)' +
'\n in Foo (at **)',
]);
Expand Down Expand Up @@ -2353,7 +2396,14 @@ describe('ReactDOMComponent', () => {
}).toErrorDev([
'In HTML, text nodes cannot be a ' +
'child of <tr>.\n' +
'This will cause a hydration error.' +
'This will cause a hydration error.\n' +
'\n' +
' <Foo>\n' +
' <table>\n' +
' <tbody>\n' +
' <Row>\n' +
' <tr>\n' +
'- text\n' +
'\n in tr (at **)' +
'\n in Row (at **)' +
(gate(flags => flags.enableOwnerStacks)
Expand Down
6 changes: 5 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMForm-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,11 @@ describe('ReactDOMForm', () => {
});
}).toErrorDev(
'In HTML, <form> cannot be a descendant of <form>.\n' +
'This will cause a hydration error.' +
'This will cause a hydration error.\n' +
'\n' +
'- <form action={function outerAction}>\n' +
' <input>\n' +
'- <form action={function innerAction} ref={{current:null}}>\n' +
'\n in form (at **)' +
(gate(flags => flags.enableOwnerStacks) ? '' : '\n in form (at **)'),
);
Expand Down
11 changes: 11 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMOption-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ describe('ReactDOMOption', () => {
}).toErrorDev(
'In HTML, <div> cannot be a child of <option>.\n' +
'This will cause a hydration error.\n' +
'\n' +
'- <option value="12">\n' +
'- <div>\n' +
' ...\n' +
'\n' +
' in div (at **)' +
(gate(flags => flags.enableOwnerStacks)
? ''
Expand Down Expand Up @@ -271,6 +276,12 @@ describe('ReactDOMOption', () => {
}).toErrorDev(
'In HTML, <div> cannot be a child of <option>.\n' +
'This will cause a hydration error.\n' +
'\n' +
' <select readOnly={true} value="bar">\n' +
'- <option value="bar">\n' +
'- <div ref={{current:null}}>\n' +
' ...\n' +
'\n' +
' in div (at **)' +
(gate(flags => flags.enableOwnerStacks)
? ''
Expand Down
10 changes: 10 additions & 0 deletions packages/react-dom/src/__tests__/validateDOMNesting-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ describe('validateDOMNesting', () => {
? [
'In HTML, <li> cannot be a descendant of <li>.\n' +
'This will cause a hydration error.\n' +
'\n' +
' <ul>\n' +
'- <li>\n' +
' <div>\n' +
'- <li>\n' +
'\n' +
' in li (at **)',
'<li> cannot contain a nested <li>.\nSee this log for the ancestor stack trace.\n' +
' in li (at **)',
Expand Down Expand Up @@ -133,6 +139,10 @@ describe('validateDOMNesting', () => {
// TODO, this should say "In SVG",
'In HTML, <body> cannot be a child of <foreignObject>.\n' +
'This will cause a hydration error.\n' +
'\n' +
'- <foreignObject>\n' +
'- <body>\n' +
'\n' +
' in body (at **)',
'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' +
' in body (at **)',
Expand Down
Loading

0 comments on commit fd61f14

Please sign in to comment.