diff --git a/package.json b/package.json index 97d5ef1906bc5..65085e7a08b23 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 61d23326233fd..1b8aef24f9f7d 100644 --- a/packages/react-reconciler/src/ReactFiberErrorLogger.js +++ b/packages/react-reconciler/src/ReactFiberErrorLogger.js @@ -11,7 +11,21 @@ import type {CapturedError} from './ReactFiberScheduler'; import invariant from 'fbjs/lib/invariant'; -const defaultShowDialog = (capturedError: CapturedError) => true; +declare var __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__: void | boolean; + +const defaultShowDialog = (capturedError: CapturedError) => { + // This is a somewhat hacky way to disable error logging. + // Can be handy in tests. + // We can revisit this when we offer a more comprehensive + // solution for overriding warning behavior. + if ( + typeof __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ !== 'undefined' && + __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ === true + ) { + return false; + } + return true; +}; let showDialog = defaultShowDialog; diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.js index 204e78fbf92ec..b1acf19b2b480 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,44 +956,14 @@ 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); + // Note: It is globally true by default. + global.__REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ = 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 { @@ -1030,14 +1004,9 @@ 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. + // Note: It is globally true by default. + global.__REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ = false; + // Intentionally spy in production too. spyOn(console, 'error'); class ErrorThrowingComponent extends React.Component { @@ -1078,13 +1047,10 @@ describe('ReactIncrementalErrorHandling', () => { }); it('should ignore errors thrown in log method to prevent cycle', () => { - initReactFiberErrorLoggerMock(true); - // 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. + jest.resetModules(); + jest.mock('../ReactFiberErrorLogger'); + React = require('react'); + ReactNoop = require('react-noop-renderer'); spyOn(console, 'error'); class ErrorThrowingComponent extends React.Component { @@ -1124,14 +1090,9 @@ 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. + // Note: It is globally true by default. + global.__REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ = false; + // Intentionally spy in production too. spyOn(console, 'error'); class ParentComponent extends React.Component { diff --git a/scripts/jest/setup.js b/scripts/jest/setup.js deleted file mode 100644 index 63bdc9fbbf620..0000000000000 --- 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'); diff --git a/scripts/jest/setupSpecEquivalenceReporter.js b/scripts/jest/setupSpecEquivalenceReporter.js index fdc3429e559f4..1be0c94ae584a 100644 --- a/scripts/jest/setupSpecEquivalenceReporter.js +++ b/scripts/jest/setupSpecEquivalenceReporter.js @@ -22,7 +22,13 @@ global.spyOnDev = function(...args) { } }; -beforeEach(() => (numExpectations = 0)); +beforeEach(() => { + numExpectations = 0; + // Suppresses additional console.error() when an error is caught by React. + // Unless error boundary catches it, React will rethrow it anyway. + // We need this in tests or they become too noisy (we often assert on errors). + global.__REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ = true; +}); jasmine.currentEnv_.addReporter({ specDone: spec => { diff --git a/scripts/jest/test-framework-setup.js b/scripts/jest/test-framework-setup.js index d99e50948760c..707d876ae05f6 100644 --- a/scripts/jest/test-framework-setup.js +++ b/scripts/jest/test-framework-setup.js @@ -28,6 +28,11 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { env.beforeEach(() => { newMethod.__callCount = 0; + + // Suppresses additional console.error() when an error is caught by React. + // Unless error boundary catches it, React will rethrow it anyway. + // We need this in tests or they become too noisy (we often assert on errors). + global.__REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__ = true; }); env.afterEach(() => { diff --git a/scripts/rollup/validate/eslintrc.cjs.js b/scripts/rollup/validate/eslintrc.cjs.js index 82c236f7c2228..21d6d83973710 100644 --- a/scripts/rollup/validate/eslintrc.cjs.js +++ b/scripts/rollup/validate/eslintrc.cjs.js @@ -15,6 +15,7 @@ module.exports = { // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, + __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__: true, // CommonJS / Node process: true, }, diff --git a/scripts/rollup/validate/eslintrc.fb.js b/scripts/rollup/validate/eslintrc.fb.js index 527fd0a473c98..b8b6f681443f8 100644 --- a/scripts/rollup/validate/eslintrc.fb.js +++ b/scripts/rollup/validate/eslintrc.fb.js @@ -15,6 +15,7 @@ module.exports = { // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, + __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__: true, // FB __DEV__: true, }, diff --git a/scripts/rollup/validate/eslintrc.rn.js b/scripts/rollup/validate/eslintrc.rn.js index 527fd0a473c98..b8b6f681443f8 100644 --- a/scripts/rollup/validate/eslintrc.rn.js +++ b/scripts/rollup/validate/eslintrc.rn.js @@ -15,6 +15,7 @@ module.exports = { // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, + __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__: true, // FB __DEV__: true, }, diff --git a/scripts/rollup/validate/eslintrc.umd.js b/scripts/rollup/validate/eslintrc.umd.js index bfc5396ce52ec..af8b53388d7f4 100644 --- a/scripts/rollup/validate/eslintrc.umd.js +++ b/scripts/rollup/validate/eslintrc.umd.js @@ -14,6 +14,7 @@ module.exports = { // Vendor specific MSApp: true, __REACT_DEVTOOLS_GLOBAL_HOOK__: true, + __REACT_UNSTABLE_SUPPRESS_ERROR_LOGGING__: true, // UMD wrapper code // TODO: this is too permissive. // Ideally we should only allow these *inside* the UMD wrapper.