LogBox - lazily initialize on iOS, use sync APIs

Summary: Update LogBox on iOS to lazily initialize, using a synchronous RCTSurface, behind RCTSharedApplication checks. This results in faster of LogBox, without keeping around a long lived window in the background, and only used when LogBox is used. On Android, we still start the react app in the background but we create a dialog when it's shown and then destroy it when it's hidden. Once we have the sync APIs on android we can update it to use the same strategy. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D18925538 fbshipit-source-id: 1a72c39aa0fc26c8ba657d36c7fa7bc0ae777eb9
facebook · Dec 13, 2019 · 586d55d · 586d55d
1 parent 19bf202
commit 586d55d
Show file tree

Hide file tree

Showing 7 changed files with 151 additions and 166 deletions.
diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js
@@ -22,7 +22,7 @@ import type {
 } from './parseLogBoxLog';
 import parseErrorStack from '../../Core/Devtools/parseErrorStack';
 import type {ExtendedError} from '../../Core/Devtools/parseErrorStack';
-
+import NativeLogBox from '../../NativeModules/specs/NativeLogBox';
 export type LogBoxLogs = Set<LogBoxLog>;
 export type LogData = $ReadOnly<{|
   level: LogLevel,
@@ -86,20 +86,6 @@ const LOGBOX_ERROR_MESSAGE =
   'An error was thrown when attempting to render log messages via LogBox.';
 
 function getNextState() {
-  const logArray = Array.from(logs);
-  let index = logArray.length - 1;
-  while (index >= 0) {
-    // The latest syntax error is selected and displayed before all other logs.
-    if (logArray[index].level === 'syntax') {
-      return {
-        logs,
-        isDisabled: _isDisabled,
-        selectedLogIndex: index,
-      };
-    }
-    index -= 1;
-  }
-
   return {
     logs,
     isDisabled: _isDisabled,
@@ -175,9 +161,10 @@ function appendNewLog(newLog) {
     let addPendingLog = () => {
       logs.add(newLog);
       if (_selectedIndex <= 0) {
-        _selectedIndex = logs.size - 1;
+        setSelectedLog(logs.size - 1);
+      } else {
+        handleUpdate();
       }
-      handleUpdate();
       addPendingLog = null;
     };
 
@@ -196,6 +183,9 @@ function appendNewLog(newLog) {
         handleUpdate();
       }
     });
+  } else if (newLog.level === 'syntax') {
+    logs.add(newLog);
+    setSelectedLog(logs.size - 1);
   } else {
     logs.add(newLog);
     handleUpdate();
@@ -259,21 +249,42 @@ export function symbolicateLogLazy(log: LogBoxLog) {
 export function clear(): void {
   if (logs.size > 0) {
     logs = new Set();
-    _selectedIndex = -1;
-    handleUpdate();
+    setSelectedLog(-1);
   }
 }
 
-export function setSelectedLog(index: number): void {
-  _selectedIndex = index;
+export function setSelectedLog(proposedNewIndex: number): void {
+  const oldIndex = _selectedIndex;
+  let newIndex = proposedNewIndex;
+
+  const logArray = Array.from(logs);
+  let index = logArray.length - 1;
+  while (index >= 0) {
+    // The latest syntax error is selected and displayed before all other logs.
+    if (logArray[index].level === 'syntax') {
+      newIndex = index;
+      break;
+    }
+    index -= 1;
+  }
+  _selectedIndex = newIndex;
   handleUpdate();
+  if (NativeLogBox) {
+    setTimeout(() => {
+      if (oldIndex < 0 && newIndex >= 0) {
+        NativeLogBox.show();
+      } else if (oldIndex >= 0 && newIndex < 0) {
+        NativeLogBox.hide();
+      }
+    }, 0);
+  }
 }
 
 export function clearWarnings(): void {
   const newLogs = Array.from(logs).filter(log => log.level !== 'warn');
   if (newLogs.length !== logs.size) {
     logs = new Set(newLogs);
-    _selectedIndex = -1;
+    setSelectedLog(-1);
     handleUpdate();
   }
 }
@@ -284,8 +295,7 @@ export function clearErrors(): void {
   );
   if (newLogs.length !== logs.size) {
     logs = new Set(newLogs);
-    _selectedIndex = -1;
-    handleUpdate();
+    setSelectedLog(-1);
   }
 }
 

diff --git a/Libraries/LogBox/LogBoxInspectorContainer.js b/Libraries/LogBox/LogBoxInspectorContainer.js
@@ -15,43 +15,25 @@ import {View, StyleSheet} from 'react-native';
 import * as LogBoxData from './Data/LogBoxData';
 import LogBoxInspector from './UI/LogBoxInspector';
 import type LogBoxLog from './Data/LogBoxLog';
-import NativeLogBox from '../NativeModules/specs/NativeLogBox';
 
 type Props = $ReadOnly<{|
   logs: $ReadOnlyArray<LogBoxLog>,
   selectedLogIndex: number,
   isDisabled?: ?boolean,
 |}>;
 
-function NativeLogBoxVisibility(props) {
-  React.useLayoutEffect(() => {
-    if (NativeLogBox) {
-      if (props.visible) {
-        // Schedule this to try and prevent flashing the old state.
-        setTimeout(() => NativeLogBox.show(), 10);
-      } else {
-        NativeLogBox.hide();
-      }
-    }
-  }, [props.visible]);
-
-  return props.children;
-}
-
 export class _LogBoxInspectorContainer extends React.Component<Props> {
   render(): React.Node {
     return (
-      <NativeLogBoxVisibility visible={this.props.selectedLogIndex >= 0}>
-        <View style={StyleSheet.absoluteFill}>
-          <LogBoxInspector
-            onDismiss={this._handleDismiss}
-            onMinimize={this._handleMinimize}
-            onChangeSelectedIndex={this._handleSetSelectedLog}
-            logs={this.props.logs}
-            selectedIndex={this.props.selectedLogIndex}
-          />
-        </View>
-      </NativeLogBoxVisibility>
+      <View style={StyleSheet.absoluteFill}>
+        <LogBoxInspector
+          onDismiss={this._handleDismiss}
+          onMinimize={this._handleMinimize}
+          onChangeSelectedIndex={this._handleSetSelectedLog}
+          logs={this.props.logs}
+          selectedIndex={this.props.selectedLogIndex}
+        />
+      </View>
     );
   }
 

diff --git a/Libraries/LogBox/UI/LogBoxInspector.js b/Libraries/LogBox/UI/LogBoxInspector.js
@@ -36,8 +36,8 @@ type Props = $ReadOnly<{|
 
 function LogBoxInspector(props: Props): React.Node {
   const {logs, selectedIndex} = props;
+  let log = logs[selectedIndex];
 
-  const log = logs[selectedIndex];
   React.useEffect(() => {
     if (log) {
       LogBoxData.symbolicateLogNow(log);

diff --git a/Libraries/LogBox/UI/LogBoxInspectorFooter.js b/Libraries/LogBox/UI/LogBoxInspectorFooter.js
@@ -101,9 +101,11 @@ const styles = StyleSheet.create({
   syntaxErrorText: {
     textAlign: 'center',
     width: '100%',
+    height: 48,
     fontSize: 14,
-    paddingTop: 15,
-    paddingBottom: 15,
+    lineHeight: 20,
+    paddingTop: 20,
+    paddingBottom: 50,
     fontStyle: 'italic',
     color: LogBoxStyle.getTextColor(0.6),
   },

diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap
@@ -29,8 +29,10 @@ exports[`LogBoxInspectorFooter should render no buttons and a message for syntax
           "color": "rgba(255, 255, 255, 0.6)",
           "fontSize": 14,
           "fontStyle": "italic",
-          "paddingBottom": 15,
-          "paddingTop": 15,
+          "height": 48,
+          "lineHeight": 20,
+          "paddingBottom": 50,
+          "paddingTop": 20,
           "textAlign": "center",
           "width": "100%",
         }

diff --git a/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap b/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap
@@ -1,66 +1,62 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`LogBoxNotificationContainer should render inspector with logs, even when disabled 1`] = `
-<NativeLogBoxVisibility
-  visible={false}
->
-  <View
-    style={
-      Object {
-        "bottom": 0,
-        "left": 0,
-        "position": "absolute",
-        "right": 0,
-        "top": 0,
-      }
+<View
+  style={
+    Object {
+      "bottom": 0,
+      "left": 0,
+      "position": "absolute",
+      "right": 0,
+      "top": 0,
     }
-  >
-    <LogBoxInspector
-      logs={
-        Array [
-          LogBoxLog {
-            "category": "Some kind of message",
-            "codeFrame": undefined,
-            "componentStack": Array [],
-            "count": 1,
-            "isComponentError": false,
-            "level": "warn",
-            "message": Object {
-              "content": "Some kind of message",
-              "substitutions": Array [],
-            },
-            "stack": Array [],
-            "symbolicated": Object {
-              "error": null,
-              "stack": null,
-              "status": "NONE",
-            },
+  }
+>
+  <LogBoxInspector
+    logs={
+      Array [
+        LogBoxLog {
+          "category": "Some kind of message",
+          "codeFrame": undefined,
+          "componentStack": Array [],
+          "count": 1,
+          "isComponentError": false,
+          "level": "warn",
+          "message": Object {
+            "content": "Some kind of message",
+            "substitutions": Array [],
+          },
+          "stack": Array [],
+          "symbolicated": Object {
+            "error": null,
+            "stack": null,
+            "status": "NONE",
           },
-          LogBoxLog {
-            "category": "Some kind of message (latest)",
-            "codeFrame": undefined,
-            "componentStack": Array [],
-            "count": 1,
-            "isComponentError": false,
-            "level": "error",
-            "message": Object {
-              "content": "Some kind of message (latest)",
-              "substitutions": Array [],
-            },
-            "stack": Array [],
-            "symbolicated": Object {
-              "error": null,
-              "stack": null,
-              "status": "NONE",
-            },
+        },
+        LogBoxLog {
+          "category": "Some kind of message (latest)",
+          "codeFrame": undefined,
+          "componentStack": Array [],
+          "count": 1,
+          "isComponentError": false,
+          "level": "error",
+          "message": Object {
+            "content": "Some kind of message (latest)",
+            "substitutions": Array [],
           },
-        ]
-      }
-      onChangeSelectedIndex={[Function]}
-      onDismiss={[Function]}
-      onMinimize={[Function]}
-      selectedIndex={-1}
-    />
-  </View>
-</NativeLogBoxVisibility>
+          "stack": Array [],
+          "symbolicated": Object {
+            "error": null,
+            "stack": null,
+            "status": "NONE",
+          },
+        },
+      ]
+    }
+    onChangeSelectedIndex={[Function]}
+    onDismiss={[Function]}
+    onMinimize={[Function]}
+    selectedIndex={-1}
+  />
+</View>
 `;