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

[Hydration] Bail out from hydrating suspense boundary if component suspends #22567

Closed

Conversation

salazarm
Copy link
Contributor

@salazarm salazarm commented Oct 15, 2021

Summary

This fixes the issue pointed out by @gaearon in this PR: https://github.com/facebook/react/pull/21630/files.

The issue is that when a component in a boundary suspends while hydrating we attempt to hydrate its sibling. However we don't know how many nodes the suspended component would have rendered (because we don't add comment nodes to signal this since it would bloat the HTML) so we can't tell where the sibling should hydrate. Rather than attempt to hydrate it the solution in this PR bails on hydrating the boundary from within the current parent fiber. It will continue with the sibling to the parent fiber which is fine since the hydration pointer will be correct in this case.

This means we won't call render on any siblings or children but I think it should be fine since we don't reuse the work anyways. Since we don't call render we also don't "warm up" the codepath but that might be okay if there's other CPU work to do anyways we might actually see an improvement from allowing other higher priority work to happen.

How did you test this change?

Jest

@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Oct 15, 2021
@sizebot
Copy link

sizebot commented Oct 15, 2021

Comparing: cadf94d...bf891da

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js +0.02% 130.05 kB 130.08 kB +0.02% 41.34 kB 41.35 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.02% 132.88 kB 132.91 kB = 42.31 kB 42.28 kB
facebook-www/ReactDOM-prod.classic.js +0.01% 413.66 kB 413.70 kB = 76.44 kB 76.40 kB
facebook-www/ReactDOM-prod.modern.js +0.01% 402.24 kB 402.29 kB = 74.71 kB 74.71 kB
facebook-www/ReactDOMForked-prod.classic.js +0.01% 413.66 kB 413.70 kB = 76.45 kB 76.41 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against bf891da

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants