diff --git a/packages/internal-test-utils/ReactInternalTestUtils.js b/packages/internal-test-utils/ReactInternalTestUtils.js index df77f23448b13..c6cb39fd73e08 100644 --- a/packages/internal-test-utils/ReactInternalTestUtils.js +++ b/packages/internal-test-utils/ReactInternalTestUtils.js @@ -13,6 +13,8 @@ import simulateBrowserEventDispatch from './simulateBrowserEventDispatch'; export {act} from './internalAct'; +import {thrownErrors, actingUpdatesScopeDepth} from './internalAct'; + function assertYieldsWereCleared(caller) { const actualYields = SchedulerMock.unstable_clearLog(); if (actualYields.length !== 0) { @@ -110,6 +112,14 @@ ${diff(expectedLog, actualLog)} throw error; } +function aggregateErrors(errors: Array): mixed { + if (errors.length > 1 && typeof AggregateError === 'function') { + // eslint-disable-next-line no-undef + return new AggregateError(errors); + } + return errors[0]; +} + export async function waitForThrow(expectedError: mixed): mixed { assertYieldsWereCleared(waitForThrow); @@ -126,31 +136,72 @@ export async function waitForThrow(expectedError: mixed): mixed { error.message = 'Expected something to throw, but nothing did.'; throw error; } + + const errorHandlerDOM = function (event: ErrorEvent) { + // Prevent logs from reprinting this error. + event.preventDefault(); + thrownErrors.push(event.error); + }; + const errorHandlerNode = function (err: mixed) { + thrownErrors.push(err); + }; + // We track errors that were logged globally as if they occurred in this scope and then rethrow them. + if (actingUpdatesScopeDepth === 0) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.addEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.on('uncaughtException', errorHandlerNode); + } + } try { SchedulerMock.unstable_flushAllWithoutAsserting(); } catch (x) { + thrownErrors.push(x); + } finally { + if (actingUpdatesScopeDepth === 0) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.removeEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.off('uncaughtException', errorHandlerNode); + } + } + } + if (thrownErrors.length > 0) { + const thrownError = aggregateErrors(thrownErrors); + thrownErrors.length = 0; + if (expectedError === undefined) { // If no expected error was provided, then assume the caller is OK with // any error being thrown. We're returning the error so they can do // their own checks, if they wish. - return x; + return thrownError; } - if (equals(x, expectedError)) { - return x; + if (equals(thrownError, expectedError)) { + return thrownError; } if ( typeof expectedError === 'string' && - typeof x === 'object' && - x !== null && - typeof x.message === 'string' + typeof thrownError === 'object' && + thrownError !== null && + typeof thrownError.message === 'string' ) { - if (x.message.includes(expectedError)) { - return x; + if (thrownError.message.includes(expectedError)) { + return thrownError; } else { error.message = ` Expected error was not thrown. -${diff(expectedError, x.message)} +${diff(expectedError, thrownError.message)} `; throw error; } @@ -158,7 +209,7 @@ ${diff(expectedError, x.message)} error.message = ` Expected error was not thrown. -${diff(expectedError, x)} +${diff(expectedError, thrownError)} `; throw error; } diff --git a/packages/internal-test-utils/internalAct.js b/packages/internal-test-utils/internalAct.js index 082be156dc89b..7b22c44697e36 100644 --- a/packages/internal-test-utils/internalAct.js +++ b/packages/internal-test-utils/internalAct.js @@ -20,7 +20,9 @@ import * as Scheduler from 'scheduler/unstable_mock'; import enqueueTask from './enqueueTask'; -let actingUpdatesScopeDepth: number = 0; +export let actingUpdatesScopeDepth: number = 0; + +export const thrownErrors: Array = []; async function waitForMicrotasks() { return new Promise(resolve => { @@ -28,6 +30,14 @@ async function waitForMicrotasks() { }); } +function aggregateErrors(errors: Array): mixed { + if (errors.length > 1 && typeof AggregateError === 'function') { + // eslint-disable-next-line no-undef + return new AggregateError(errors); + } + return errors[0]; +} + export async function act(scope: () => Thenable): Thenable { if (Scheduler.unstable_flushUntilNextPaint === undefined) { throw Error( @@ -63,6 +73,28 @@ export async function act(scope: () => Thenable): Thenable { // public version of `act`, though we maybe should in the future. await waitForMicrotasks(); + const errorHandlerDOM = function (event: ErrorEvent) { + // Prevent logs from reprinting this error. + event.preventDefault(); + thrownErrors.push(event.error); + }; + const errorHandlerNode = function (err: mixed) { + thrownErrors.push(err); + }; + // We track errors that were logged globally as if they occurred in this scope and then rethrow them. + if (actingUpdatesScopeDepth === 1) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.addEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.on('uncaughtException', errorHandlerNode); + } + } + try { const result = await scope(); @@ -106,10 +138,27 @@ export async function act(scope: () => Thenable): Thenable { Scheduler.unstable_flushUntilNextPaint(); } while (true); + if (thrownErrors.length > 0) { + // Rethrow any errors logged by the global error handling. + const thrownError = aggregateErrors(thrownErrors); + thrownErrors.length = 0; + throw thrownError; + } + return result; } finally { const depth = actingUpdatesScopeDepth; if (depth === 1) { + if ( + typeof window === 'object' && + typeof window.addEventListener === 'function' + ) { + // We're in a JS DOM environment. + window.removeEventListener('error', errorHandlerDOM); + } else if (typeof process === 'object') { + // Node environment + process.off('uncaughtException', errorHandlerNode); + } global.IS_REACT_ACT_ENVIRONMENT = previousIsActEnvironment; } actingUpdatesScopeDepth = depth - 1; diff --git a/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js b/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js index f26ad485590d6..7e7221a6d393f 100644 --- a/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom-bindings/src/events/DOMPluginEventSystem.js @@ -68,6 +68,8 @@ import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; import * as FormActionEventPlugin from './plugins/FormActionEventPlugin'; +import reportGlobalError from 'shared/reportGlobalError'; + type DispatchListener = { instance: null | Fiber, listener: Function, @@ -226,9 +228,6 @@ export const nonDelegatedEvents: Set = new Set([ ...mediaEventTypes, ]); -let hasError: boolean = false; -let caughtError: mixed = null; - function executeDispatch( event: ReactSyntheticEvent, listener: Function, @@ -238,12 +237,7 @@ function executeDispatch( try { listener(event); } catch (error) { - if (!hasError) { - hasError = true; - caughtError = error; - } else { - // TODO: Make sure this error gets logged somehow. - } + reportGlobalError(error); } event.currentTarget = null; } @@ -285,13 +279,6 @@ export function processDispatchQueue( processDispatchQueueItemsInOrder(event, listeners, inCapturePhase); // event system doesn't use pooling. } - // This would be a good time to rethrow if any of the event handlers threw. - if (hasError) { - const error = caughtError; - hasError = false; - caughtError = null; - throw error; - } } function dispatchEventsForPlugins( diff --git a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js index e412256ab2678..d7045d2756ad4 100644 --- a/packages/react-dom/src/__tests__/InvalidEventListeners-test.js +++ b/packages/react-dom/src/__tests__/InvalidEventListeners-test.js @@ -51,13 +51,11 @@ describe('InvalidEventListeners', () => { } window.addEventListener('error', handleWindowError); try { - await act(() => { - node.dispatchEvent( - new MouseEvent('click', { - bubbles: true, - }), - ); - }); + node.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + }), + ); } finally { window.removeEventListener('error', handleWindowError); } diff --git a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js index 9c023f06d9d2a..965c20dbddf53 100644 --- a/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js +++ b/packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.js @@ -195,9 +195,7 @@ describe('ReactBrowserEventEmitter', () => { }); window.addEventListener('error', errorHandler); try { - await act(() => { - CHILD.click(); - }); + CHILD.click(); expect(idCallOrder.length).toBe(3); expect(idCallOrder[0]).toBe(CHILD); expect(idCallOrder[1]).toBe(PARENT); diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index bf082369e0610..561928b24faf3 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -223,12 +223,12 @@ describe('ReactCompositeComponent', () => { const el = document.createElement('div'); const root = ReactDOMClient.createRoot(el); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow( + }).rejects.toThrow( 'Objects are not valid as a React child (found: object with keys {render}).', ); }).toErrorDev( @@ -526,12 +526,12 @@ describe('ReactCompositeComponent', () => { } } const root = ReactDOMClient.createRoot(container); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(TypeError); + }).rejects.toThrow(TypeError); }).toErrorDev( 'Warning: The component appears to have a render method, ' + "but doesn't extend React.Component. This is likely to cause errors. " + @@ -539,11 +539,11 @@ describe('ReactCompositeComponent', () => { ); // Test deduplication - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(TypeError); + }).rejects.toThrow(TypeError); }); it('should warn about `setState` in render', async () => { @@ -596,11 +596,11 @@ describe('ReactCompositeComponent', () => { expect(ReactCurrentOwner.current).toBe(null); const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await act(() => { root.render(instance); }); - }).toThrow(); + }).rejects.toThrow(); expect(ReactCurrentOwner.current).toBe(null); }); @@ -884,7 +884,7 @@ describe('ReactCompositeComponent', () => { ); }); - it('should only call componentWillUnmount once', () => { + it('should only call componentWillUnmount once', async () => { let app; let count = 0; @@ -919,14 +919,14 @@ describe('ReactCompositeComponent', () => { }; const root = ReactDOMClient.createRoot(container); - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await act(() => { root.render(); }); - ReactDOM.flushSync(() => { + await act(() => { root.render(); }); - }).toThrow(); + }).rejects.toThrow(); expect(count).toBe(1); }); @@ -1211,7 +1211,7 @@ describe('ReactCompositeComponent', () => { assertLog(['setState callback called']); }); - it('should return a meaningful warning when constructor is returned', () => { + it('should return a meaningful warning when constructor is returned', async () => { class RenderTextInvalidConstructor extends React.Component { constructor(props) { super(props); @@ -1224,12 +1224,12 @@ describe('ReactCompositeComponent', () => { } const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(); + }).rejects.toThrow(); }).toErrorDev([ 'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' + 'did you accidentally return an object from the constructor?', @@ -1260,16 +1260,16 @@ describe('ReactCompositeComponent', () => { ); }); - it('should return error if render is not defined', () => { + it('should return error if render is not defined', async () => { class RenderTestUndefinedRender extends React.Component {} const root = ReactDOMClient.createRoot(document.createElement('div')); - expect(() => { - expect(() => { - ReactDOM.flushSync(() => { + await expect(async () => { + await expect(async () => { + await act(() => { root.render(); }); - }).toThrow(); + }).rejects.toThrow(); }).toErrorDev([ 'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' + 'you may have forgotten to define `render`.', diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 83e3e0229d7b1..e72f726c16503 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -166,6 +166,9 @@ describe('ReactDOM', () => { // @gate !disableLegacyMode it('throws in render() if the mount callback in legacy roots is not a function', async () => { + spyOnDev(console, 'warn'); + spyOnDev(console, 'error'); + function Foo() { this.a = 1; this.b = 2; @@ -180,40 +183,55 @@ describe('ReactDOM', () => { } const myDiv = document.createElement('div'); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, 'no'); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: no.', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, 'no'); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: no', + }).toErrorDev( + [ + 'Warning: Expected the last optional `callback` argument to be a function. Instead received: no.', + 'Warning: Expected the last optional `callback` argument to be a function. Instead received: no.', + ], + {withoutStack: 2}, ); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, {foo: 'bar'}); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, {foo: 'bar'}); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, new Foo()); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, new Foo()); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); }); @@ -234,42 +252,57 @@ describe('ReactDOM', () => { const myDiv = document.createElement('div'); ReactDOM.render(, myDiv); - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, 'no'); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: no.', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, 'no'); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: no', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: no', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: no.', + 'Expected the last optional `callback` argument to be a function. Instead received: no.', + ], + {withoutStack: 2}, ); ReactDOM.render(, myDiv); // Re-mount - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, {foo: 'bar'}); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, {foo: 'bar'}); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); ReactDOM.render(, myDiv); // Re-mount - expect(() => { - expect(() => { - ReactDOM.render(, myDiv, new Foo()); - }).toErrorDev( - 'Expected the last optional `callback` argument to be ' + - 'a function. Instead received: [object Object].', + await expect(async () => { + await expect(async () => { + await act(() => { + ReactDOM.render(, myDiv, new Foo()); + }); + }).rejects.toThrowError( + 'Invalid argument passed as callback. Expected a function. Instead ' + + 'received: [object Object]', ); - }).toThrowError( - 'Invalid argument passed as callback. Expected a function. Instead ' + - 'received: [object Object]', + }).toErrorDev( + [ + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + 'Expected the last optional `callback` argument to be a function. Instead received: [object Object].', + ], + {withoutStack: 2}, ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js index bd80905478bf0..65aa67d342579 100644 --- a/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMConsoleErrorReporting-test.js @@ -16,16 +16,14 @@ describe('ReactDOMConsoleErrorReporting', () => { let NoError; let container; let windowOnError; - let waitForThrow; + let Scheduler; beforeEach(() => { jest.resetModules(); act = require('internal-test-utils').act; React = require('react'); ReactDOMClient = require('react-dom/client'); - - const InternalTestUtils = require('internal-test-utils'); - waitForThrow = InternalTestUtils.waitForThrow; + Scheduler = require('scheduler'); ErrorBoundary = class extends React.Component { state = {error: null}; @@ -46,6 +44,8 @@ describe('ReactDOMConsoleErrorReporting', () => { document.body.appendChild(container); windowOnError = jest.fn(); window.addEventListener('error', windowOnError); + spyOnDevAndProd(console, 'error').mockImplementation(() => {}); + spyOnDevAndProd(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { @@ -54,11 +54,14 @@ describe('ReactDOMConsoleErrorReporting', () => { jest.restoreAllMocks(); }); + async function fakeAct(cb) { + // We don't use act/waitForThrow here because we want to observe how errors are reported for real. + await cb(); + Scheduler.unstable_flushAll(); + } + describe('ReactDOMClient.createRoot', () => { it('logs errors during event handlers', async () => { - const originalError = console.error; - console.error = jest.fn(); - function Foo() { return (