Skip to content

Commit 408c5f8

Browse files
acdliteacdlite
committed
Only log hydration error if client render succeeds
Similar to previous step. When an error occurs during hydration, we only want to log it if falling back to client rendering _succeeds_. If client rendering fails, the error will get reported to the nearest error boundary, so there's no need for a duplicate log. To implement this, I added a list of errors to the hydration context. If the Suspense boundary successfully completes, they are added to the main recoverable errors queue (the one I added in the previous step.)
1 parent 667e431 commit 408c5f8

10 files changed

+103
-47
lines changed

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1983,14 +1983,15 @@ describe('ReactDOMFizzServer', () => {
19831983
isClient = true;
19841984
ReactDOM.hydrateRoot(container, <App />, {
19851985
onRecoverableError(error) {
1986-
// TODO: We logged a hydration error, but the same error ends up
1987-
// being thrown during the fallback to client rendering, too. Maybe
1988-
// we should only log if the client render succeeds.
1989-
Scheduler.unstable_yieldValue(error.message);
1986+
Scheduler.unstable_yieldValue(
1987+
'Log recoverable error: ' + error.message,
1988+
);
19901989
},
19911990
});
19921991

1993-
expect(Scheduler).toFlushAndYield(['Oops!']);
1992+
// Because we failed to recover from the error, onRecoverableError
1993+
// shouldn't be called.
1994+
expect(Scheduler).toFlushAndYield([]);
19941995
expect(getVisibleChildren(container)).toEqual('Oops!');
19951996
},
19961997
);

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,7 @@ describe('ReactDOMServerPartialHydration', () => {
214214
},
215215
});
216216
if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) {
217-
// Hydration error is logged
218-
expect(Scheduler).toFlushAndYield([
219-
'An error occurred during hydration. The server HTML was replaced ' +
220-
'with client content',
221-
]);
217+
Scheduler.unstable_flushAll();
222218
} else {
223219
expect(() => {
224220
Scheduler.unstable_flushAll();
@@ -309,13 +305,6 @@ describe('ReactDOMServerPartialHydration', () => {
309305
'Component',
310306
'Component',
311307
'Component',
312-
313-
// Hydration mismatch errors are logged.
314-
// TODO: This could get noisy. Is there some way to dedupe?
315-
'An error occurred during hydration. The server HTML was replaced with client content',
316-
'An error occurred during hydration. The server HTML was replaced with client content',
317-
'An error occurred during hydration. The server HTML was replaced with client content',
318-
'An error occurred during hydration. The server HTML was replaced with client content',
319308
]);
320309
jest.runAllTimers();
321310

‎packages/react-reconciler/src/ReactFiberCompleteWork.new.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ import {
131131
resetHydrationState,
132132
getIsHydrating,
133133
hasUnhydratedTailNodes,
134+
queueRecoverableHydrationErrors,
134135
} from './ReactFiberHydrationContext.new';
135136
import {
136137
enableSuspenseCallback,
@@ -1099,6 +1100,12 @@ function completeWork(
10991100
return null;
11001101
}
11011102
}
1103+
1104+
// Successfully completed this tree. If this was a forced client render,
1105+
// there may have been recoverable errors during first hydration
1106+
// attempt. If so, add them to a queue so we can log them in the
1107+
// commit phase.
1108+
queueRecoverableHydrationErrors(workInProgress);
11021109
}
11031110

11041111
if ((workInProgress.flags & DidCapture) !== NoFlags) {

‎packages/react-reconciler/src/ReactFiberCompleteWork.old.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ import {
131131
resetHydrationState,
132132
getIsHydrating,
133133
hasUnhydratedTailNodes,
134+
queueRecoverableHydrationErrors,
134135
} from './ReactFiberHydrationContext.old';
135136
import {
136137
enableSuspenseCallback,
@@ -1099,6 +1100,12 @@ function completeWork(
10991100
return null;
11001101
}
11011102
}
1103+
1104+
// Successfully completed this tree. If this was a forced client render,
1105+
// there may have been recoverable errors during first hydration
1106+
// attempt. If so, add them to a queue so we can log them in the
1107+
// commit phase.
1108+
queueRecoverableHydrationErrors(workInProgress);
11021109
}
11031110

11041111
if ((workInProgress.flags & DidCapture) !== NoFlags) {

‎packages/react-reconciler/src/ReactFiberHydrationContext.new.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,17 @@ import {
7777
getSuspendedTreeContext,
7878
restoreSuspendedTreeContext,
7979
} from './ReactFiberTreeContext.new';
80+
import {queueRecoverableErrors} from './ReactFiberWorkLoop.new';
8081

8182
// The deepest Fiber on the stack involved in a hydration context.
8283
// This may have been an insertion or a hydration.
8384
let hydrationParentFiber: null | Fiber = null;
8485
let nextHydratableInstance: null | HydratableInstance = null;
8586
let isHydrating: boolean = false;
8687

88+
// Hydration errors that were thrown inside this boundary
89+
let hydrationErrors: Array<mixed> | null = null;
90+
8791
function warnIfHydrating() {
8892
if (__DEV__) {
8993
if (isHydrating) {
@@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean {
105109
);
106110
hydrationParentFiber = fiber;
107111
isHydrating = true;
112+
hydrationErrors = null;
108113
return true;
109114
}
110115

@@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
121126
);
122127
hydrationParentFiber = fiber;
123128
isHydrating = true;
129+
hydrationErrors = null;
124130
if (treeContext !== null) {
125131
restoreSuspendedTreeContext(fiber, treeContext);
126132
}
@@ -601,10 +607,28 @@ function resetHydrationState(): void {
601607
isHydrating = false;
602608
}
603609

610+
export function queueRecoverableHydrationErrors(): void {
611+
if (hydrationErrors !== null) {
612+
// Successfully completed a forced client render. The errors that occurred
613+
// during the hydration attempt are now recovered. We will log them in
614+
// commit phase, once the entire tree has finished.
615+
queueRecoverableErrors(hydrationErrors);
616+
hydrationErrors = null;
617+
}
618+
}
619+
604620
function getIsHydrating(): boolean {
605621
return isHydrating;
606622
}
607623

624+
export function queueHydrationError(error: mixed): void {
625+
if (hydrationErrors === null) {
626+
hydrationErrors = [error];
627+
} else {
628+
hydrationErrors.push(error);
629+
}
630+
}
631+
608632
export {
609633
warnIfHydrating,
610634
enterHydrationState,

‎packages/react-reconciler/src/ReactFiberHydrationContext.old.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,17 @@ import {
7777
getSuspendedTreeContext,
7878
restoreSuspendedTreeContext,
7979
} from './ReactFiberTreeContext.old';
80+
import {queueRecoverableErrors} from './ReactFiberWorkLoop.old';
8081

8182
// The deepest Fiber on the stack involved in a hydration context.
8283
// This may have been an insertion or a hydration.
8384
let hydrationParentFiber: null | Fiber = null;
8485
let nextHydratableInstance: null | HydratableInstance = null;
8586
let isHydrating: boolean = false;
8687

88+
// Hydration errors that were thrown inside this boundary
89+
let hydrationErrors: Array<mixed> | null = null;
90+
8791
function warnIfHydrating() {
8892
if (__DEV__) {
8993
if (isHydrating) {
@@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean {
105109
);
106110
hydrationParentFiber = fiber;
107111
isHydrating = true;
112+
hydrationErrors = null;
108113
return true;
109114
}
110115

@@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
121126
);
122127
hydrationParentFiber = fiber;
123128
isHydrating = true;
129+
hydrationErrors = null;
124130
if (treeContext !== null) {
125131
restoreSuspendedTreeContext(fiber, treeContext);
126132
}
@@ -601,10 +607,28 @@ function resetHydrationState(): void {
601607
isHydrating = false;
602608
}
603609

610+
export function queueRecoverableHydrationErrors(): void {
611+
if (hydrationErrors !== null) {
612+
// Successfully completed a forced client render. The errors that occurred
613+
// during the hydration attempt are now recovered. We will log them in
614+
// commit phase, once the entire tree has finished.
615+
queueRecoverableErrors(hydrationErrors);
616+
hydrationErrors = null;
617+
}
618+
}
619+
604620
function getIsHydrating(): boolean {
605621
return isHydrating;
606622
}
607623

624+
export function queueHydrationError(error: mixed): void {
625+
if (hydrationErrors === null) {
626+
hydrationErrors = [error];
627+
} else {
628+
hydrationErrors.push(error);
629+
}
630+
}
631+
608632
export {
609633
warnIfHydrating,
610634
enterHydrationState,

‎packages/react-reconciler/src/ReactFiberThrow.new.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import {
3737
import {
3838
supportsPersistence,
3939
getOffscreenContainerProps,
40-
logRecoverableError,
4140
} from './ReactFiberHostConfig';
4241
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
4342
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -80,7 +79,10 @@ import {
8079
mergeLanes,
8180
pickArbitraryLane,
8281
} from './ReactFiberLane.new';
83-
import {getIsHydrating} from './ReactFiberHydrationContext.new';
82+
import {
83+
getIsHydrating,
84+
queueHydrationError,
85+
} from './ReactFiberHydrationContext.new';
8486

8587
const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
8688

@@ -511,11 +513,7 @@ function throwException(
511513

512514
// Even though the user may not be affected by this error, we should
513515
// still log it so it can be fixed.
514-
// TODO: For now, we only log errors that occur during hydration, but we
515-
// probably want to log any error that is recovered from without
516-
// triggering an error boundary — or maybe even those, too. Need to
517-
// figure out the right API.
518-
logRecoverableError(root.errorLoggingConfig, value);
516+
queueHydrationError(value);
519517
return;
520518
}
521519
} else {

‎packages/react-reconciler/src/ReactFiberThrow.old.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import {
3737
import {
3838
supportsPersistence,
3939
getOffscreenContainerProps,
40-
logRecoverableError,
4140
} from './ReactFiberHostConfig';
4241
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
4342
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
@@ -80,7 +79,10 @@ import {
8079
mergeLanes,
8180
pickArbitraryLane,
8281
} from './ReactFiberLane.old';
83-
import {getIsHydrating} from './ReactFiberHydrationContext.old';
82+
import {
83+
getIsHydrating,
84+
queueHydrationError,
85+
} from './ReactFiberHydrationContext.old';
8486

8587
const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
8688

@@ -511,11 +513,7 @@ function throwException(
511513

512514
// Even though the user may not be affected by this error, we should
513515
// still log it so it can be fixed.
514-
// TODO: For now, we only log errors that occur during hydration, but we
515-
// probably want to log any error that is recovered from without
516-
// triggering an error boundary — or maybe even those, too. Need to
517-
// figure out the right API.
518-
logRecoverableError(root.errorLoggingConfig, value);
516+
queueHydrationError(value);
519517
return;
520518
}
521519
} else {

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -909,14 +909,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
909909
// The errors from the failed first attempt have been recovered. Add
910910
// them to the collection of recoverable errors. We'll log them in the
911911
// commit phase.
912-
if (workInProgressRootConcurrentErrors === null) {
913-
workInProgressRootRecoverableErrors = errorsFromFirstAttempt;
914-
} else {
915-
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
916-
null,
917-
errorsFromFirstAttempt,
918-
);
919-
}
912+
queueRecoverableErrors(errorsFromFirstAttempt);
920913
}
921914
} else {
922915
// The UI failed to recover.
@@ -927,6 +920,17 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
927920
return exitStatus;
928921
}
929922

923+
export function queueRecoverableErrors(errors: Array<mixed>) {
924+
if (workInProgressRootConcurrentErrors === null) {
925+
workInProgressRootRecoverableErrors = errors;
926+
} else {
927+
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
928+
null,
929+
errors,
930+
);
931+
}
932+
}
933+
930934
function finishConcurrentRender(root, exitStatus, lanes) {
931935
switch (exitStatus) {
932936
case RootIncomplete:

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -909,14 +909,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
909909
// The errors from the failed first attempt have been recovered. Add
910910
// them to the collection of recoverable errors. We'll log them in the
911911
// commit phase.
912-
if (workInProgressRootConcurrentErrors === null) {
913-
workInProgressRootRecoverableErrors = errorsFromFirstAttempt;
914-
} else {
915-
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
916-
null,
917-
errorsFromFirstAttempt,
918-
);
919-
}
912+
queueRecoverableErrors(errorsFromFirstAttempt);
920913
}
921914
} else {
922915
// The UI failed to recover.
@@ -927,6 +920,17 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
927920
return exitStatus;
928921
}
929922

923+
export function queueRecoverableErrors(errors: Array<mixed>) {
924+
if (workInProgressRootConcurrentErrors === null) {
925+
workInProgressRootRecoverableErrors = errors;
926+
} else {
927+
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
928+
null,
929+
errors,
930+
);
931+
}
932+
}
933+
930934
function finishConcurrentRender(root, exitStatus, lanes) {
931935
switch (exitStatus) {
932936
case RootIncomplete:

0 commit comments

Comments
 (0)