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

Resolve default onRecoverableError before passing to root constructor #23264

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
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