Refactor Partial Hydration #16346
Merged
Refactor Partial Hydration #16346
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
ReactDOM: size: 0.0%, gzip: 0.0% Details of bundled changes.Comparing: c203471...3243916 react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
Generated by 🚫 dangerJS |
acdlite
reviewed
Aug 12, 2019
acdlite
reviewed
Aug 12, 2019
scripts/error-codes/codes.json
Outdated
| @@ -338,5 +338,7 @@ | |||
| "337": "An invalid event responder was provided to host component", | |||
| "338": "ReactDOMServer does not yet support the fundamental API.", | |||
| "339": "An invalid value was used as an event responder. Expect one or many event responders created via React.unstable_createResponer().", | |||
| "340": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponer()." | |||
| "340": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponer().", | |||
There was a problem hiding this comment.
@trueadm Noticed a typo here. Should be unstable_useResponder()
There was a problem hiding this comment.
Oh yeah, sorry. I mean I can do it too :D
acdlite
approved these changes
Aug 12, 2019
bvaughn
reviewed
Aug 12, 2019
| if (current === null || current.memoizedState !== null) { | ||
| if ( | ||
| current === null || | ||
| (current.memoizedState: null | SuspenseState) !== null |
There was a problem hiding this comment.
It's so that it becomes greppable.
Once we add types for fields like memoizedState it will also be used to ensure that those types are correct, as we make that change.
There was a problem hiding this comment.
The type SuspenseState should probably be nullable by itself since it has meaning and would be a variant if we had those.
We now store the comment node on SuspenseState instead and that indicates that this SuspenseComponent is still dehydrated. We also store a child but that is only used to represent the DOM node for deletions and getNextHostSibling.
Forked based on SuspenseState.dehydrated instead.
We can now simplify the logic for retrying dehydrated boundaries without hydrating. This is becomes simply a reconciliation against the dehydrated fragment which gets deleted, and the new children gets inserted.
Instead we use the regular Suspense path. To save code, we attach retry listeners in the commit phase even though technically we don't have to.
I think this is right...?
The DidCapture flag isn't used consistently in the same way. We need further refactor for this.
If we remove the dehydration status in the first pass and then do a second pass because we suspended, then we need to continue as if it didn't previously suspend. Since there is no fragment child etc. However, we must readd the deletion.
sebmarkbage
force-pushed
the
dehydratedrefactor
branch
from
b708aaa to
a108e3f
Compare
This now doesn't represent a suspense boundary itself. Its parent does. This Fiber represents the fragment around the dehydrated content.
It does a two pass render that client renders the content.
Avoids the temporary mutable variables. I kept losing track of them.
Placing it in the type since that's the central point as opposed to spread out.
sebmarkbage
force-pushed
the
dehydratedrefactor
branch
from
a108e3f to
3243916
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a pretty invasive refactor of the SuspenseComponent and DehydratedSuspenseComponent.
The primary purpose of this refactor is to avoid the hacky "upgrade" and "downgrade" by mutating the tag.
In the new model, a boundary is always represented by the same SuspenseComponent fiber. Inside it I store a "DehydratedFragment". The inner fiber represents the dehydrated nodes in the tree. This can be used to delete the whole thing or insert before it.
I also switched legacy mode to always client-render the boundary content since we can't hydrate partially in legacy mode. It also warns.
Otherwise, this (hopefully) shouldn't have an semantic differences.