Skip to content

Commit

Permalink
[RFC] Add onHydrationError option to hydrateRoot
Browse files Browse the repository at this point in the history
This is not the final API but I'm pushing it for discussion purposes.

When an error is thrown during hydration, we fallback to client
rendering, without triggering an error boundary. This is good because,
in many cases, the UI will recover and the user won't even notice that
something has gone wrong behind the scenes.

However, we shouldn't recover from these errors silently, because the
underlying cause might be pretty serious. Server-client mismatches are
not supposed to happen, even if UI doesn't break from the users
perspective. Ignoring them could lead to worse problems later. De-opting
from server to client rendering could also be a significant performance
regression, depending on the scope of the UI it affects.

So we need a way to log when hydration errors occur.

This adds a new option for `hydrateRoot` called `onHydrationError`. It's
symmetrical to the server renderer's `onError` option, and serves the
same purpose.

When no option is provided, the default behavior is to schedule a
browser task and rethrow the error. This will trigger the normal browser
behavior for errors, including dispatching an error event. If the app
already has error monitoring, this likely will just work as expected
without additional configuration.

However, we can also expose additional metadata about these errors, like
which Suspense boundaries were affected by the de-opt to client
rendering. (I have not exposed any metadata in this commit; API needs
more design work.)

There are other situations besides hydration where we recover from an
error without surfacing it to the user, or notifying an error boundary.
For example, if an error occurs during a concurrent render, it could be
due to a data race, so we try again synchronously in case that fixes it.
We should probably expose a way to log these types of errors, too. (Also
not implemented in this commit.)
  • Loading branch information
acdlite committed Jan 28, 2022
1 parent 13036bf commit 6af98f4
Show file tree
Hide file tree
Showing 22 changed files with 185 additions and 12 deletions.
4 changes: 4 additions & 0 deletions packages/react-art/src/ReactARTHostConfig.js
Expand Up @@ -451,3 +451,7 @@ export function preparePortalMount(portalInstance: any): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logHydrationError(config, error) {
// noop
}
31 changes: 25 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Expand Up @@ -1897,9 +1897,15 @@ describe('ReactDOMFizzServer', () => {
// Hydrate the tree. Child will throw during hydration, but not when it
// falls back to client rendering.
isClient = true;
ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onHydrationError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});

expect(Scheduler).toFlushAndYield(['Yay!']);
// An error logged but instead of surfacing it to the UI, we switched
// to client rendering.
expect(Scheduler).toFlushAndYield(['Yay!', 'Hydration error']);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
Expand Down Expand Up @@ -1975,9 +1981,16 @@ describe('ReactDOMFizzServer', () => {

// Hydrate the tree. Child will throw during render.
isClient = true;
ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onHydrationError(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);
},
});

expect(Scheduler).toFlushAndYield([]);
expect(Scheduler).toFlushAndYield(['Oops!']);
expect(getVisibleChildren(container)).toEqual('Oops!');
},
);
Expand Down Expand Up @@ -2049,9 +2062,15 @@ describe('ReactDOMFizzServer', () => {
// Hydrate the tree. Child will throw during hydration, but not when it
// falls back to client rendering.
isClient = true;
ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onHydrationError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});

expect(Scheduler).toFlushAndYield([]);
// An error logged but instead of surfacing it to the UI, we switched
// to client rendering.
expect(Scheduler).toFlushAndYield(['Hydration error']);
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
Expand Down
Expand Up @@ -208,9 +208,17 @@ describe('ReactDOMServerPartialHydration', () => {
// On the client we don't have all data yet but we want to start
// hydrating anyway.
suspend = true;
ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onHydrationError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});
if (gate(flags => flags.enableClientRenderFallbackOnHydrationMismatch)) {
Scheduler.unstable_flushAll();
// Hydration error is logged
expect(Scheduler).toFlushAndYield([
'An error occurred during hydration. The server HTML was replaced ' +
'with client content',
]);
} else {
expect(() => {
Scheduler.unstable_flushAll();
Expand Down Expand Up @@ -290,13 +298,24 @@ describe('ReactDOMServerPartialHydration', () => {
suspend = true;
client = true;

ReactDOM.hydrateRoot(container, <App />);
ReactDOM.hydrateRoot(container, <App />, {
onHydrationError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});
expect(Scheduler).toFlushAndYield([
'Suspend',
'Component',
'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 All @@ -316,12 +335,16 @@ describe('ReactDOMServerPartialHydration', () => {
'Component',
'Component',
'Component',

// second pass as client render
'Hello',
'Component',
'Component',
'Component',
'Component',

// Hydration mismatch is logged
'An error occurred during hydration. The server HTML was replaced with client content',
]);

// Client rendered - suspense comment nodes removed
Expand Down
24 changes: 24 additions & 0 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Expand Up @@ -70,6 +70,7 @@ import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';

import {DefaultEventPriority} from 'react-reconciler/src/ReactEventPriorities';
import {scheduleCallback, IdlePriority} from 'react-reconciler/src/Scheduler';

export type Type = string;
export type Props = {
Expand Down Expand Up @@ -123,6 +124,10 @@ export type TimeoutHandle = TimeoutID;
export type NoTimeout = -1;
export type RendererInspectionConfig = $ReadOnly<{||}>;

// Right now this is a single callback, but could be multiple in the in the
// future.
export type ErrorLoggingConfig = null | ((error: mixed) => void);

type SelectionInformation = {|
focusedElem: null | HTMLElement,
selectionRange: mixed,
Expand Down Expand Up @@ -374,6 +379,25 @@ export function getCurrentEventPriority(): * {
return getEventPriority(currentEvent.type);
}

export function logHydrationError(
config: ErrorLoggingConfig,
error: mixed,
): void {
const onHydrationError = config;
if (onHydrationError !== null) {
// Schedule a callback to invoke the user-provided logging function.
scheduleCallback(IdlePriority, () => {
onHydrationError(error);
});
} else {
// Default behavior is to rethrow the error in a separate task. This will
// trigger a browser error event.
scheduleCallback(IdlePriority, () => {
throw error;
});
}
}

export const isPrimaryRenderer = true;
export const warnsIfNotActing = true;
// This initialization code may run even on server environments
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/client/ReactDOMLegacy.js
Expand Up @@ -122,6 +122,7 @@ function legacyCreateRootFromDOMContainer(
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identifierPrefix
null,
);
markContainerAsRoot(root.current, container);

Expand Down
7 changes: 7 additions & 0 deletions packages/react-dom/src/client/ReactDOMRoot.js
Expand Up @@ -36,6 +36,7 @@ export type HydrateRootOptions = {
unstable_strictMode?: boolean,
unstable_concurrentUpdatesByDefault?: boolean,
identifierPrefix?: string,
onHydrationError?: (error: mixed) => void,
...
};

Expand Down Expand Up @@ -173,6 +174,7 @@ export function createRoot(
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
null,
);
markContainerAsRoot(root.current, container);

Expand Down Expand Up @@ -213,6 +215,7 @@ export function hydrateRoot(
let isStrictMode = false;
let concurrentUpdatesByDefaultOverride = false;
let identifierPrefix = '';
let onHydrationError = null;
if (options !== null && options !== undefined) {
if (options.unstable_strictMode === true) {
isStrictMode = true;
Expand All @@ -226,6 +229,9 @@ export function hydrateRoot(
if (options.identifierPrefix !== undefined) {
identifierPrefix = options.identifierPrefix;
}
if (options.onHydrationError !== undefined) {
onHydrationError = options.onHydrationError;
}
}

const root = createContainer(
Expand All @@ -236,6 +242,7 @@ export function hydrateRoot(
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
onHydrationError,
);
markContainerAsRoot(root.current, container);
// This can't be a comment node since hydration doesn't work on comment nodes anyway.
Expand Down
1 change: 1 addition & 0 deletions packages/react-native-renderer/src/ReactFabric.js
Expand Up @@ -214,6 +214,7 @@ function render(
false,
null,
'',
null,
);
roots.set(containerTag, root);
}
Expand Down
9 changes: 9 additions & 0 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
Expand Up @@ -95,6 +95,8 @@ export type RendererInspectionConfig = $ReadOnly<{|
) => void,
|}>;

export type ErrorLoggingConfig = null;

// TODO: Remove this conditional once all changes have propagated.
if (registerEventHandler) {
/**
Expand Down Expand Up @@ -525,3 +527,10 @@ export function preparePortalMount(portalInstance: Instance): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logHydrationError(
config: ErrorLoggingConfig,
error: mixed,
): void {
// noop
}
9 changes: 9 additions & 0 deletions packages/react-native-renderer/src/ReactNativeHostConfig.js
Expand Up @@ -55,6 +55,8 @@ export type RendererInspectionConfig = $ReadOnly<{|
) => void,
|}>;

export type ErrorLoggingConfig = null;

const UPDATE_SIGNAL = {};
if (__DEV__) {
Object.freeze(UPDATE_SIGNAL);
Expand Down Expand Up @@ -513,3 +515,10 @@ export function preparePortalMount(portalInstance: Instance): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logHydrationError(
config: ErrorLoggingConfig,
error: mixed,
): void {
// noop
}
1 change: 1 addition & 0 deletions packages/react-native-renderer/src/ReactNativeRenderer.js
Expand Up @@ -210,6 +210,7 @@ function render(
false,
null,
'',
null,
);
roots.set(containerTag, root);
}
Expand Down
4 changes: 4 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Expand Up @@ -466,6 +466,10 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
},

detachDeletedInstance() {},

logHydrationError() {
// no-op
},
};

const hostConfig = useMutation
Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.new.js
Expand Up @@ -15,6 +15,7 @@ import type {
TextInstance,
Container,
PublicInstance,
ErrorLoggingConfig,
} from './ReactFiberHostConfig';
import type {RendererInspectionConfig} from './ReactFiberHostConfig';
import type {ReactNodeList} from 'shared/ReactTypes';
Expand Down Expand Up @@ -245,6 +246,7 @@ export function createContainer(
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
identifierPrefix: string,
errorLoggingConfig: ErrorLoggingConfig,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
Expand All @@ -254,6 +256,7 @@ export function createContainer(
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
errorLoggingConfig,
);
}

Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberReconciler.old.js
Expand Up @@ -15,6 +15,7 @@ import type {
TextInstance,
Container,
PublicInstance,
ErrorLoggingConfig,
} from './ReactFiberHostConfig';
import type {RendererInspectionConfig} from './ReactFiberHostConfig';
import type {ReactNodeList} from 'shared/ReactTypes';
Expand Down Expand Up @@ -245,6 +246,7 @@ export function createContainer(
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
identifierPrefix: string,
errorLoggingConfig: ErrorLoggingConfig,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
Expand All @@ -254,6 +256,7 @@ export function createContainer(
isStrictMode,
concurrentUpdatesByDefaultOverride,
identifierPrefix,
errorLoggingConfig,
);
}

Expand Down
16 changes: 15 additions & 1 deletion packages/react-reconciler/src/ReactFiberRoot.new.js
Expand Up @@ -9,6 +9,7 @@

import type {FiberRoot, SuspenseHydrationCallbacks} from './ReactInternalTypes';
import type {RootTag} from './ReactRootTags';
import type {ErrorLoggingConfig} from './ReactFiberHostConfig';

import {noTimeout, supportsHydration} from './ReactFiberHostConfig';
import {createHostRootFiber} from './ReactFiber.new';
Expand All @@ -30,7 +31,13 @@ import {initializeUpdateQueue} from './ReactUpdateQueue.new';
import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
import {createCache, retainCache} from './ReactFiberCacheComponent.new';

function FiberRootNode(containerInfo, tag, hydrate, identifierPrefix) {
function FiberRootNode(
containerInfo,
tag,
hydrate,
identifierPrefix,
errorLoggingConfig,
) {
this.tag = tag;
this.containerInfo = containerInfo;
this.pendingChildren = null;
Expand All @@ -57,6 +64,7 @@ function FiberRootNode(containerInfo, tag, hydrate, identifierPrefix) {
this.entanglements = createLaneMap(NoLanes);

this.identifierPrefix = identifierPrefix;
this.errorLoggingConfig = errorLoggingConfig;

if (enableCache) {
this.pooledCache = null;
Expand Down Expand Up @@ -103,13 +111,19 @@ export function createFiberRoot(
hydrationCallbacks: null | SuspenseHydrationCallbacks,
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
// TODO: We have several of these arguments that are conceptually part of the
// host config, but because they are passed in at runtime, we have to thread
// them through the root constructor. Perhaps we should put them all into a
// single type, like a DynamicHostConfig that is defined by the renderer.
identifierPrefix: string,
errorLoggingConfig: ErrorLoggingConfig,
): FiberRoot {
const root: FiberRoot = (new FiberRootNode(
containerInfo,
tag,
hydrate,
identifierPrefix,
errorLoggingConfig,
): any);
if (enableSuspenseCallback) {
root.hydrationCallbacks = hydrationCallbacks;
Expand Down

0 comments on commit 6af98f4

Please sign in to comment.