-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Slightly improve performance of hydration. #15998
Conversation
Avoid loading nodeType and data couple times from the same node in a row, but instead load them only once, which will help engines to run this code faster, especially during startup of the application. The general approach is still not ideal, since hydrating this way forces the browser engine to materialize JavaScript wrapper objects for all DOM nodes, even if they are not interesting to hydration itself.
ReactDOM: size: 0.0%, gzip: -0.1% Details of bundled changes.Comparing: 9b55bcf...fba82af react-dom
Generated by 🚫 dangerJS |
In normal usage, it is not expected that there will be any non-hydratable nodes. So these would all be materialized anyway. This is here mostly as a precaution against browser extensions or proxies inserting unexpected content. |
Thanks. I'm wondering if it's possible to do something about this in general. While it's probably possible to optimize this a bit more with the current approach, having to materialize all these wrapper objects is always gonna remain a bottleneck. cc @developit @ timneutkens |
One thing we could do is drop all validation but still preresolve references to the node. Is there a way to walk the DOM while not materializing JS wrappers or cheaper ones? We used to have a lazy approach but that instead paid the cost when the user interacted with something and also added complexity and meta-data that made it overall not worth it. We’re moving to a selective hydration approach driven by interaction which effectively makes the paths lazy but also able to warm up in preparation for an interaction. So the characteristics are about to change. |
Interesting. The lazy approach sounds like a viable path. Not validating, i.e. not loading |
} | ||
if (enableSuspenseServerRenderer) { | ||
if (nodeType === COMMENT_NODE) { | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should actually have been if (nodeType !== COMMENT_NODE) continue
. Logic is hard. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, test coverage is hard :D
Avoid loading nodeType and data couple times from the same node in a row,
but instead load them only once, which will help engines to run this code
faster, especially during startup of the application. The general approach
is still not ideal, since hydrating this way forces the browser engine
to materialize JavaScript wrapper objects for all DOM nodes, even if they
are not interesting to hydration itself.