Skip to content

Commit

Permalink
Change hydration warning index algorithm to use .return pointers, n…
Browse files Browse the repository at this point in the history
…ot stack

The stack was required to track where to return after child siblings traversal.

The `fiber.return` pointers were made for that purpose,
it's safe to overwrite them because they are only used to traverse the fiber tree,
and their values outside the traversal functions are not relied upon.

One test fails for yet unknown reason, the text node is inserted too early:
```
  ● ReactMount › should warn when hydrate inserts a text node after matching elements (insertion diff)

    Error: Unexpected warning recorded: - Expected
    + Received

      Warning: Expected server HTML to contain a matching text node for {'SSRMismatchTest client text'} in <div>.

        <div data-reactroot="">
          <div data-ssr-mismatch-padding-before="1"><span></span></div>
          <div data-ssr-mismatch-padding-before="2"></div>
    + +   {'SSRMismatchTest client text'}
          <div data-ssr-mismatch-padding-before="3"></div>
          <div data-ssr-mismatch-padding-before="4"></div>
          <div data-ssr-mismatch-padding-before="5"></div>
          <div data-ssr-mismatch-padding-before="6"></div>
          <div data-ssr-mismatch-padding-before="7"></div>
          <div data-ssr-mismatch-padding-before="8"></div>
          <div data-ssr-mismatch-padding-before="9"></div>
          <div data-ssr-mismatch-padding-before="10"></div>
          <div data-ssr-mismatch-padding-before="11"></div>
          <div data-ssr-mismatch-padding-before="12"></div>
          <div data-ssr-mismatch-padding-before="13"></div>
    - +   {'SSRMismatchTest client text'}
        </div>

          in div (at **)
```
  • Loading branch information
