From dd8e5f468a29e299647ffbd0887f53afd24936e3 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Tue, 10 Dec 2019 02:28:06 -0800 Subject: [PATCH] Add unstable_enableLogBox Summary: This diff adds a new `unstable_enableLogBox` function to opt-into the new LogBox experience. If LogBox is not enabled early enough, we show an error with instructions. With this, LogBox can be enabled with: ``` require('react-native').unstable_enableLogBox(); ``` Changelog: [General] [Adds] unstable_enableLogBox Reviewed By: zackargyle, rubennorte Differential Revision: D18808940 fbshipit-source-id: 4b0234ddc4d1646515bf63110d5b02133780512e --- Libraries/Core/ExceptionsManager.js | 2 +- Libraries/ReactNative/AppContainer.js | 30 +++----- Libraries/ReactNative/AppRegistry.js | 16 ++-- Libraries/YellowBox/YellowBox.js | 77 ++++++++++--------- Libraries/YellowBox/YellowBoxContainer.js | 65 ++++++++++++++++ .../YellowBox/__tests__/YellowBox-test.js | 53 +++++++++++++ .../__snapshots__/YellowBox-test.js.snap | 5 ++ index.js | 3 + 8 files changed, 189 insertions(+), 62 deletions(-) create mode 100644 Libraries/YellowBox/YellowBoxContainer.js create mode 100644 Libraries/YellowBox/__tests__/__snapshots__/YellowBox-test.js.snap diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index dc759f0b958fd3..d26f824ab5c280 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -83,7 +83,7 @@ function reportException(e: ExtendedError, isFatal: boolean) { e.jsEngine == null ? message : `${message}, js engine: ${e.jsEngine}`; const isHandledByLogBox = - e.forceRedbox !== true && global.__reactExperimentalLogBox; + e.forceRedbox !== true && global.__unstable_isLogBoxEnabled === true; const data = preprocessException({ message, diff --git a/Libraries/ReactNative/AppContainer.js b/Libraries/ReactNative/AppContainer.js index 30b3b212255d6e..eab3b420958363 100644 --- a/Libraries/ReactNative/AppContainer.js +++ b/Libraries/ReactNative/AppContainer.js @@ -94,17 +94,14 @@ class AppContainer extends React.Component { } render(): React.Node { - let logBox = null; - if (__DEV__ && !this.props.internal_excludeLogBox) { - if (!global.__RCTProfileIsProfiling) { - if (global.__reactExperimentalLogBox) { - const LogBoxNotificationContainer = require('../LogBox/LogBoxNotificationContainer') - .default; - logBox = ; - } else { - const YellowBox = require('../YellowBox/YellowBox'); - logBox = ; - } + let yellowBox = null; + if (__DEV__) { + if ( + !global.__RCTProfileIsProfiling && + !this.props.internal_excludeLogBox + ) { + const YellowBox = require('../YellowBox/YellowBox'); + yellowBox = ; } } @@ -138,7 +135,7 @@ class AppContainer extends React.Component { {!this.state.hasError && innerView} {this.state.inspector} - {logBox} + {yellowBox} ); @@ -153,13 +150,8 @@ const styles = StyleSheet.create({ if (__DEV__) { if (!global.__RCTProfileIsProfiling) { - if (global.__reactExperimentalLogBox) { - const LogBox = require('../LogBox/LogBox'); - LogBox.install(); - } else { - const YellowBox = require('../YellowBox/YellowBox'); - YellowBox.install(); - } + const YellowBox = require('../YellowBox/YellowBox'); + YellowBox.install(); } } diff --git a/Libraries/ReactNative/AppRegistry.js b/Libraries/ReactNative/AppRegistry.js index 682db4682a0071..06a93340bc8606 100644 --- a/Libraries/ReactNative/AppRegistry.js +++ b/Libraries/ReactNative/AppRegistry.js @@ -181,13 +181,15 @@ const AppRegistry = { * See http://facebook.github.io/react-native/docs/appregistry.html#runapplication */ runApplication(appKey: string, appParameters: any): void { - const msg = - 'Running "' + appKey + '" with ' + JSON.stringify(appParameters); - infoLog(msg); - BugReporting.addSource( - 'AppRegistry.runApplication' + runCount++, - () => msg, - ); + if (appKey !== 'LogBox') { + const msg = + 'Running "' + appKey + '" with ' + JSON.stringify(appParameters); + infoLog(msg); + BugReporting.addSource( + 'AppRegistry.runApplication' + runCount++, + () => msg, + ); + } invariant( runnables[appKey] && runnables[appKey].run, `"${appKey}" has not been registered. This can happen if:\n` + diff --git a/Libraries/YellowBox/YellowBox.js b/Libraries/YellowBox/YellowBox.js index b91e88f644741a..ad1b37ab58b1fb 100644 --- a/Libraries/YellowBox/YellowBox.js +++ b/Libraries/YellowBox/YellowBox.js @@ -12,12 +12,7 @@ const React = require('react'); -import type {Category} from './Data/YellowBoxCategory'; -import type { - Registry, - Subscription, - IgnorePattern, -} from './Data/YellowBoxRegistry'; +import type {Registry, IgnorePattern} from './Data/YellowBoxRegistry'; import * as LogBoxData from '../LogBox/Data/LogBoxData'; type Props = $ReadOnly<{||}>; type State = {| @@ -47,14 +42,19 @@ let YellowBox; if (__DEV__) { const Platform = require('../Utilities/Platform'); const RCTLog = require('../Utilities/RCTLog'); - const YellowBoxList = require('./UI/YellowBoxList'); + const YellowBoxContainer = require('./YellowBoxContainer').default; + const LogBox = require('../LogBox/LogBox'); const YellowBoxRegistry = require('./Data/YellowBoxRegistry'); + const LogBoxNotificationContainer = require('../LogBox/LogBoxNotificationContainer') + .default; // YellowBox needs to insert itself early, // in order to access the component stacks appended by React DevTools. const {error, warn} = console; let errorImpl = error; let warnImpl = warn; + let _isLogBoxEnabled = false; + let _isInstalled = false; (console: any).error = function(...args) { errorImpl(...args); }; @@ -70,6 +70,11 @@ if (__DEV__) { } static install(): void { + _isInstalled = true; + if (_isLogBoxEnabled) { + LogBox.install(); + return; + } errorImpl = function(...args) { error.call(console, ...args); // Show YellowBox for the `warning` module. @@ -102,46 +107,39 @@ if (__DEV__) { } static uninstall(): void { + if (_isLogBoxEnabled) { + LogBox.uninstall(); + return; + } + _isInstalled = false; errorImpl = error; warnImpl = warn; delete (console: any).disableYellowBox; } - _subscription: ?Subscription; - - state = { - registry: null, - }; + static __unstable_enableLogBox(): void { + if (_isInstalled) { + throw new Error( + 'LogBox must be enabled before AppContainer is required so that it can properly wrap the console methods.\n\nPlease enable LogBox earlier in your app.\n\n', + ); + } + _isLogBoxEnabled = true; - render(): React.Node { - // TODO: Ignore warnings that fire when rendering `YellowBox` itself. - return this.state.registry == null ? null : ( - - ); + // TODO: Temporary hack to prevent cycles with the ExceptionManager. + global.__unstable_isLogBoxEnabled = true; } - componentDidMount(): void { - this._subscription = YellowBoxRegistry.observe(registry => { - this.setState({registry}); - }); + static __unstable_isLogBoxEnabled(): boolean { + return !!_isLogBoxEnabled; } - componentWillUnmount(): void { - if (this._subscription != null) { - this._subscription.unsubscribe(); + render(): React.Node { + if (_isLogBoxEnabled) { + return ; } - } - - _handleDismiss = (category: Category): void => { - YellowBoxRegistry.delete(category); - }; - _handleDismissAll(): void { - YellowBoxRegistry.clear(); + // TODO: Ignore warnings that fire when rendering `YellowBox` itself. + return ; } }; @@ -162,6 +160,13 @@ if (__DEV__) { // Do nothing. } + static __unstable_enableLogBox(): void { + // Do nothing. + } + static __unstable_isLogBoxEnabled(): boolean { + return false; + } + render(): React.Node { return null; } @@ -172,5 +177,7 @@ module.exports = (YellowBox: Class> & { ignoreWarnings($ReadOnlyArray): void, install(): void, uninstall(): void, + __unstable_enableLogBox(): void, + __unstable_isLogBoxEnabled(): boolean, ... }); diff --git a/Libraries/YellowBox/YellowBoxContainer.js b/Libraries/YellowBox/YellowBoxContainer.js new file mode 100644 index 00000000000000..b66bdd2d274119 --- /dev/null +++ b/Libraries/YellowBox/YellowBoxContainer.js @@ -0,0 +1,65 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + * @format + */ + +'use strict'; + +const React = require('react'); + +import type {Category} from './Data/YellowBoxCategory'; +import type {Registry, Subscription} from './Data/YellowBoxRegistry'; + +type Props = $ReadOnly<{||}>; +type State = $ReadOnly<{| + registry: ?Registry, +|}>; + +const YellowBoxList = require('./UI/YellowBoxList'); +const YellowBoxRegistry = require('./Data/YellowBoxRegistry'); + +class YellowBoxContainer extends React.Component { + _subscription: ?Subscription; + + state: State = { + registry: null, + }; + + render(): React.Node { + // TODO: Ignore warnings that fire when rendering `YellowBox` itself. + return this.state.registry == null ? null : ( + + ); + } + + componentDidMount(): void { + this._subscription = YellowBoxRegistry.observe(registry => { + this.setState({registry}); + }); + } + + componentWillUnmount(): void { + if (this._subscription != null) { + this._subscription.unsubscribe(); + } + } + + _handleDismiss = (category: Category): void => { + YellowBoxRegistry.delete(category); + }; + + _handleDismissAll(): void { + YellowBoxRegistry.clear(); + } +} + +export default YellowBoxContainer; diff --git a/Libraries/YellowBox/__tests__/YellowBox-test.js b/Libraries/YellowBox/__tests__/YellowBox-test.js index 8969f27ccb41d7..acd7e67f98e29a 100644 --- a/Libraries/YellowBox/__tests__/YellowBox-test.js +++ b/Libraries/YellowBox/__tests__/YellowBox-test.js @@ -11,8 +11,16 @@ 'use strict'; +import * as React from 'react'; const YellowBox = require('../YellowBox'); const YellowBoxRegistry = require('../Data/YellowBoxRegistry'); +const LogBoxData = require('../../LogBox/Data/LogBoxData'); +const render = require('../../../jest/renderer'); + +jest.mock('../../LogBox/LogBoxNotificationContainer', () => ({ + __esModule: true, + default: 'LogBoxNotificationContainer', +})); describe('YellowBox', () => { const {error, warn} = console; @@ -74,4 +82,49 @@ describe('YellowBox', () => { (console: any).error('Warning: ...'); expect(YellowBoxRegistry.add).toBeCalled(); }); + + it('if LogBox is enabled, installs and uninstalls LogBox', () => { + jest.mock('../../LogBox/Data/LogBoxData'); + jest.mock('../Data/YellowBoxRegistry'); + + YellowBox.__unstable_enableLogBox(); + YellowBox.install(); + + (console: any).warn('Some warning'); + expect(YellowBoxRegistry.add).not.toBeCalled(); + expect(LogBoxData.addLog).toBeCalled(); + expect(YellowBox.__unstable_isLogBoxEnabled()).toBe(true); + + YellowBox.uninstall(); + (LogBoxData.addLog: any).mockClear(); + + (console: any).warn('Some warning'); + expect(YellowBoxRegistry.add).not.toBeCalled(); + expect(LogBoxData.addLog).not.toBeCalled(); + expect(YellowBox.__unstable_isLogBoxEnabled()).toBe(true); + }); + + it('throws if LogBox is enabled after YellowBox is installed', () => { + jest.mock('../Data/YellowBoxRegistry'); + + YellowBox.install(); + + expect(() => YellowBox.__unstable_enableLogBox()).toThrow( + 'LogBox must be enabled before AppContainer is required so that it can properly wrap the console methods.\n\nPlease enable LogBox earlier in your app.\n\n', + ); + }); + + it('should render YellowBoxContainer by default', () => { + const output = render.shallowRender(); + + expect(output).toMatchSnapshot(); + }); + + it('should render LogBoxNotificationContainer when LogBox is enabled', () => { + YellowBox.__unstable_enableLogBox(); + + const output = render.shallowRender(); + + expect(output).toMatchSnapshot(); + }); }); diff --git a/Libraries/YellowBox/__tests__/__snapshots__/YellowBox-test.js.snap b/Libraries/YellowBox/__tests__/__snapshots__/YellowBox-test.js.snap new file mode 100644 index 00000000000000..974f032a963f2e --- /dev/null +++ b/Libraries/YellowBox/__tests__/__snapshots__/YellowBox-test.js.snap @@ -0,0 +1,5 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`YellowBox should render LogBoxNotificationContainer when LogBox is enabled 1`] = ``; + +exports[`YellowBox should render YellowBoxContainer by default 1`] = ``; diff --git a/index.js b/index.js index ec131de9ec2c23..e9b7b659f60aee 100644 --- a/index.js +++ b/index.js @@ -438,6 +438,9 @@ module.exports = { get unstable_RootTagContext(): RootTagContext { return require('./Libraries/ReactNative/RootTagContext'); }, + get unstable_enableLogBox(): () => void { + return require('./Libraries/YellowBox/YellowBox').__unstable_enableLogBox; + }, // Prop Types get ColorPropType(): DeprecatedColorPropType {