[Partial Hydration] Add "hydrationOptions" behind the enableSuspenseCallback flag #16434
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: 6f86294...16c34b7 react-art
react-native-renderer
react-dom
react-test-renderer
react-reconciler
Generated by 🚫 dangerJS |
sebmarkbage
force-pushed
the
hydrationcallbacks
branch
from
958c8f6 to
d4320e0
Compare
gaearon
reviewed
Aug 19, 2019
This gets invoked when a boundary is either hydrated or if it is deleted because it updated or got deleted before it mounted.
sebmarkbage
force-pushed
the
hydrationcallbacks
branch
from
d4320e0 to
16c34b7
Compare
gaearon
approved these changes
Aug 19, 2019
| @@ -644,7 +649,8 @@ function hideOrUnhideAllChildren(finishedWork, isHidden) { | |||
| } | |||
| } else if ( | |||
| node.tag === SuspenseComponent && | |||
| node.memoizedState !== null | |||
| node.memoizedState !== null && | |||
| node.memoizedState.dehydrated === null | |||
There was a problem hiding this comment.
What is this about? I don't see you setting it in this PR.
Is this a behavior change? Wasn't the feature logging-only? Or was this always null anyway?
There was a problem hiding this comment.
This probably should've been in #16346 but it was not observable because we weren't scheduling an Update effect in this scenario. Now that we do, we need to differentiate what purpose the Update effect has.
| const hydrationCallbacks = finishedRoot.hydrationCallbacks; | ||
| if (hydrationCallbacks !== null) { | ||
| const onDeleted = hydrationCallbacks.onDeleted; | ||
| if (onDeleted) { |
There was a problem hiding this comment.
This is an optional property. In some other cases we check typeof ... === 'function' when it's a prop, but in some cases we check truthiness. Not sure which one to be consistent with so I went with the one that takes fewer bytes.
| @@ -859,6 +859,10 @@ function completeWork( | |||
| if ((workInProgress.effectTag & DidCapture) === NoEffect) { | |||
| // This boundary did not suspend so it's now hydrated and unsuspended. | |||
| workInProgress.memoizedState = null; | |||
| if (enableSuspenseCallback) { | |||
| // Notify the callback. | |||
| workInProgress.effectTag |= Update; | |||
There was a problem hiding this comment.
Seems a bit worrying to have a generic effect tag like this behind a flag since this may significantly fork execution path wherever we check for Update.
There was a problem hiding this comment.
Yea, that's a fair concern. I'm not very confident that this will be a lasting API form though so wouldn't want to do a big refactor yet nor should be pay the cost if it's not needed.
The event replaying might need this flag too and that might move out to a shared path sooner.
| @@ -91,6 +100,7 @@ type ProfilingOnlyFiberRootProperties = {| | |||
| export type FiberRoot = { | |||
There was a problem hiding this comment.
While we're here, should this have been exact?
There was a problem hiding this comment.
I don't think Flow supported the spreading as exact but arguably we shouldn't do the spreading neither. We don't for Fiber.
NMinhNguyen
referenced
this pull request
in enzymejs/react-shallow-renderer
Jan 29, 2020
This gets invoked when a boundary is either hydrated or if it is deleted because it updated or got deleted before it mounted.
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 gets invoked when a boundary is either hydrated or if it is deleted because its props/context updated before getting hydrated or a parent got deleted before it mounted.
If there are nested boundaries in the tree that we haven't gotten to yet and the parent gets deleted, then no callback is fired. Only the top one.
If we have a hydration error that causes us to delete the parent node, then there also isn't a callback. That could however be a different callback we could add.