diff --git a/package.json b/package.json index 97d5ef1906b..65085e7a08b 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,6 @@ ".*": "./scripts/jest/preprocessor.js" }, "setupFiles": [ - "./scripts/jest/setup.js", "./scripts/jest/environment.js" ], "setupTestFrameworkScriptFile": "./scripts/jest/test-framework-setup.js", diff --git a/packages/react-reconciler/src/ReactFiberErrorLogger.js b/packages/react-reconciler/src/ReactFiberErrorLogger.js index 61d23326233..06cc1678479 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -25,6 +25,11 @@ export function logCapturedError(capturedError: CapturedError): void { } const error = (capturedError.error: any); + const suppressLogging = error && error.suppressReactErrorLogging; + if (suppressLogging) { + return; + } + if (__DEV__) { const { componentName, diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 412e65fb316..e8cd43c25f3 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -1004,7 +1004,10 @@ export default function( } catch (e) { // Prevent cycle if logCapturedError() throws. // A cycle may still occur if logCapturedError renders a component that throws. - console.error(e); + const suppressLogging = e && e.suppressReactErrorLogging; + if (!suppressLogging) { + console.error(e); + } } // If we're in the commit phase, defer scheduling an update on the diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js index 204e78fbf92..78ee49d7e0f 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js @@ -21,6 +21,10 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop = require('react-noop-renderer'); }); + afterEach(() => { + jest.unmock('../ReactFiberErrorLogger'); + }); + function div(...children) { children = children.map(c => (typeof c === 'string' ? {text: c} : c)); return {type: 'div', children, prop: undefined}; @@ -952,49 +956,22 @@ describe('ReactIncrementalErrorHandling', () => { }); describe('ReactFiberErrorLogger', () => { - function initReactFiberErrorLoggerMock(mock) { - jest.resetModules(); - if (mock) { - jest.mock('../ReactFiberErrorLogger'); - } else { - jest.unmock('../ReactFiberErrorLogger'); - } - React = require('react'); - ReactNoop = require('react-noop-renderer'); - } - - beforeEach(() => { - // Assert that we're mocking this file by default. - // If this ever changes, we'll need to update this test suite anyway. - expect( - require('../ReactFiberErrorLogger').logCapturedError._isMockFunction, - ).toBe(true); - }); - - afterEach(() => { - // Restore the default (we verified it was being mocked in beforeEach). - jest.mock('../ReactFiberErrorLogger'); - }); - function normalizeCodeLocInfo(str) { return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); } it('should log errors that occur during the begin phase', () => { - initReactFiberErrorLoggerMock(false); // Intentionally spy in production too. - // TODO: It is confusing how these tests correctly "see" console.error()s - // from exceptions becase they called initReactFiberErrorLoggerMock(false) - // and thus unmocked the error logger. It is globally mocked in all other - // tests. We did this to silence warnings from intentionally caught errors - // in tests. But perhaps we should instead make a pass through all tests, - // intentionally assert console.error() is always called for uncaught - // exceptions, and then remove the global mock of ReactFiberErrorLogger. spyOn(console, 'error'); class ErrorThrowingComponent extends React.Component { componentWillMount() { - throw Error('componentWillMount error'); + const error = new Error('componentWillMount error'); + // Note: it's `true` on the Error prototype our test environment. + // That lets us avoid asserting on warnings for each expected error. + // Here we intentionally shadow it to test logging, like in real apps. + error.suppressReactErrorLogging = undefined; + throw error; } render() { return
; @@ -1030,19 +1007,17 @@ describe('ReactIncrementalErrorHandling', () => { }); it('should log errors that occur during the commit phase', () => { - initReactFiberErrorLoggerMock(false); - // TODO: It is confusing how these tests correctly "see" console.error()s - // from exceptions becase they called initReactFiberErrorLoggerMock(false) - // and thus unmocked the error logger. It is globally mocked in all other - // tests. We did this to silence warnings from intentionally caught errors - // in tests. But perhaps we should instead make a pass through all tests, - // intentionally assert console.error() is always called for uncaught - // exceptions, and then remove the global mock of ReactFiberErrorLogger. + // Intentionally spy in production too. spyOn(console, 'error'); class ErrorThrowingComponent extends React.Component { componentDidMount() { - throw Error('componentDidMount error'); + const error = new Error('componentDidMount error'); + // Note: it's `true` on the Error prototype our test environment. + // That lets us avoid asserting on warnings for each expected error. + // Here we intentionally shadow it to test logging, like in real apps. + error.suppressReactErrorLogging = undefined; + throw error; } render() { return
; @@ -1078,18 +1053,16 @@ describe('ReactIncrementalErrorHandling', () => { }); it('should ignore errors thrown in log method to prevent cycle', () => { - initReactFiberErrorLoggerMock(true); + jest.resetModules(); + jest.mock('../ReactFiberErrorLogger'); + React = require('react'); + ReactNoop = require('react-noop-renderer'); // Intentionally spy in production too. - // Note: we're seeing console.error() even though ReactFiberErrorLogger is - // mocked because we're currently testing the console.error() call inside - // ReactFiberScheduler (for cycles). It doesn't get affected by mocking. - // TODO: mocking is confusing and we should simplify this and remove - // special cases. spyOn(console, 'error'); class ErrorThrowingComponent extends React.Component { render() { - throw Error('render error'); + throw new Error('render error'); } } @@ -1099,7 +1072,12 @@ describe('ReactIncrementalErrorHandling', () => { ReactFiberErrorLogger.logCapturedError.mockImplementation( capturedError => { logCapturedErrorCalls.push(capturedError); - throw Error('logCapturedError error'); + const error = new Error('logCapturedError error'); + // Note: it's `true` on the Error prototype our test environment. + // That lets us avoid asserting on warnings for each expected error. + // Here we intentionally shadow it to test logging, like in real apps. + error.suppressReactErrorLogging = undefined; + throw error; }, ); @@ -1124,14 +1102,7 @@ describe('ReactIncrementalErrorHandling', () => { }); it('should relay info about error boundary and retry attempts if applicable', () => { - initReactFiberErrorLoggerMock(false); - // TODO: It is confusing how these tests correctly "see" console.error()s - // from exceptions becase they called initReactFiberErrorLoggerMock(false) - // and thus unmocked the error logger. It is globally mocked in all other - // tests. We did this to silence warnings from intentionally caught errors - // in tests. But perhaps we should instead make a pass through all tests, - // intentionally assert console.error() is always called for uncaught - // exceptions, and then remove the global mock of ReactFiberErrorLogger. + // Intentionally spy in production too. spyOn(console, 'error'); class ParentComponent extends React.Component { @@ -1155,7 +1126,12 @@ describe('ReactIncrementalErrorHandling', () => { class ErrorThrowingComponent extends React.Component { componentDidMount() { - throw Error('componentDidMount error'); + const error = new Error('componentDidMount error'); + // Note: it's `true` on the Error prototype our test environment. + // That lets us avoid asserting on warnings for each expected error. + // Here we intentionally shadow it to test logging, like in real apps. + error.suppressReactErrorLogging = undefined; + throw error; } render() { renderAttempts++; diff --git a/scripts/jest/environment.js b/scripts/jest/environment.js index 27d5bbe1a40..ea10c6bf414 100644 --- a/scripts/jest/environment.js +++ b/scripts/jest/environment.js @@ -23,3 +23,12 @@ global.requestIdleCallback = function(callback) { global.cancelIdleCallback = function(callbackID) { clearTimeout(callbackID); }; + +// By default React console.error()'s any errors, caught or uncaught. +// However it is annoying to assert that a warning fired each time +// we assert that there is an exception in our tests. This lets us +// opt out of extra console error reporting for most tests except +// for the few that specifically test the logging by shadowing this +// property. In real apps, it would usually not be defined at all. +Error.prototype.suppressReactErrorLogging = true; +DOMException.prototype.suppressReactErrorLogging = true; diff --git a/scripts/jest/setup.js b/scripts/jest/setup.js deleted file mode 100644 index 63bdc9fbbf6..00000000000 --- a/scripts/jest/setup.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; - -jest.mock('shared/ReactFeatureFlags', () => { - // We can alter flags based on environment here - // (e.g. for CI runs with different flags). - return require.requireActual('shared/ReactFeatureFlags'); -}); - -// Error logging varies between Fiber and Stack; -// Rather than fork dozens of tests, mock the error-logging file by default. -// TODO: direct imports like some-package/src/* are bad. Fix me. -jest.mock('react-reconciler/src/ReactFiberErrorLogger');