Consider Text node in the root hydration
Summary
When a 3rd party script adds a text node in the body, the hydration fails instead of trying to get the next sibling to compare. With this PR the canHydrateInstance() keeps looking for the nextSibling considering also text nodes.
The issue can be viewed in this repo.
It is related to the current opening issue regarding hydration issues #24430
Frameworks usually uses text nodes as a fragment since DocumentFragment loses the reference of their children as they are appended to the DOM.
How did you test this change?
- Added a test case with issue
- Changed with the same code implemented locally in the browser to fix the issue as shown bellow
https://github.com/user-attachments/assets/fda7c7d6-acc4-4078-b19d-1728c30f55c0
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| react-compiler-playground | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Sep 29, 2024 11:29pm |
The size diff is too large to display in a single comment. The GitHub action for this pull request contains an artifact called 'sizebot-message.md' with the full message.
Generated by :no_entry_sign: dangerJS against 77761222acc51f97d345c7698fff7e5ffafe93c4
Thank you @gnoff for the review, and your comment make sense. We were able to remove this use case of empty text around the div on our side, but I still think it should be handled properly as it is common for frameworks to use empty text node for fragments.
I have some other use cases that I am noticing causes hydration issues:
- Empty text node added around elements (The use case that this PR was trying to solve)
- A new element is prepended to the body with the same element nodeType
- A new class was added to the body (this is a dev warning)
I added those use cases in this repo.
I was wondering if we could add a flag on the server on the elements that needs hydration and the hydration checks only those elements (like a d-rh="<random>" attribute). In that way during hydration we keep getting the next hydratable element also based on this attribute and ignore newly added elements.
<html>
<script>window.__reactHydrateDataAttribute = 'a1b2c';</script>
<head></head>
<body data-rh="a1b2c">
<div></div> <!-- Added by a script -->
<div data-rh="a1b2c"></div>
</body>
</html>
The random it is just to make sure it doesn't conflict, but it can be removed for simplicity. The downside is that now each element will have this, but we could also use only on the children of the body instead of all the elements.
@gnoff, it seems that Solid utilizes a data-hk attribute to indicate a hydration key, enabling fine-grained hydration based on sibling and child elements. While this addresses a different challenge and isn't necessary in React due to the virtual DOM's handling of such issues, a similar strategy could be implemented to identify elements that require hydration. This ensures that non-React elements remain untouched, in contrast to other frameworks that may remove them. Additionally, the hydration key can be generated per request and limited to direct children of the body, helping to minimize payload size. It would be worthwhile to test the performance impact of checking whether an element is a child of the body versus adding the attribute to all elements, which would increase the payload size.
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.
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!