sompylasar committed Sep 8, 2018
1 parent 221bc1f commit 1a15f08
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 29 deletions.
40 changes: 25 additions & 15 deletions packages/react-dom/src/__tests__/ReactMount-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,9 @@ describe('ReactMount', () => {
render() {
return (
<React.Fragment>
<div data-ssr-mismatch-padding-before="1" />
<div data-ssr-mismatch-padding-before="1">
<span />
</div>
<TestPaddingBeforeInnerComponent />
<div data-ssr-mismatch-padding-before="4" />
<div data-ssr-mismatch-padding-before="5" />
Expand Down Expand Up @@ -739,7 +741,7 @@ describe('ReactMount', () => {
).toWarnDev(
'Warning: Expected server HTML to contain a matching <h2> in <div>.\n\n' +
' <div data-reactroot="">\n' +
' <div data-ssr-mismatch-padding-before="1"></div>\n' +
' <div data-ssr-mismatch-padding-before="1"><span></span></div>\n' +
' <div data-ssr-mismatch-padding-before="2"></div>\n' +
' <div data-ssr-mismatch-padding-before="3"></div>\n' +
' <div data-ssr-mismatch-padding-before="4"></div>\n' +
Expand Down Expand Up @@ -910,15 +912,15 @@ describe('ReactMount', () => {

class TestPaddingBeforeInnerInnerComponent extends React.Component {
render() {
return <div data-ssr-mismatch-padding-before="6" />;
return <div data-ssr-mismatch-padding-before="8" />;
}
}
class TestPaddingBeforeInnerComponent extends React.Component {
render() {
return (
<React.Fragment>
<div data-ssr-mismatch-padding-before="4" />
<div data-ssr-mismatch-padding-before="5" />
<div data-ssr-mismatch-padding-before="6" />
<div data-ssr-mismatch-padding-before="7" />
<TestPaddingBeforeInnerInnerComponent />
</React.Fragment>
);
Expand All @@ -928,12 +930,11 @@ describe('ReactMount', () => {
render() {
return (
<React.Fragment>
<div data-ssr-mismatch-padding-before="2" />
<div data-ssr-mismatch-padding-before="3" />
<div data-ssr-mismatch-padding-before="4" />
<div data-ssr-mismatch-padding-before="5" />
<TestPaddingBeforeInnerComponent />
<div data-ssr-mismatch-padding-before="7" />
<div data-ssr-mismatch-padding-before="8" />
<div data-ssr-mismatch-padding-before="9" />
<div data-ssr-mismatch-padding-before="10" />
</React.Fragment>
);
}
Expand All @@ -942,32 +943,40 @@ describe('ReactMount', () => {
const div = document.createElement('div');
const markup = ReactDOMServer.renderToString(
<div>
<div data-ssr-mismatch-padding-before="1" />
<div data-ssr-mismatch-padding-before="1">
<span />
</div>
<div data-ssr-mismatch-padding-before="2" />
<div data-ssr-mismatch-padding-before="3" />
<TestPaddingBeforeComponent />
<div data-ssr-mismatch-padding-before="10" />
<div data-ssr-mismatch-padding-before="11" />
<div data-ssr-mismatch-padding-before="12" />
<div data-ssr-mismatch-padding-before="13" />
</div>,
);
div.innerHTML = markup;

expect(() =>
ReactDOM.hydrate(
<div>
<div data-ssr-mismatch-padding-before="1" />
<div data-ssr-mismatch-padding-before="1">
<span />
</div>
<div data-ssr-mismatch-padding-before="2" />
<div data-ssr-mismatch-padding-before="3" />
<TestPaddingBeforeComponent />
<div data-ssr-mismatch-padding-before="10" />
<div data-ssr-mismatch-padding-before="11" />
<div data-ssr-mismatch-padding-before="12" />
SSRMismatchTest client text
<div data-ssr-mismatch-padding-before="13" />
{'SSRMismatchTest client text'}
</div>,
div,
),
).toWarnDev(
'Warning: Expected server HTML to contain a matching text node' +
" for {'SSRMismatchTest client text'} in <div>.\n\n" +
' <div data-reactroot="">\n' +
' <div data-ssr-mismatch-padding-before="1"></div>\n' +
' <div data-ssr-mismatch-padding-before="1"><span></span></div>\n' +
' <div data-ssr-mismatch-padding-before="2"></div>\n' +
' <div data-ssr-mismatch-padding-before="3"></div>\n' +
' <div data-ssr-mismatch-padding-before="4"></div>\n' +
Expand All @@ -979,6 +988,7 @@ describe('ReactMount', () => {
' <div data-ssr-mismatch-padding-before="10"></div>\n' +
' <div data-ssr-mismatch-padding-before="11"></div>\n' +
' <div data-ssr-mismatch-padding-before="12"></div>\n' +
' <div data-ssr-mismatch-padding-before="13"></div>\n' +
"+ {'SSRMismatchTest client text'}\n" +
' </div>\n\n' +
' in div (at **)',
Expand Down
38 changes: 24 additions & 14 deletions packages/react-reconciler/src/ReactFiberHydrationContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ import type {
HostContext,
} from './ReactFiberHostConfig';

import {HostComponent, HostText, HostRoot} from 'shared/ReactWorkTags';
import {
HostComponent,
HostText,
HostRoot,
HostPortal,
} from 'shared/ReactWorkTags';
import {Deletion, Placement} from 'shared/ReactSideEffectTags';
import invariant from 'shared/invariant';

Expand Down Expand Up @@ -108,24 +113,29 @@ function insertNonHydratedInstance(
if (__DEV__) {
let hydrationWarningHostInstanceIndex = 0;
{
// Count rendered host nodes by traversing `returnFiber` subtree until `fiber` is found.
let node = returnFiber.child;
const nextNodeStack = [];
while (node && node !== fiber) {
// Find index of `fiber`, the place where hydration failed, among immediate children host nodes of `returnFiber`.
const startNode = returnFiber.child;
let node: Fiber | null = startNode;
search: while (node && node !== fiber) {
if (node.tag === HostComponent || node.tag === HostText) {
++hydrationWarningHostInstanceIndex;
} else if (node.tag === HostPortal) {
// Do not count HostPortal and do not descend into them as they do not affect the index within the parent.
} else if (node.child !== null) {
// Do not descend into HostComponent or HostText as they do not affect the index within the parent.
node.child.return = node;
node = node.child;
continue;
}
// Depth-first traversal.
if (node.child) {
if (node.sibling) {
// Remember where to continue on this tree level, then go deeper.
nextNodeStack.push(node.sibling);
while (node && node.sibling === null) {
if (node.return === null || node.return === startNode) {
break search;
}
node = node.child;
} else if (node.sibling) {
node = node.return;
}
if (node && node.sibling) {
node.sibling.return = node.return;
node = node.sibling;
} else {
node = nextNodeStack.pop();
}
}
}
Expand Down

0 comments on commit 1a15f08

Please sign in to comment.