From 0364831c5d0992a58544b48e062fb17f11dc3631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sat, 24 Feb 2024 00:45:42 -0500 Subject: [PATCH] Validate DOM nesting for hydration before the hydration warns / errors (#28434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Screenshot 2024-02-23 at 7 34 01 PM After: Screenshot 2024-02-23 at 7 22 24 PM Cameo: RSC stacks. --- .../src/client/ReactFiberConfigDOM.js | 27 ++++++++ .../src/client/validateDOMNesting.js | 33 ++++++--- .../src/__tests__/ReactDOMComponent-test.js | 39 ++++++----- .../src/__tests__/ReactDOMFloat-test.js | 20 +++--- .../src/__tests__/ReactDOMForm-test.js | 3 +- .../src/__tests__/ReactDOMOption-test.js | 5 +- .../src/__tests__/validateDOMNesting-test.js | 37 +++++++--- .../src/ReactFiberBeginWork.js | 4 +- .../src/ReactFiberConfigWithNoHydration.js | 2 + .../src/ReactFiberHydrationContext.js | 67 ++++++++++++++++--- .../src/forks/ReactFiberConfig.custom.js | 3 + 11 files changed, 181 insertions(+), 59 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 281ec1fea0582..9e2c5e41c062f 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -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, @@ -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, diff --git a/packages/react-dom-bindings/src/client/validateDOMNesting.js b/packages/react-dom-bindings/src/client/validateDOMNesting.js index f910455f008be..ab49cb4f4b025 100644 --- a/packages/react-dom-bindings/src/client/validateDOMNesting.js +++ b/packages/react-dom-bindings/src/client/validateDOMNesting.js @@ -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; @@ -455,7 +455,7 @@ function validateDOMNesting( : findInvalidAncestorForTag(childTag, ancestorInfo); const invalidParentOrAncestor = invalidParent || invalidAncestor; if (!invalidParentOrAncestor) { - return; + return true; } const ancestorTag = invalidParentOrAncestor.tag; @@ -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; @@ -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}; diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 53ad186761b74..6541293f51333 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -2188,8 +2188,9 @@ describe('ReactDOMComponent', () => { ); }); }).toErrorDev([ - 'Warning: cannot appear as a child of ' + - '
.' + + 'Warning: In HTML, cannot be a child of ' + + '
.\n' + + 'This will cause a hydration error.' + '\n in tr (at **)' + '\n in div (at **)', ]); @@ -2208,8 +2209,9 @@ describe('ReactDOMComponent', () => { ); }); }).toErrorDev( - 'Warning:

cannot appear as a descendant ' + - 'of

.' + + 'Warning: In HTML,

cannot be a descendant ' + + 'of

.\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 **)', @@ -2241,22 +2243,25 @@ describe('ReactDOMComponent', () => { root.render(); }); }).toErrorDev([ - 'Warning: cannot appear as a child of ' + + 'Warning: In HTML, cannot be a child of ' + '. Add a , or 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 .' + + 'Warning: In HTML, text nodes cannot be a ' + + 'child of .\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
. 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
. 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 **)', ]); @@ -2283,9 +2288,10 @@ describe('ReactDOMComponent', () => { root.render( ); }); }).toErrorDev([ - 'Warning: Whitespace text nodes cannot ' + - "appear as a child of
. 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
. 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 **)', ]); @@ -2311,8 +2317,9 @@ describe('ReactDOMComponent', () => { ); }); }).toErrorDev([ - 'Warning: Text nodes cannot appear as a ' + - 'child of .' + + 'Warning: In HTML, text nodes cannot be a ' + + 'child of .\n' + + 'This will cause a hydration error.' + '\n in tr (at **)' + '\n in Row (at **)' + '\n in tbody (at **)' + diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 64f79140917c8..31c1607cc149c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -523,7 +523,7 @@ describe('ReactDOMFloat', () => { }).toErrorDev( [ 'Cannot render