Centralize public access to LogBox in LogBox module

Summary: Some components are using `LogBoxData` directly, forcing logs to be shown on the screen even when LogBox is uninstalled. This changes all accesses to `LogBoxData` to go through `LogBox` so `uninstall` is used correctly. It also changes when LogBox is installed, moving it from `AppContainer` to `InitializeCore` (which happens earlier) so we can capture more logs in LogBox. Changelog: [General][Changed] Initialized LogBox earlier and centralized access in LogBox module Reviewed By: rickhanlonii Differential Revision: D27999361 fbshipit-source-id: 1115ef6b71e08cc33743d205da0064fbe9a74a0e
facebook · May 4, 2021 · 8abe737 · 8abe737
1 parent ab45509
commit 8abe737
Show file tree

Hide file tree

Showing 7 changed files with 238 additions and 66 deletions.
diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js
@@ -105,8 +105,8 @@ function reportException(
     }
 
     if (__DEV__ && isHandledByLogBox) {
-      const LogBoxData = require('../LogBox/Data/LogBoxData');
-      LogBoxData.addException({
+      const LogBox = require('../LogBox/LogBox');
+      LogBox.addException({
         ...data,
         isComponentError: !!e.isComponentError,
       });

diff --git a/Libraries/Core/InitializeCore.js b/Libraries/Core/InitializeCore.js
@@ -43,6 +43,7 @@ require('./setUpSegmentFetcher');
 if (__DEV__) {
   require('./checkNativeVersion');
   require('./setUpDeveloperTools');
+  require('../LogBox/LogBox').install();
 }
 
 const GlobalPerformanceLogger = require('../Utilities/GlobalPerformanceLogger');

diff --git a/Libraries/LogBox/LogBox.js b/Libraries/LogBox/LogBox.js
@@ -11,56 +11,69 @@
 import Platform from '../Utilities/Platform';
 import RCTLog from '../Utilities/RCTLog';
 
-import type {IgnorePattern} from './Data/LogBoxData';
+import type {IgnorePattern, LogData} from './Data/LogBoxData';
+import type {ExtendedExceptionData} from './Data/parseLogBoxLog';
+
+export type {LogData, ExtendedExceptionData, IgnorePattern};
 
 let LogBox;
 
+interface ILogBox {
+  install(): void;
+  uninstall(): void;
+  isInstalled(): boolean;
+  ignoreLogs($ReadOnlyArray<IgnorePattern>): void;
+  ignoreAllLogs(?boolean): void;
+  clearAllLogs(): void;
+  addLog(log: LogData): void;
+  addException(error: ExtendedExceptionData): void;
+}
+
 /**
  * LogBox displays logs in the app.
  */
 if (__DEV__) {
   const LogBoxData = require('./Data/LogBoxData');
   const {parseLogBoxLog, parseInterpolation} = require('./Data/parseLogBoxLog');
 
-  // LogBox needs to insert itself early,
-  // in order to access the component stacks appended by React DevTools.
-  const {error, warn} = console;
-  let errorImpl = error.bind(console);
-  let warnImpl = warn.bind(console);
+  let originalConsoleError;
+  let originalConsoleWarn;
+  let consoleErrorImpl;
+  let consoleWarnImpl;
 
-  (console: any).error = function(...args) {
-    errorImpl(...args);
-  };
-  (console: any).warn = function(...args) {
-    warnImpl(...args);
-  };
+  let isLogBoxInstalled: boolean = false;
 
   LogBox = {
-    ignoreLogs: (patterns: $ReadOnlyArray<IgnorePattern>): void => {
-      LogBoxData.addIgnorePatterns(patterns);
-    },
+    install(): void {
+      if (isLogBoxInstalled) {
+        return;
+      }
 
-    ignoreAllLogs: (value?: ?boolean): void => {
-      LogBoxData.setDisabled(value == null ? true : value);
-    },
+      isLogBoxInstalled = true;
 
-    uninstall: (): void => {
-      errorImpl = error;
-      warnImpl = warn;
-      delete (console: any).disableLogBox;
-    },
-
-    install: (): void => {
       // Trigger lazy initialization of module.
       require('../NativeModules/specs/NativeLogBox');
 
-      errorImpl = function(...args) {
-        registerError(...args);
-      };
+      // IMPORTANT: we only overwrite `console.error` and `console.warn` once.
+      // When we uninstall we keep the same reference and only change its
+      // internal implementation
+      const isFirstInstall = originalConsoleError == null;
+      if (isFirstInstall) {
+        originalConsoleError = console.error.bind(console);
+        originalConsoleWarn = console.warn.bind(console);
+
+        // $FlowExpectedError[cannot-write]
+        console.error = (...args) => {
+          consoleErrorImpl(...args);
+        };
+        // $FlowExpectedError[cannot-write]
+        console.warn = (...args) => {
+          consoleWarnImpl(...args);
+        };
+      }
 
-      warnImpl = function(...args) {
-        registerWarning(...args);
-      };
+      consoleErrorImpl = registerError;
+      consoleWarnImpl = registerWarning;
 
       if ((console: any).disableYellowBox === true) {
         LogBoxData.setDisabled(true);
@@ -88,6 +101,50 @@ if (__DEV__) {
         registerWarning(...args);
       });
     },
+
+    uninstall(): void {
+      if (!isLogBoxInstalled) {
+        return;
+      }
+
+      isLogBoxInstalled = false;
+
+      // IMPORTANT: we don't re-assign to `console` in case the method has been
+      // decorated again after installing LogBox. E.g.:
+      // Before uninstalling: original > LogBox > OtherErrorHandler
+      // After uninstalling:  original > LogBox (noop) > OtherErrorHandler
+      consoleErrorImpl = originalConsoleError;
+      consoleWarnImpl = originalConsoleWarn;
+      delete (console: any).disableLogBox;
+    },
+
+    isInstalled(): boolean {
+      return isLogBoxInstalled;
+    },
+
+    ignoreLogs(patterns: $ReadOnlyArray<IgnorePattern>): void {
+      LogBoxData.addIgnorePatterns(patterns);
+    },
+
+    ignoreAllLogs(value?: ?boolean): void {
+      LogBoxData.setDisabled(value == null ? true : value);
+    },
+
+    clearAllLogs(): void {
+      LogBoxData.clear();
+    },
+
+    addLog(log: LogData): void {
+      if (isLogBoxInstalled) {
+        LogBoxData.addLog(log);
+      }
+    },
+
+    addException(error: ExtendedExceptionData): void {
+      if (isLogBoxInstalled) {
+        LogBoxData.addException(error);
+      }
+    },
   };
 
   const isRCTLogAdviceWarning = (...args) => {
@@ -103,7 +160,7 @@ if (__DEV__) {
   const registerWarning = (...args): void => {
     // Let warnings within LogBox itself fall through.
     if (LogBoxData.isLogBoxErrorMessage(String(args[0]))) {
-      error.call(console, ...args);
+      originalConsoleError(...args);
       return;
     }
 
@@ -113,7 +170,7 @@ if (__DEV__) {
 
         if (!LogBoxData.isMessageIgnored(message.content)) {
           // Be sure to pass LogBox warnings through.
-          warn.call(console, ...args);
+          originalConsoleWarn(...args);
 
           LogBoxData.addLog({
             level: 'warn',
@@ -131,7 +188,7 @@ if (__DEV__) {
   const registerError = (...args): void => {
     // Let errors within LogBox itself fall through.
     if (LogBoxData.isLogBoxErrorMessage(args[0])) {
-      error.call(console, ...args);
+      originalConsoleError(...args);
       return;
     }
 
@@ -144,7 +201,7 @@ if (__DEV__) {
         //
         // The 'warning' module needs to be handled here because React internally calls
         // `console.error('Warning: ')` with the component stack already included.
-        error.call(console, ...args);
+        originalConsoleError(...args);
         return;
       }
 
@@ -169,7 +226,7 @@ if (__DEV__) {
         // Interpolate the message so they are formatted for adb and other CLIs.
         // This is different than the message.content above because it includes component stacks.
         const interpolated = parseInterpolation(args);
-        error.call(console, interpolated.message.content);
+        originalConsoleError(interpolated.message.content);
 
         LogBoxData.addLog({
           level,
@@ -184,28 +241,38 @@ if (__DEV__) {
   };
 } else {
   LogBox = {
-    ignoreLogs: (patterns: $ReadOnlyArray<IgnorePattern>): void => {
+    install(): void {
+      // Do nothing.
+    },
+
+    uninstall(): void {
+      // Do nothing.
+    },
+
+    isInstalled(): boolean {
+      return false;
+    },
+
+    ignoreLogs(patterns: $ReadOnlyArray<IgnorePattern>): void {
+      // Do nothing.
+    },
+
+    ignoreAllLogs(value?: ?boolean): void {
       // Do nothing.
     },
 
-    ignoreAllLogs: (value?: ?boolean): void => {
+    clearAllLogs(): void {
       // Do nothing.
     },
 
-    install: (): void => {
+    addLog(log: LogData): void {
       // Do nothing.
     },
 
-    uninstall: (): void => {
+    addException(error: ExtendedExceptionData): void {
       // Do nothing.
     },
   };
 }
 
-module.exports = (LogBox: {
-  ignoreLogs($ReadOnlyArray<IgnorePattern>): void,
-  ignoreAllLogs(?boolean): void,
-  install(): void,
-  uninstall(): void,
-  ...
-});
+module.exports = (LogBox: ILogBox);
diff --git a/Libraries/LogBox/__tests__/LogBox-test.js b/Libraries/LogBox/__tests__/LogBox-test.js
@@ -31,11 +31,13 @@ function mockFilterResult(returnValues) {
 
 describe('LogBox', () => {
   const {error, warn} = console;
+  let consoleError;
+  let consoleWarn;
 
   beforeEach(() => {
     jest.resetModules();
-    console.error = jest.fn();
-    console.warn = jest.fn();
+    console.error = consoleError = jest.fn();
+    console.warn = consoleWarn = jest.fn();
     console.disableYellowBox = false;
   });
 
@@ -276,4 +278,107 @@ describe('LogBox', () => {
       },
     });
   });
+
+  it('ignores console methods after uninstalling', () => {
+    jest.mock('../Data/LogBoxData');
+
+    LogBox.install();
+    LogBox.uninstall();
+
+    console.log('Test');
+    console.warn('Test');
+    console.error('Test');
+
+    expect(LogBoxData.addLog).not.toHaveBeenCalled();
+  });
+
+  it('does not add logs after uninstalling', () => {
+    jest.mock('../Data/LogBoxData');
+
+    LogBox.install();
+    LogBox.uninstall();
+
+    LogBox.addLog({
+      level: 'warn',
+      category: 'test',
+      message: {content: 'Some warning', substitutions: []},
+      componentStack: [],
+    });
+
+    expect(LogBoxData.addLog).not.toHaveBeenCalled();
+  });
+
+  it('does not add exceptions after uninstalling', () => {
+    jest.mock('../Data/LogBoxData');
+
+    LogBox.install();
+    LogBox.uninstall();
+
+    LogBox.addException({
+      message: 'Some error',
+      originalMessage: null,
+      name: 'Error',
+      componentStack: null,
+      stack: [],
+      id: 12,
+      isFatal: true,
+      isComponentError: false,
+    });
+
+    expect(LogBoxData.addException).not.toHaveBeenCalled();
+  });
+
+  it('preserves decorations of console.error after installing/uninstalling', () => {
+    LogBox.install();
+
+    const originalConsoleError = console.error;
+    console.error = message => {
+      originalConsoleError('Custom: ' + message);
+    };
+
+    console.error('before uninstalling');
+
+    expect(consoleError).toHaveBeenCalledWith('Custom: before uninstalling');
+
+    LogBox.uninstall();
+
+    console.error('after uninstalling');
+
+    expect(consoleError).toHaveBeenCalledWith('Custom: after uninstalling');
+
+    LogBox.install();
+
+    console.error('after installing for the second time');
+
+    expect(consoleError).toHaveBeenCalledWith(
+      'Custom: after installing for the second time',
+    );
+  });
+
+  it('preserves decorations of console.warn after installing/uninstalling', () => {
+    LogBox.install();
+
+    const originalConsoleWarn = console.warn;
+    console.warn = message => {
+      originalConsoleWarn('Custom: ' + message);
+    };
+
+    console.warn('before uninstalling');
+
+    expect(consoleWarn).toHaveBeenCalledWith('Custom: before uninstalling');
+
+    LogBox.uninstall();
+
+    console.warn('after uninstalling');
+
+    expect(consoleWarn).toHaveBeenCalledWith('Custom: after uninstalling');
+
+    LogBox.install();
+
+    console.warn('after installing for the second time');
+
+    expect(consoleWarn).toHaveBeenCalledWith(
+      'Custom: after installing for the second time',
+    );
+  });
 });