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

Allow updating dehydrated root at lower priority without forcing client render #24080

Merged
merged 1 commit into from
Mar 12, 2022

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Mar 12, 2022

Currently, if a root is updated before the shell has finished hydrating (for example, due to a top-level navigation), we immediately revert to client rendering. This is rare because the root is expected is finish quickly, but not exceedingly rare because the root may be suspended.

This adds support for updating the root without forcing a client render as long as the update has lower priority than the initial hydration, i.e. if the update is wrapped in startTransition.

To implement this, I had to do some refactoring. The main idea here is to make it closer to how we implement hydration in Suspense boundaries:

  • I moved isDehydrated from the shared FiberRoot object to the HostRoot's state object.
  • In the begin phase, I check if the root has received an update by comparing the new children to the initial children. If they are different, we revert to client rendering, and set isDehydrated to false using a derived state update (a la getDerivedStateFromProps).
  • There are a few places where we used to set root.isDehydrated to false as a way to force a client render. Instead, I set the ForceClientRender flag on the root work-in-progress fiber.
  • Whenever we fall back to client rendering, I log a recoverable error.

The overall code structure is almost identical to the corresponding logic for Suspense components.

The reason this works is because if the update has lower priority than the initial hydration, it won't be processed during the hydration render, so the children will be the same.

We can go even further and allow updates at higher priority (though not sync) by implementing selective hydration at the root, like we do for Suspense boundaries: interrupt the current render, attempt hydration at slightly higher priority than the update, then continue rendering the update. I haven't implemented this yet, but I've structured the code in anticipation of adding this later.

I already made this change for the concurrent root API in facebook#23309. This
does the same thing for the legacy API.

Doesn't change any behavior, but I will use this in the next steps.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 12, 2022
@sizebot
Copy link

sizebot commented Mar 12, 2022

Comparing: c8e4789...c8e4789

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 = 131.25 kB 131.25 kB = 41.99 kB 41.99 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 136.32 kB 136.32 kB = 43.49 kB 43.49 kB
facebook-www/ReactDOM-prod.classic.js = 436.79 kB 436.79 kB = 79.96 kB 79.96 kB
facebook-www/ReactDOM-prod.modern.js = 421.95 kB 421.95 kB = 77.77 kB 77.77 kB
facebook-www/ReactDOMForked-prod.classic.js = 436.79 kB 436.79 kB = 79.96 kB 79.96 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against c8e4789

@acdlite acdlite force-pushed the allow-root-updates-lower-pri branch from 02b65fd to c8e4789 Compare March 12, 2022 02:06
@acdlite acdlite merged commit c8e4789 into facebook:main Mar 12, 2022
@acdlite
Copy link
Collaborator Author

acdlite commented Mar 12, 2022

This was not meant to be merged yet... I think GitHub glitched out when I pushed a commit to this PR branch that I had already pushed previously (I did that because I wanted to run CI on an earlier commit in the sequence). I'll reopen.

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