Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better identify nodes in rehydration warnings #12115

Closed
wants to merge 2 commits into from
Closed

Better identify nodes in rehydration warnings #12115

wants to merge 2 commits into from

Conversation

giles-v
Copy link

@giles-v giles-v commented Jan 29, 2018

This is related to #10307 and #10085. I didn't want to open a new ticket given that there's overlap, but I'm happy to do so if preferred.

Summary: the warning...

Did not expect server HTML to contain a <div> in <div>.

... is too vague to debug.

This PR changes this to:

Did not expect server HTML to contain <div class="child"> in <div class="parent">.

I've not added new tests, because the existing test for the extra <meta> element already exposes the new behavior (and that test is updated).

sompylasar added a commit to sompylasar/react that referenced this pull request Feb 26, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 6c425e7

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
@sompylasar
Copy link
Contributor

Hi @giles-v , I've incorporated your idea and partially implementation into my PR that enhances the warnings with not just node signatures, but also diffs and component stacks. Please have a look: #12063 (comment)

@giles-v
Copy link
Author

giles-v commented Feb 26, 2018

Hey @sompylasar thanks for the link! Looks pretty impressive, I will leave a comment over there too.

sompylasar added a commit to sompylasar/react that referenced this pull request May 22, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 6c425e7

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jun 8, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 6c425e7

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jun 9, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 6c425e7

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jun 19, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
@pull-bot
Copy link

Details of bundled changes.

Comparing: b1b3acb...8d77bd1

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
React-dev.js +1.3% +1.6% 45.8 KB 46.38 KB 12.48 KB 12.68 KB FB_DEV
React-prod.js 🔺+2.7% -2.2% 13.43 KB 13.79 KB 3.73 KB 3.64 KB FB_PROD

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js 0.0% +0.1% 624.3 KB 624.58 KB 145.77 KB 145.87 KB UMD_DEV
react-dom.development.js 0.0% +0.1% 616.71 KB 616.99 KB 143.68 KB 143.78 KB NODE_DEV
ReactDOM-dev.js -7.1% -4.5% 618.04 KB 574.4 KB 139.01 KB 132.75 KB FB_DEV
ReactDOM-prod.js -6.2% -2.1% 284.94 KB 267.3 KB 52.3 KB 51.2 KB FB_PROD
ReactTestUtils-dev.js -1.8% -1.4% 36.89 KB 36.23 KB 10.46 KB 10.32 KB FB_DEV
ReactDOMUnstableNativeDependencies-dev.js -1.0% -0.7% 57.09 KB 56.52 KB 14.56 KB 14.46 KB FB_DEV
ReactDOMUnstableNativeDependencies-prod.js -0.5% -1.8% 26.34 KB 26.2 KB 5.38 KB 5.28 KB FB_PROD
ReactDOMServer-dev.js -3.2% -2.9% 94.19 KB 91.17 KB 24.05 KB 23.35 KB FB_DEV
ReactDOMServer-prod.js 🔺+0.9% 🔺+3.2% 31.62 KB 31.91 KB 7.78 KB 8.02 KB FB_PROD
ReactDOM-dev.js 0.0% +0.1% 626.77 KB 627.08 KB 143.11 KB 143.23 KB FB_WWW_DEV

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactART-dev.js -12.1% -8.7% 346.44 KB 304.62 KB 70.26 KB 64.14 KB FB_DEV
ReactART-prod.js -10.1% -5.6% 167.61 KB 150.6 KB 27.75 KB 26.19 KB FB_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js -6.5% -3.5% 456.78 KB 427.2 KB 97.42 KB 94.04 KB RN_DEV
ReactNativeRenderer-prod.js -9.7% -5.7% 218.3 KB 197.07 KB 36.68 KB 34.59 KB RN_PROD
ReactFabric-dev.js -3.9% +0.1% 439.19 KB 422.04 KB 92.99 KB 93.08 KB RN_DEV
ReactFabric-prod.js -3.5% 🔺+1.5% 200.63 KB 193.62 KB 33.57 KB 34.06 KB RN_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactTestRenderer-dev.js -12.7% -9.2% 345.1 KB 301.35 KB 69.35 KB 62.98 KB FB_DEV
ReactShallowRenderer-dev.js +27.4% +19.3% 14.41 KB 18.36 KB 3.55 KB 4.23 KB FB_DEV

Generated by 🚫 dangerJS

@giles-v
Copy link
Author

giles-v commented Jun 19, 2018

@mihalskiy @gaearon would be good to get some clarity on whether this PR is expected to go in. It was approved almost 5 months ago, and is much more modest than #12063 - I prefer the simplicity of this PR, but if you're leaning the other way, I'll close this. Thank you!

@gaearon
Copy link
Collaborator

gaearon commented Jun 19, 2018

So, unfortunately I don't use server rendering so it's very hard for me to assess which one is more useful. I've literally never debugged a mismatch in my life. So I'm trusting folks from the community to decide on the approach.

@mxstbr said he found #12063 helpful. I'd love feedback on this simpler approach too.

sompylasar added a commit to sompylasar/react that referenced this pull request Jun 23, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jul 8, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jul 16, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jul 23, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Jul 23, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Sep 8, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
sompylasar added a commit to sompylasar/react that referenced this pull request Sep 8, 2018
Renders DOM attributes in the tags mentioned in the warnings. Borrows the idea and partially implementation of `getNodeSignature` from @giles-v facebook#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 <div>{['children ', …]}</div> in <div>nested<!-- --> <p>children <b>text</b></p></div>.
  <div>
    {'nested'}
    {' '}
    <p>children <b>text</b></p>
+   <div>{['children ', …]}</div>
  </div>
    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 a5e9a70

https://user-images.githubusercontent.com/498274/36652198-11bb46fe-1a62-11e8-9fa2-a612827d1463.gif
@stale
Copy link

stale bot commented Jan 10, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Jan 17, 2020

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

@stale stale bot closed this Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Resolution: Stale Automatically closed due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants