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

[Partial Hydration] Attempt hydration at a higher pri first if props/context changes #16352

Merged
merged 4 commits into from
Aug 14, 2019

Conversation

sebmarkbage
Copy link
Collaborator

Builds on top of #16346

I attempted this in an early PR but the approach of using mutation to change fiber tags was broken and lead to bugs. Now that's fixed in #16346 and I can finally rebase this fix.

If we haven't yet hydrated a boundary, we may still get props or context flowing into it. Currently, the delete the content and replace it with a content render in that case. Dropping events and state as a result.

When we're already rendering the future state, it is too late to hydrate since we can't continue rendering down with the new props/context as it wouldn't yield the same result as what was rendered on the server. However, assuming people use immutable data for hydration as they should, then we can solve this problem. We solve it by "going back in time" to an earlier point on the timeline and hydrate using those props/context. After that is done, we then render the new props/context on top of it.

In practice, this just means that we increase the priority of the hydration to slightly higher than the render that otherwise would have to delete the content.

We also mark this as a "bad" loading state which means that we'll also take advantage of our maximum allowed suspense time to wait for any remaining data/code that we'd need to complete the hydration. Meaning that we can do updates to parents before we have the code to the children and it still looks seamless to the user!

let attemptHydrationAtExpirationTime = renderExpirationTime + 1;
suspenseState.retryTime = attemptHydrationAtExpirationTime;
scheduleWork(current, attemptHydrationAtExpirationTime);
// TODO: Early abort this render.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acdlite We don't currently have a way to abort a render while inside the work loop. I attempted something like sebmarkbage@c9c6c0a but I don't think that's the right approach.

This works fine as long as we yield every 5ms anyway but it's not ideal when there are longer work loop times e.g. due to isInputPending or similar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could throw something and then handle the interruption in ReactFiberThrow

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to call markSpawnedWork?

Copy link
Collaborator Author

@sebmarkbage sebmarkbage Aug 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that and I don't think I should call markSpawnedWork here because the hydration isn't actually part of this work that caused this update. The hydration is the work that is scheduled on Idle which isn't really part of this update.

cc @bvaughn

{timeoutMs: 6000}
);
}}
/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To run this fixture you need to turn on enableSuspenseServerRenderer in ReactFeatureFlags.

@sizebot
Copy link

sizebot commented Aug 10, 2019

Warnings
⚠️

Base commit is broken: a29adc9

Generated by 🚫 dangerJS

This might quickly become an already expired time.
This allows the suspense config to kick in and we can wait for much longer
before we're forced to give up on hydrating.
@sebmarkbage sebmarkbage merged commit 6fbe630 into facebook:master Aug 14, 2019
gaearon pushed a commit to gaearon/react that referenced this pull request Aug 14, 2019
…context changes (facebook#16352)

* Test that we can suspend updates while waiting to hydrate

* Attempt hydration at a higher pri first if props/context changes

* Retrying a dehydrated boundary pings at the earliest forced time

This might quickly become an already expired time.

* Mark the render as delayed if we have to retry

This allows the suspense config to kick in and we can wait for much longer
before we're forced to give up on hydrating.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants