Skip to content

Commit

Permalink
Resolve default onRecoverableError at root init (facebook#23264)
Browse files Browse the repository at this point in the history
Minor follow up to initial onRecoverableError PR.

When onRecoverableError is not provided to `createRoot`, the
renderer falls back to a default implementation. Originally I
implemented this with a host config method, but what we can do instead
is pass the default implementation the root constructor as if it were
a user provided one.
  • Loading branch information
acdlite authored and zhengjitf committed Apr 15, 2022
1 parent c91ba1a commit f804fbf
Show file tree
Hide file tree
Showing 16 changed files with 57 additions and 55 deletions.
4 changes: 0 additions & 4 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,3 @@ export function preparePortalMount(portalInstance: any): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logRecoverableError(error) {
// noop
}
12 changes: 0 additions & 12 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,18 +374,6 @@ export function getCurrentEventPriority(): * {
return getEventPriority(currentEvent.type);
}

/* global reportError */
export const logRecoverableError =
typeof reportError === 'function'
? // In modern browsers, reportError will dispatch an error event,
// emulating an uncaught JavaScript error.
reportError
: (error: mixed) => {
// In older browsers and test environments, fallback to console.error.
// eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args
console.error(error);
};

export const isPrimaryRenderer = true;
export const warnsIfNotActing = true;
// This initialization code may run even on server environments
Expand Down
7 changes: 6 additions & 1 deletion packages/react-dom/src/client/ReactDOMLegacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ function getReactRootElementInContainer(container: any) {
}
}

function noopOnRecoverableError() {
// This isn't reachable because onRecoverableError isn't called in the
// legacy API.
}

