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

Add onRecoverableError option to hydrateRoot, createRoot #23207

Merged
merged 8 commits into from
Feb 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
11 changes: 6 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1983,14 +1983,15 @@ describe('ReactDOMFizzServer', () => {
isClient = true;
ReactDOM.hydrateRoot(container, <App />, {
onRecoverableError(error) {
// TODO: We logged a hydration error, but the same error ends up
// being thrown during the fallback to client rendering, too. Maybe
// we should only log if the client render succeeds.
Scheduler.unstable_yieldValue(error.message);
Scheduler.unstable_yieldValue(
'Log recoverable error: ' + error.message,
);
},
});

expect(Scheduler).toFlushAndYield(['Oops!']);
// Because we failed to recover from the error, onRecoverableError
// shouldn't be called.
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual('Oops!');
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,7 @@ describe('ReactDOMServerPartialHydration', () => {
},
});
if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) {
// Hydration error is logged
expect(Scheduler).toFlushAndYield([
'An error occurred during hydration. The server HTML was replaced ' +
'with client content',
]);
Scheduler.unstable_flushAll();
} else {
expect(() => {
Scheduler.unstable_flushAll();
Expand Down Expand Up @@ -309,13 +305,6 @@ describe('ReactDOMServerPartialHydration', () => {
'Component',
'Component',
'Component',

// Hydration mismatch errors are logged.
// TODO: This could get noisy. Is there some way to dedupe?
'An error occurred during hydration. The server HTML was replaced with client content',
'An error occurred during hydration. The server HTML was replaced with client content',
'An error occurred during hydration. The server HTML was replaced with client content',
'An error occurred during hydration. The server HTML was replaced with client content',
]);
jest.runAllTimers();

Expand Down
7 changes: 7 additions & 0 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ import {
resetHydrationState,
getIsHydrating,
hasUnhydratedTailNodes,
queueRecoverableHydrationErrors,
} from './ReactFiberHydrationContext.new';
import {
enableSuspenseCallback,
Expand Down Expand Up @@ -1099,6 +1100,12 @@ function completeWork(
return null;
}
}

// Successfully completed this tree. If this was a forced client render,
// there may have been recoverable errors during first hydration
// attempt. If so, add them to a queue so we can log them in the
// commit phase.
queueRecoverableHydrationErrors();
}

if ((workInProgress.flags & DidCapture) !== NoFlags) {
Expand Down
7 changes: 7 additions & 0 deletions packages/react-reconciler/src/ReactFiberCompleteWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ import {
resetHydrationState,
getIsHydrating,
hasUnhydratedTailNodes,
queueRecoverableHydrationErrors,
} from './ReactFiberHydrationContext.old';
import {
enableSuspenseCallback,
Expand Down Expand Up @@ -1099,6 +1100,12 @@ function completeWork(
return null;
}
}

// Successfully completed this tree. If this was a forced client render,
// there may have been recoverable errors during first hydration
// attempt. If so, add them to a queue so we can log them in the
// commit phase.
queueRecoverableHydrationErrors();
}

if ((workInProgress.flags & DidCapture) !== NoFlags) {
Expand Down
24 changes: 24 additions & 0 deletions packages/react-reconciler/src/ReactFiberHydrationContext.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,17 @@ import {
getSuspendedTreeContext,
restoreSuspendedTreeContext,
} from './ReactFiberTreeContext.new';
import {queueRecoverableErrors} from './ReactFiberWorkLoop.new';

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

// Hydration errors that were thrown inside this boundary
let hydrationErrors: Array<mixed> | null = null;

function warnIfHydrating() {
if (__DEV__) {
if (isHydrating) {
Expand All @@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean {
);
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
return true;
}

Expand All @@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
);
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
if (treeContext !== null) {
restoreSuspendedTreeContext(fiber, treeContext);
}
Expand Down Expand Up @@ -601,10 +607,28 @@ function resetHydrationState(): void {
isHydrating = false;
}

export function queueRecoverableHydrationErrors(): void {
Copy link
Collaborator

Choose a reason for hiding this comment

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

These two methods sound the same. Maybe "apply" or "upstream"...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

upgradeHydrationErrorsToRecoverable?

if (hydrationErrors !== null) {
// Successfully completed a forced client render. The errors that occurred
// during the hydration attempt are now recovered. We will log them in
// commit phase, once the entire tree has finished.
queueRecoverableErrors(hydrationErrors);
hydrationErrors = null;
}
}

function getIsHydrating(): boolean {
return isHydrating;
}

export function queueHydrationError(error: mixed): void {
if (hydrationErrors === null) {
hydrationErrors = [error];
} else {
hydrationErrors.push(error);
}
}

export {
warnIfHydrating,
enterHydrationState,
Expand Down
24 changes: 24 additions & 0 deletions packages/react-reconciler/src/ReactFiberHydrationContext.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,17 @@ import {
getSuspendedTreeContext,
restoreSuspendedTreeContext,
} from './ReactFiberTreeContext.old';
import {queueRecoverableErrors} from './ReactFiberWorkLoop.old';

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

// Hydration errors that were thrown inside this boundary
let hydrationErrors: Array<mixed> | null = null;

function warnIfHydrating() {
if (__DEV__) {
if (isHydrating) {
Expand All @@ -105,6 +109,7 @@ function enterHydrationState(fiber: Fiber): boolean {
);
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
return true;
}

Expand All @@ -121,6 +126,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
);
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
if (treeContext !== null) {
restoreSuspendedTreeContext(fiber, treeContext);
}
Expand Down Expand Up @@ -601,10 +607,28 @@ function resetHydrationState(): void {
isHydrating = false;
}

export function queueRecoverableHydrationErrors(): void {
if (hydrationErrors !== null) {
// Successfully completed a forced client render. The errors that occurred
// during the hydration attempt are now recovered. We will log them in
// commit phase, once the entire tree has finished.
queueRecoverableErrors(hydrationErrors);
hydrationErrors = null;
}
}

function getIsHydrating(): boolean {
return isHydrating;
}

export function queueHydrationError(error: mixed): void {
if (hydrationErrors === null) {
hydrationErrors = [error];
} else {
hydrationErrors.push(error);
}
}

export {
warnIfHydrating,
enterHydrationState,
Expand Down
12 changes: 5 additions & 7 deletions packages/react-reconciler/src/ReactFiberThrow.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
import {
supportsPersistence,
getOffscreenContainerProps,
logRecoverableError,
} from './ReactFiberHostConfig';
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new';
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -80,7 +79,10 @@ import {
mergeLanes,
pickArbitraryLane,
} from './ReactFiberLane.new';
import {getIsHydrating} from './ReactFiberHydrationContext.new';
import {
getIsHydrating,
queueHydrationError,
} from './ReactFiberHydrationContext.new';

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

Expand Down Expand Up @@ -511,11 +513,7 @@ function throwException(

// Even though the user may not be affected by this error, we should
// still log it so it can be fixed.
// TODO: For now, we only log errors that occur during hydration, but we
// probably want to log any error that is recovered from without
// triggering an error boundary — or maybe even those, too. Need to
// figure out the right API.
logRecoverableError(root.errorLoggingConfig, value);
queueHydrationError(value);
return;
}
} else {
Expand Down
12 changes: 5 additions & 7 deletions packages/react-reconciler/src/ReactFiberThrow.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
import {
supportsPersistence,
getOffscreenContainerProps,
logRecoverableError,
} from './ReactFiberHostConfig';
import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.old';
import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -80,7 +79,10 @@ import {
mergeLanes,
pickArbitraryLane,
} from './ReactFiberLane.old';
import {getIsHydrating} from './ReactFiberHydrationContext.old';
import {
getIsHydrating,
queueHydrationError,
} from './ReactFiberHydrationContext.old';

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

Expand Down Expand Up @@ -511,11 +513,7 @@ function throwException(

// Even though the user may not be affected by this error, we should
// still log it so it can be fixed.
// TODO: For now, we only log errors that occur during hydration, but we
// probably want to log any error that is recovered from without
// triggering an error boundary — or maybe even those, too. Need to
// figure out the right API.
logRecoverableError(root.errorLoggingConfig, value);
queueHydrationError(value);
return;
}
} else {
Expand Down
20 changes: 12 additions & 8 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -909,14 +909,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
// The errors from the failed first attempt have been recovered. Add
// them to the collection of recoverable errors. We'll log them in the
// commit phase.
if (workInProgressRootConcurrentErrors === null) {
workInProgressRootRecoverableErrors = errorsFromFirstAttempt;
} else {
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
null,
errorsFromFirstAttempt,
);
}
queueRecoverableErrors(errorsFromFirstAttempt);
}
} else {
// The UI failed to recover.
Expand All @@ -927,6 +920,17 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
return exitStatus;
}

export function queueRecoverableErrors(errors: Array<mixed>) {
if (workInProgressRootConcurrentErrors === null) {
workInProgressRootRecoverableErrors = errors;
} else {
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
null,
errors,
);
}
}

function finishConcurrentRender(root, exitStatus, lanes) {
switch (exitStatus) {
case RootIncomplete:
Expand Down
20 changes: 12 additions & 8 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -909,14 +909,7 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
// The errors from the failed first attempt have been recovered. Add
// them to the collection of recoverable errors. We'll log them in the
// commit phase.
if (workInProgressRootConcurrentErrors === null) {
workInProgressRootRecoverableErrors = errorsFromFirstAttempt;
} else {
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
null,
errorsFromFirstAttempt,
);
}
queueRecoverableErrors(errorsFromFirstAttempt);
}
} else {
// The UI failed to recover.
Expand All @@ -927,6 +920,17 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
return exitStatus;
}

export function queueRecoverableErrors(errors: Array<mixed>) {
if (workInProgressRootConcurrentErrors === null) {
workInProgressRootRecoverableErrors = errors;
} else {
workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
null,
errors,
);
}
}

function finishConcurrentRender(root, exitStatus, lanes) {
switch (exitStatus) {
case RootIncomplete:
Expand Down