Skip to content

[Hydration] Fallback to client render if server rendered extra nodes #23176

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 1, 2022
Merged

[Hydration] Fallback to client render if server rendered extra nodes #23176

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a6a05e0
rename
salazarm Jan 31, 2022
a902b50
rename
salazarm Jan 31, 2022
f8b927c
replace-fork
salazarm Jan 31, 2022
52ac465
rename
salazarm Jan 31, 2022
3ae3605
warn in a loop
salazarm Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
rename
  • Loading branch information
@salazarm
salazarm committed Jan 31, 2022
commit a902b50c066254e3c3f4ddfe942a9e01dd0285fb
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ import {
prepareToHydrateHostInstance,
prepareToHydrateHostTextInstance,
prepareToHydrateHostSuspenseInstance,
warnDeleteNextHydratableInstance,
warnUnhydratedNextInstance,
popHydrationState,
resetHydrationState,
getIsHydrating,
Expand Down Expand Up @@ -1035,7 +1035,7 @@ function completeWork(
(workInProgress.mode & ConcurrentMode) !== NoMode &&
(workInProgress.flags & DidCapture) === NoFlags
) {
warnDeleteNextHydratableInstance(workInProgress);
warnUnhydratedNextInstance(workInProgress);
resetHydrationState();
workInProgress.flags |=
ForceClientRender | Incomplete | ShouldCapture;
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberCompleteWork.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ import {
prepareToHydrateHostInstance,
prepareToHydrateHostTextInstance,
prepareToHydrateHostSuspenseInstance,
warnDeleteNextHydratableInstance,
warnUnhydratedNextInstance,
popHydrationState,
resetHydrationState,
getIsHydrating,
Expand Down Expand Up @@ -1034,7 +1034,7 @@ function completeWork(
hasUnhydratedTailNodes() &&
(workInProgress.flags & DidCapture) === NoFlags
) {
warnDeleteNextHydratableInstance(workInProgress);
warnUnhydratedNextInstance(workInProgress);
resetHydrationState();
workInProgress.flags |=
ForceClientRender | Incomplete | ShouldCapture;
Expand Down
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/ReactFiberHydrationContext.new.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ function popHydrationState(fiber: Fiber): boolean {
) {
let nextInstance = nextHydratableInstance;
if (nextInstance) {
warnDeleteNextHydratableInstance(fiber);
warnUnhydratedNextInstance(fiber);
throwOnHydrationMismatchIfConcurrentMode(fiber);
while (nextInstance) {
deleteHydratableInstance(fiber, nextInstance);
Expand All @@ -576,7 +576,7 @@ function hasUnhydratedTailNodes() {
return isHydrating && nextHydratableInstance !== null;
}

function warnDeleteNextHydratableInstance(fiber: Fiber) {
function warnUnhydratedNextInstance(fiber: Fiber) {
if (nextHydratableInstance) {
warnUnhydratedInstance(fiber, nextHydratableInstance);
}
Expand Down Expand Up @@ -607,6 +607,6 @@ export {
prepareToHydrateHostTextInstance,
prepareToHydrateHostSuspenseInstance,
popHydrationState,
hasMore,
warnDeleteNextHydratableInstance,
hasUnhydratedTailNodes,
warnUnhydratedNextInstance,
};
6 changes: 3 additions & 3 deletions packages/react-reconciler/src/ReactFiberHydrationContext.old.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ function popHydrationState(fiber: Fiber): boolean {
) {
let nextInstance = nextHydratableInstance;
if (nextInstance) {
warnDeleteNextHydratableInstance(fiber);
warnUnhydratedNextInstance(fiber);
throwOnHydrationMismatchIfConcurrentMode(fiber);
while (nextInstance) {
deleteHydratableInstance(fiber, nextInstance);
Expand All @@ -576,7 +576,7 @@ function hasUnhydratedTailNodes() {
return isHydrating && nextHydratableInstance !== null;
}

function warnDeleteNextHydratableInstance(fiber: Fiber) {
function warnUnhydratedNextInstance(fiber: Fiber) {
Copy link
Collaborator

@sebmarkbage sebmarkbage Jan 31, 2022

Choose a reason for hiding this comment

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

The "NextInstance" part is an implementation detail of this file. Maybe go with the other terminology you had. "warnUnhydratedTailNodes"

Copy link
Contributor Author

@salazarm salazarm Jan 31, 2022

Choose a reason for hiding this comment

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

Sure, one thing though I only warn for the nextHydratableInstance instead of warning for them in a loop because I know the implementation uses a flag to only warn the first time anyways. Though with the error logging API maybe we will want it to there every time so maybe I should add the loop here to warn for all for all of the tail nodes and not just the next one.

Similarly above we throw after the first one is warned for so we don't go through through the warning function for all of the tail nodes there either.

Copy link
Collaborator

@sebmarkbage sebmarkbage Jan 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The reason it logged them in a loop is so that the implementation in the host config could batch them up and build a tree that shows all the errors in one message. Without that the batching it's too noisy. We never landed the PR that did that but we should.

So this should probably call it in a loop too.

if (nextHydratableInstance) {
warnUnhydratedInstance(fiber, nextHydratableInstance);
}
Expand Down Expand Up @@ -608,5 +608,5 @@ export {
prepareToHydrateHostSuspenseInstance,
popHydrationState,
hasUnhydratedTailNodes,
warnDeleteNextHydratableInstance,
warnUnhydratedNextInstance,
};