From 73e0abad20369e40452fdfc4e1120228e56995fa Mon Sep 17 00:00:00 2001 From: Ivan Babak Date: Sun, 25 Feb 2018 19:31:36 -0800 Subject: [PATCH] Add diff to hydrate warnings (#10085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v https://github.com/facebook/react/pull/12115 Renders DOM diff showing visually the location where the hydration failed. Example warning with a diff: ``` Warning: Expected server HTML to contain a matching
{['children ', …]}
in
nested

children text

.
{'nested'} {' '}

children text

+
{['children ', …]}
in div (at SSRMismatchTest.js:280) in div (at SSRMismatchTest.js:275) in div (at SSRMismatchTest.js:308) in SSRMismatchTest (at App.js:14) in div (at App.js:11) in body (at Chrome.js:17) in html (at Chrome.js:9) in Chrome (at App.js:10) in App (at index.js:8) ``` Requires changes to ReactFiberReconciler interface functions that handle hydration errors to distinguish insertion from replacement and show insertion as one added line in the diff; show replacement as one removed, one added line, at correct position among the parentInstance's DOM children: - add `index` (use `fiber.index`) to point at which child node the insertion or replacement occurs; - add `isReplaced` to distinguish insertion from replacement. Extends the proof-of-concept at commit 6c425e7b90cd61f1124c566b26fa2a5d00261b1b https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif --- .../ssr/src/components/SSRMismatchTest.js | 41 ++- .../src/__tests__/ReactMount-test.js | 205 +++++++++--- .../src/__tests__/ReactRenderDocument-test.js | 83 ++++- .../src/client/ReactDOMFiberComponent.js | 305 ++++++++++++++---- .../src/client/ReactDOMHostConfig.js | 26 +- .../src/ReactFiberHydrationContext.js | 21 +- scripts/rollup/results.json | 72 ++--- 7 files changed, 593 insertions(+), 160 deletions(-) diff --git a/fixtures/ssr/src/components/SSRMismatchTest.js b/fixtures/ssr/src/components/SSRMismatchTest.js index 74918fa2cccae..6b6c8e7de8013 100644 --- a/fixtures/ssr/src/components/SSRMismatchTest.js +++ b/fixtures/ssr/src/components/SSRMismatchTest.js @@ -145,7 +145,7 @@ const testCases = [ key: 'ssr-warnForDeletedHydratableElement-didNotHydrateInstance', renderServer: () => (
-
+
SSRMismatchTest default text
), @@ -243,18 +243,45 @@ const testCases = [ ), }, { - key: 'ssr-warnForInsertedHydratedText-didNotFindHydratableTextInstance', + key: + 'ssr-warnForInsertedHydratedText-didNotFindHydratableTextInstance-replacement', renderServer: () => (
- - + nested{' '} +

+ children text +

), renderBrowser: () => (
- - SSRMismatchTest client text - + nested{' '} +
+ children text +
+
+ ), + }, + { + key: + 'ssr-warnForInsertedHydratedText-didNotFindHydratableTextInstance-insertion', + renderServer: () => ( +
+ nested{' '} +

+ children text +

+
+ ), + renderBrowser: () => ( +
+ nested{' '} +

+ children text +

+
+ children text +
), }, diff --git a/packages/react-dom/src/__tests__/ReactMount-test.js b/packages/react-dom/src/__tests__/ReactMount-test.js index be55ecfdde76f..7c7e1e2fcf3f8 100644 --- a/packages/react-dom/src/__tests__/ReactMount-test.js +++ b/packages/react-dom/src/__tests__/ReactMount-test.js @@ -125,7 +125,8 @@ describe('ReactMount', () => { container.innerHTML = ReactDOMServer.renderToString(
) + ' '; expect(() => ReactDOM.hydrate(
, container)).toWarnDev( - 'Did not expect server HTML to contain the text node " " in .', + "Warning: Did not expect server HTML to contain the text node {' '}" + + ' in
.', ); }); @@ -134,7 +135,8 @@ describe('ReactMount', () => { container.innerHTML = ' ' + ReactDOMServer.renderToString(
); expect(() => ReactDOM.hydrate(
, container)).toWarnDev( - 'Did not expect server HTML to contain the text node " " in .', + "Warning: Did not expect server HTML to contain the text node {' '}" + + ' in
.', ); }); @@ -249,8 +251,52 @@ describe('ReactMount', () => { ); }); - it('should warn when a hydrated element has children mismatch', () => { - // See fixtures/ssr: ssr-warnForInsertedHydratedText-didNotFindHydratableTextInstance + it('should warn when a hydrated element has children mismatch, warning has replacement diff', () => { + // See fixtures/ssr: ssr-warnForInsertedHydratedText-didNotFindHydratableTextInstance-replacement + + class Component extends React.Component { + render() { + return this.props.children; + } + } + + const div = document.createElement('div'); + const markup = ReactDOMServer.renderToString( + + nested{' '} +

+ children text +

+
, + ); + div.innerHTML = markup; + + expect(() => + ReactDOM.hydrate( + + nested{' '} +
+ children text +
+
, + div, + ), + ).toWarnDev( + "Warning: Expected server HTML to contain a matching
{['children ', …]}
" + + ' in
nested

children text

.\n' + + '
\n' + + " {'nested'}\n" + + " {' '}\n" + + '-

children text

\n' + + "+
{['children ', …]}
\n" + + '
\n' + + ' in div (at **)\n' + + ' in Component (at **)', + ); + }); + + it('should warn when a hydrated element has extra child element, warning has insertion diff', () => { + // See fixtures/ssr: ssr-warnForInsertedHydratedText-didNotFindHydratableTextInstance-insertion class Component extends React.Component { render() { @@ -273,6 +319,9 @@ describe('ReactMount', () => { ReactDOM.hydrate( nested{' '} +

+ children text +

children text
@@ -280,15 +329,14 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Expected server HTML to contain a matching
in
.\n' + - '
\n' + - ' nested\n' + - ' \n' + - '- \n' + - '+
\n' + - '

children text

\n' + - '
\n' + - '\n' + + "Warning: Expected server HTML to contain a matching
{['children ', …]}
" + + ' in
nested

children text

.\n' + + '
\n' + + " {'nested'}\n" + + " {' '}\n" + + '

children text

\n' + + "+
{['children ', …]}
\n" + + '
\n' + ' in div (at **)\n' + ' in Component (at **)', ); @@ -307,7 +355,7 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Text content did not match. ' + + 'Warning: Text content did not match. ' + 'Server: "This markup contains an nbsp entity:   server text" ' + 'Client: "This markup contains an nbsp entity:   client text"\n' + ' in div (at **)', @@ -333,7 +381,7 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Prop `data-ssr-extra-prop` did not match. ' + + 'Warning: Prop `data-ssr-extra-prop` did not match. ' + 'Server: "null" ' + 'Client: "true"\n' + ' in div (at **)', @@ -380,7 +428,7 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Extra attributes from the server: data-ssr-extra-prop,data-ssr-extra-prop-2\n' + + 'Warning: Extra attributes from the server: data-ssr-extra-prop,data-ssr-extra-prop-2\n' + ' in div (at **)', ); }); @@ -393,7 +441,7 @@ describe('ReactMount', () => { div.innerHTML = markup; expect(() => ReactDOM.hydrate(
, div)).toWarnDev( - 'Expected `onClick` listener to be a function, instead got `false`.\n\n' + + 'Warning: Expected `onClick` listener to be a function, instead got `false`.\n\n' + 'If you used to conditionally omit it with onClick={condition && value}, ' + 'pass onClick={condition ? value : undefined} instead.\n' + ' in div (at **)', @@ -408,7 +456,7 @@ describe('ReactMount', () => { div.innerHTML = markup; expect(() => ReactDOM.hydrate(
, div)).toWarnDev( - 'Expected `onClick` listener to be a function, instead got a value of `string` type.\n' + + 'Warning: Expected `onClick` listener to be a function, instead got a value of `string` type.\n' + ' in div (at **)', ); }); @@ -417,6 +465,7 @@ describe('ReactMount', () => { // See fixtures/ssr: ssr-warnForDeletedHydratableElement-didNotHydrateContainerInstance const div = document.createElement('div'); + div.setAttribute('data-ssr-mismatch-test-hydrate-root', ''); const markup = 'SSRMismatchTest first text' + '
' + @@ -433,7 +482,17 @@ describe('ReactMount', () => { ], div, ), - ).toWarnDev('Did not expect server HTML to contain a
in
.'); + ).toWarnDev( + 'Warning: Did not expect server HTML to contain a
' + + ' in
SSRMismatchTest first text' + + '

SSRMismatchTest second text
.\n' + + '
\n' + + " {'SSRMismatchTest first text'}\n" + + '
\n' + + '-
\n' + + " {'SSRMismatchTest second text'}\n" + + '
', + ); }); it('should warn when hydrate removes an element from a server-rendered sequence', () => { @@ -442,8 +501,13 @@ describe('ReactMount', () => { const div = document.createElement('div'); const markup = ReactDOMServer.renderToString(
-
+
SSRMismatchTest default text
+
+
+
+
+
, ); div.innerHTML = markup; @@ -456,7 +520,18 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Did not expect server HTML to contain a
in
.\n' + + 'Warning: Did not expect server HTML to contain a
SSRMismatchTest default text
' + + ' in
SSRMismatchTest default text
' + + '
.\n' + + '
\n' + + '-
SSRMismatchTest default text
\n' + + ' \n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + ' in span (at **)\n' + ' in div (at **)', ); @@ -466,14 +541,34 @@ describe('ReactMount', () => { // See fixtures/ssr: ssr-warnForDeletedHydratableText-didNotHydrateContainerInstance const div = document.createElement('div'); + div.setAttribute('data-ssr-mismatch-test-hydrate-root', ''); const markup = - 'SSRMismatchTest server text' + '
' + 'SSRMismatchTest default text'; + 'SSRMismatchTest server text' + + '
' + + 'SSRMismatchTest default text' + + '
' + + '
' + + '
' + + '
' + + '
'; div.innerHTML = markup; expect(() => ReactDOM.hydrate([
, 'SSRMismatchTest default text'], div), ).toWarnDev( - 'Did not expect server HTML to contain the text node "SSRMismatchTest server text" in
.\n' + + "Warning: Did not expect server HTML to contain the text node {'SSRMismatchTest server text'}" + + ' in
SSRMismatchTest server text' + + '
SSRMismatchTest default text
<…
.\n' + + '
\n' + + "- {'SSRMismatchTest server text'}\n" + + '
\n' + + " {'SSRMismatchTest default text'}\n" + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + ' in br (at **)', ); }); @@ -486,6 +581,11 @@ describe('ReactMount', () => {
SSRMismatchTest server text +
+
+
+
+
, ); div.innerHTML = markup; @@ -498,7 +598,18 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Did not expect server HTML to contain the text node "SSRMismatchTest server text" in
.\n' + + "Warning: Did not expect server HTML to contain the text node {'SSRMismatchTest server text'}" + + ' in
SSRMismatchTest server text' + + '
.\n' + + '
\n' + + "- {'SSRMismatchTest server text'}\n" + + ' \n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + ' in span (at **)\n' + ' in div (at **)', ); @@ -514,12 +625,12 @@ describe('ReactMount', () => { expect(() => ReactDOM.hydrate(SSRMismatchTest default text, div), ).toWarnDev( - 'Expected server HTML to contain a matching in
.\n' + - '
\n' + - '- SSRMismatchTest default text\n' + - '+ \n' + - '
\n' + - '\n' + + 'Warning: Expected server HTML to contain a matching SSRMismatchTest default text' + + ' in
SSRMismatchTest default text
.\n' + + '
\n' + + "- {'SSRMismatchTest default text'}\n" + + '+ SSRMismatchTest default text\n' + + '
\n' + ' in span (at **)', ); }); @@ -543,12 +654,12 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Expected server HTML to contain a matching

in

.\n' + - '
\n' + - '- SSRMismatchTest default text\n' + - '+

\n' + - '

\n' + - '\n' + + 'Warning: Expected server HTML to contain a matching

SSRMismatchTest default text

' + + ' in
SSRMismatchTest default text
.\n' + + '
\n' + + '- SSRMismatchTest default text\n' + + '+

SSRMismatchTest default text

\n' + + '
\n' + ' in p (at **)\n' + ' in div (at **)', ); @@ -566,7 +677,8 @@ describe('ReactMount', () => { expect(() => ReactDOM.hydrate('SSRMismatchTest default text', div), ).toWarnDev( - 'Expected server HTML to contain a matching text node for "SSRMismatchTest default text" in
.', + "Warning: Expected server HTML to contain a matching text node for {'SSRMismatchTest default text'}" + + ' in
SSRMismatchTest default text
.', ); }); @@ -578,6 +690,11 @@ describe('ReactMount', () => {
+
+
+
+
+
, ); div.innerHTML = markup; @@ -592,7 +709,19 @@ describe('ReactMount', () => { div, ), ).toWarnDev( - 'Expected server HTML to contain a matching text node for "SSRMismatchTest client text" in
.\n' + + "Warning: Expected server HTML to contain a matching text node for {'SSRMismatchTest client text'}" + + ' in
' + + '
.\n' + + '
\n' + + ' \n' + + '- \n' + + "+ {'SSRMismatchTest client text'}\n" + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + + '
\n' + ' in div (at **)', ); }); diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index c9b8cb89f3c53..26f23a18df0f5 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -364,15 +364,37 @@ describe('rendering React components at document', () => { expect(testDocument.body.innerHTML).toBe('Hello world'); }); - it('renders over an existing text child without throwing', () => { + it('renders over an existing text child without throwing, warning has deletion diff', () => { const container = document.createElement('div'); container.textContent = 'potato'; expect(() => ReactDOM.hydrate(
parsnip
, container)).toWarnDev( - 'Expected server HTML to contain a matching
in
.', + 'Warning: Expected server HTML to contain a matching
parsnip
in
potato
.\n' + + '
\n' + + "- {'potato'}\n" + + '+
parsnip
\n' + + '
\n' + + ' in div (at **)', ); expect(container.textContent).toBe('parsnip'); }); + it('renders an array of texts over an existing text child without throwing, warning has deletion diff', () => { + const container = document.createElement('div'); + container.textContent = 'potato'; + expect(() => + ReactDOM.hydrate(
{['parsnip', 'carrot']}
, container), + ).toWarnDev( + "Warning: Expected server HTML to contain a matching
{['parsnip', 'carrot']}
" + + ' in
potato
.\n' + + '
\n' + + "- {'potato'}\n" + + "+
{['parsnip', 'carrot']}
\n" + + '
\n' + + ' in div (at **)', + ); + expect(container.textContent).toBe('parsnipcarrot'); + }); + it('should give helpful errors on state desync', () => { class Component extends React.Component { render() { @@ -417,7 +439,62 @@ describe('rendering React components at document', () => { // getTestDocument() has an extra that we didn't render. expect(() => ReactDOM.hydrate(, testDocument), - ).toWarnDev('Did not expect server HTML to contain a in .'); + ).toWarnDev( + 'Warning: Did not expect server HTML to contain a ' + + ' in test doc.\n' + + ' \n' + + '- \n' + + ' test doc\n' + + ' \n' + + ' in title (at **)\n' + + ' in head (at **)\n' + + ' in html (at **)\n' + + ' in Component (at **)', + ); + expect(testDocument.body.innerHTML).toBe('Hello world'); + }); + + it('clips the hydrate warning if the content is too long', () => { + let longTitle = 'test doc'; + while (longTitle.length < 100) { + longTitle += ' long title'; + } + const testDocument = getTestDocument( + '' + + longTitle + + '', + ); + + class Component extends React.Component { + render() { + return ( + + + Hello World + + {this.props.text} + + ); + } + } + + // getTestDocument() has an extra that we didn't render. + expect(() => + ReactDOM.hydrate(, testDocument), + ).toWarnDev( + 'Warning: Did not expect server HTML to contain a ' + + ' in ' + + 'test doc long title long title long title long title long title long ti…</head>.\n' + + ' <head>\n' + + '- <meta charset="utf-8" />\n' + + ' <title>test doc long title long title long title long title long title' + + ' long title long title long title lon…\n' + + ' \n' + + ' in title (at **)\n' + + ' in head (at **)\n' + + ' in html (at **)\n' + + ' in Component (at **)', + ); expect(testDocument.body.innerHTML).toBe('Hello world'); }); diff --git a/packages/react-dom/src/client/ReactDOMFiberComponent.js b/packages/react-dom/src/client/ReactDOMFiberComponent.js index 54bf2b7c73f71..2f77cf009e6d9 100644 --- a/packages/react-dom/src/client/ReactDOMFiberComponent.js +++ b/packages/react-dom/src/client/ReactDOMFiberComponent.js @@ -40,11 +40,14 @@ import { } from '../shared/DOMProperty'; import assertValidProps from '../shared/assertValidProps'; import { + ELEMENT_NODE, + TEXT_NODE, + COMMENT_NODE, DOCUMENT_NODE, DOCUMENT_FRAGMENT_NODE, - ELEMENT_NODE, } from '../shared/HTMLNodeType'; import isCustomComponent from '../shared/isCustomComponent'; +import omittedCloseTags from '../shared/omittedCloseTags'; import possibleStandardNames from '../shared/possibleStandardNames'; import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook'; import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook'; @@ -68,6 +71,10 @@ const HTML = '__html'; const {html: HTML_NAMESPACE} = Namespaces; let getStack = emptyFunction.thatReturns(''); +let getNodeSignature = emptyFunction.thatReturns('<…>'); +let getTagWithPropsSignature = emptyFunction.thatReturns('<…>'); +let getNodeSurroundingsAndDiff = emptyFunction.thatReturns(''); +let findNodeIndex = emptyFunction.thatReturns(-1); let warnedUnknownTags; let suppressHydrationWarning; @@ -84,6 +91,190 @@ let normalizeHTML; if (__DEV__) { getStack = getCurrentFiberStackAddendum; + getNodeSignature = function(node: Node, openTagOnly = false) { + // TODO: How do we want to print quotes inside quotes? + // TODO: How do we want to print special characters such as  ? + const nodeType = node.nodeType; + if (nodeType === COMMENT_NODE) { + return ''; + } + if (nodeType === TEXT_NODE) { + return "{'" + node.textContent + "'}"; + } + if (nodeType !== ELEMENT_NODE) { + return node.nodeName; + } + const tag = node.nodeName.toLowerCase(); + let ret = '<' + tag; + if (node instanceof Element) { + const attrs = node.attributes; + const ic = attrs.length; + for (let i = 0; i < ic; ++i) { + ret += ' ' + attrs[i].name + '="' + attrs[i].value + '"'; + } + } + if (openTagOnly) { + ret += '>'; + } else { + const childrenString = + node instanceof Element ? node.innerHTML : node.textContent; + if (omittedCloseTags.hasOwnProperty(tag)) { + ret += ' />'; + } else { + if (childrenString) { + ret += + '>' + + childrenString.substring(0, 100) + + (childrenString.length > 100 ? '…' : '') + + ''; + } + } + return ret; + }; + + const objectPrototypeToString = Object.prototype.toString; + + // TODO: Reuse RESERVED_PROPS from ReactPartialRenderer. + const RESERVED_PROPS = { + children: null, + dangerouslySetInnerHTML: null, + suppressContentEditableWarning: null, + suppressHydrationWarning: null, + }; + + getTagWithPropsSignature = function(tag: string, props: Object) { + let ret = '<' + tag; + + for (const propKey in props) { + if (!props.hasOwnProperty(propKey)) { + continue; + } + let propValue = props[propKey]; + if (propValue == null) { + continue; + } + if (RESERVED_PROPS.hasOwnProperty(propKey)) { + continue; + } + let markup = null; + if (propKey === STYLE) { + markup = propKey + '={…}'; + } else if (typeof propValue === 'function') { + markup = propKey + '={' + propValue.name || propValue.toString() + '}'; + } else { + try { + markup = propKey + '={' + JSON.stringify(propValue) + '}'; + } catch (ex) { + markup = + propKey + '={' + objectPrototypeToString.call(propValue) + '}'; + } + } + if (markup) { + ret += ' ' + markup; + } + } + + if (omittedCloseTags.hasOwnProperty(tag)) { + ret += ' />'; + } else { + if (typeof props.children === 'string') { + ret += '>' + props.children + '{['; + const ic = props.children.length; + for (let i = 0; i < ic; ++i) { + ret += i > 0 ? ', ' : ''; + const child = props.children[i]; + if (typeof child === 'string') { + ret += "'" + child + "'"; + } else { + ret += '…'; + } + } + ret += ']}{…}'; + } + + return ret; + }; + + getNodeSurroundingsAndDiff = function( + parentNode: Element | Document, + deletedIndex: number, + insertedIndex: number, + insertTag: string | null, + insertProps: Object | null, + insertText: string | null, + ) { + let ret = ''; + const INDENT = ' '; + const DIFF_ADDED = '\n+ '; + const DIFF_REMOVED = '\n- '; + const DIFF_UNCHANGED = '\n '; + ret += DIFF_UNCHANGED + getNodeSignature(parentNode, true); + let inserted = false; + const insert = () => { + if (!inserted) { + if (insertTag) { + inserted = true; + ret += + DIFF_ADDED + + INDENT + + getTagWithPropsSignature(insertTag, insertProps || {}); + } else if (typeof insertText === 'string') { + inserted = true; + ret += DIFF_ADDED + INDENT + "{'" + insertText + "'}"; + } + } + }; + const childNodes = parentNode.childNodes; + const ic = childNodes.length; + let skipped = 0; + for (let i = 0; i < ic; ++i) { + const node = childNodes[i]; + // TODO: Copy-paste condition from `getNextHydratableSibling`, reuse? + if ( + node && + node.nodeType !== ELEMENT_NODE && + node.nodeType !== TEXT_NODE + ) { + ++skipped; + continue; + } + if (i - skipped === deletedIndex) { + ret += DIFF_REMOVED + INDENT + getNodeSignature(node); + } else { + ret += DIFF_UNCHANGED + INDENT + getNodeSignature(node); + } + if (i - skipped === insertedIndex) { + insert(); + } + } + insert(); + // TODO: Cannot tell if more sibling React elements were expected to be hydrated after the current one. + ret += DIFF_UNCHANGED + ''; + return ret; + }; + + findNodeIndex = function(nodes: NodeList, node: Node) { + const ic = nodes.length; + for (let i = 0; i < ic; ++i) { + if (nodes[i] === node) { + return i; + } + } + return -1; + }; + warnedUnknownTags = { // Chrome is the only major browser not shipping