function legacyCreateRootFromDOMContainer(
container: Container,
forceHydrate: boolean,
Expand All @@ -122,7 +127,7 @@ function legacyCreateRootFromDOMContainer(
false, // isStrictMode
false, // concurrentUpdatesByDefaultOverride,
'', // identifierPrefix
null,
noopOnRecoverableError,
);
markContainerAsRoot(root.current, container);

Expand Down
16 changes: 14 additions & 2 deletions packages/react-dom/src/client/ReactDOMRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ import {
import {ConcurrentRoot} from 'react-reconciler/src/ReactRootTags';
import {allowConcurrentByDefault} from 'shared/ReactFeatureFlags';

/* global reportError */
const defaultOnRecoverableError =
typeof reportError === 'function'
? // In modern browsers, reportError will dispatch an error event,
// emulating an uncaught JavaScript error.
reportError
: (error: mixed) => {
// In older browsers and test environments, fallback to console.error.
// eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args
console.error(error);
};

function ReactDOMRoot(internalRoot: FiberRoot) {
this._internalRoot = internalRoot;
}
Expand Down Expand Up @@ -145,7 +157,7 @@ export function createRoot(
let isStrictMode = false;
let concurrentUpdatesByDefaultOverride = false;
let identifierPrefix = '';
let onRecoverableError = null;
let onRecoverableError = defaultOnRecoverableError;
if (options !== null && options !== undefined) {
if (__DEV__) {
if ((options: any).hydrate) {
Expand Down Expand Up @@ -220,7 +232,7 @@ export function hydrateRoot(
let isStrictMode = false;
let concurrentUpdatesByDefaultOverride = false;
let identifierPrefix = '';
let onRecoverableError = null;
let onRecoverableError = defaultOnRecoverableError;
if (options !== null && options !== undefined) {
if (options.unstable_strictMode === true) {
isStrictMode = true;
Expand Down
8 changes: 7 additions & 1 deletion packages/react-native-renderer/src/ReactFabric.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ function sendAccessibilityEvent(handle: any, eventType: string) {
}
}

function onRecoverableError(error) {
// TODO: Expose onRecoverableError option to userspace
// eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args
console.error(error);
}

function render(
element: Element<ElementType>,
containerTag: number,
Expand All @@ -214,7 +220,7 @@ function render(
false,
null,
'',
null,
onRecoverableError,
);
roots.set(containerTag, root);
}
Expand Down
4 changes: 0 additions & 4 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,3 @@ export function preparePortalMount(portalInstance: Instance): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logRecoverableError(error: mixed): void {
// noop
}
4 changes: 0 additions & 4 deletions packages/react-native-renderer/src/ReactNativeHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,3 @@ export function preparePortalMount(portalInstance: Instance): void {
export function detachDeletedInstance(node: Instance): void {
// noop
}

export function logRecoverableError(error: mixed): void {
// noop
}
8 changes: 7 additions & 1 deletion packages/react-native-renderer/src/ReactNativeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ function sendAccessibilityEvent(handle: any, eventType: string) {
}
}

function onRecoverableError(error) {
// TODO: Expose onRecoverableError option to userspace
// eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args
console.error(error);
}

function render(
element: Element<ElementType>,
containerTag: number,
Expand All @@ -210,7 +216,7 @@ function render(
false,
null,
'',
null,
onRecoverableError,
);
roots.set(containerTag, root);
}
Expand Down
12 changes: 9 additions & 3 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return NoopRenderer.flushSync(fn);
}

function onRecoverableError(error) {
// TODO: Turn this on once tests are fixed
// eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args
// console.error(error);
}

let idCounter = 0;

const ReactNoop = {
Expand Down Expand Up @@ -966,7 +972,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
null,
false,
'',
null,
onRecoverableError,
);
roots.set(rootID, root);
}
Expand All @@ -988,7 +994,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
null,
false,
'',
null,
onRecoverableError,
);
return {
_Scheduler: Scheduler,
Expand Down Expand Up @@ -1018,7 +1024,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
null,
false,
'',
null,
onRecoverableError,
);
return {
_Scheduler: Scheduler,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberReconciler.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export function createContainer(
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
identifierPrefix: string,
onRecoverableError: null | ((error: mixed) => void),
onRecoverableError: (error: mixed) => void,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberReconciler.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ export function createContainer(
isStrictMode: boolean,
concurrentUpdatesByDefaultOverride: null | boolean,
identifierPrefix: string,
onRecoverableError: null | ((error: mixed) => void),
onRecoverableError: (error: mixed) => void,
): OpaqueRoot {
return createFiberRoot(
containerInfo,
Expand Down
11 changes: 2 additions & 9 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ import {
supportsMicrotasks,
errorHydratingContainer,
scheduleMicrotask,
logRecoverableError,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -2113,16 +2112,10 @@ function commitRootImpl(
if (recoverableErrors !== null) {
// There were errors during this render, but recovered from them without
// needing to surface it to the UI. We log them here.
const onRecoverableError = root.onRecoverableError;
for (let i = 0; i < recoverableErrors.length; i++) {
const recoverableError = recoverableErrors[i];
const onRecoverableError = root.onRecoverableError;
if (onRecoverableError !== null) {
onRecoverableError(recoverableError);
} else {
// No user-provided onRecoverableError. Use the default behavior
// provided by the renderer's host config.
logRecoverableError(recoverableError);
}
onRecoverableError(recoverableError);
}
}

Expand Down
11 changes: 2 additions & 9 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ import {
supportsMicrotasks,
errorHydratingContainer,
scheduleMicrotask,
logRecoverableError,
} from './ReactFiberHostConfig';

import {
Expand Down Expand Up @@ -2113,16 +2112,10 @@ function commitRootImpl(
if (recoverableErrors !== null) {
// There were errors during this render, but recovered from them without
// needing to surface it to the UI. We log them here.
const onRecoverableError = root.onRecoverableError;
for (let i = 0; i < recoverableErrors.length; i++) {
const recoverableError = recoverableErrors[i];
const onRecoverableError = root.onRecoverableError;
if (onRecoverableError !== null) {
onRecoverableError(recoverableError);
} else {
// No user-provided onRecoverableError. Use the default behavior
// provided by the renderer's host config.
logRecoverableError(recoverableError);
}
onRecoverableError(recoverableError);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactInternalTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ type BaseFiberRootProperties = {|
// a reference to.
identifierPrefix: string,

onRecoverableError: null | ((error: mixed) => void),
onRecoverableError: (error: mixed) => void,
|};

// The following attributes are only used by DevTools and are only present in DEV builds.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ export const prepareScopeUpdate = $$$hostConfig.preparePortalMount;
export const getInstanceFromScope = $$$hostConfig.getInstanceFromScope;
export const getCurrentEventPriority = $$$hostConfig.getCurrentEventPriority;
export const detachDeletedInstance = $$$hostConfig.detachDeletedInstance;
export const logRecoverableError = $$$hostConfig.logRecoverableError;

// -------------------
// Microtasks
Expand Down
8 changes: 7 additions & 1 deletion packages/react-test-renderer/src/ReactTestRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,12 @@ function propsMatch(props: Object, filter: Object): boolean {
return true;
}

function onRecoverableError(error) {
// TODO: Expose onRecoverableError option to userspace
// eslint-disable-next-line react-internal/no-production-logging, react-internal/warning-args
console.error(error);
}

function create(element: React$Element<any>, options: TestRendererOptions) {
let createNodeMock = defaultTestOptions.createNodeMock;
let isConcurrent = false;
Expand Down Expand Up @@ -472,7 +478,7 @@ function create(element: React$Element<any>, options: TestRendererOptions) {
isStrictMode,
concurrentUpdatesByDefault,
'',
null,
onRecoverableError,
);

if (root == null) {
Expand Down

0 comments on commit f804fbf

Please sign in to comment.