Add codeFrame to each warning and error in LogBox

Summary: This diff changes the LogBox to show the code frame for the first non-collapsed stack frame. Let me know what you think about this change! Changelog: [Internal] LogBox changes Reviewed By: rickhanlonii Differential Revision: D18372456 fbshipit-source-id: ddf6d6c53ab28d11d8355f4cb1cb071a00a7366e
zain148 · Nov 8, 2019 · fa4f23e · fa4f23e
1 parent 5eddf1d
commit fa4f23e
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 24 deletions.
diff --git a/Libraries/Core/Devtools/symbolicateStackTrace.js b/Libraries/Core/Devtools/symbolicateStackTrace.js
@@ -19,13 +19,27 @@ let fetch;
 
 import type {StackFrame} from '../NativeExceptionsManager';
 
+export type CodeFrame = $ReadOnly<{|
+  content: string,
+  location: {
+    row: number,
+    column: number,
+  },
+  fileName: string,
+|}>;
+
+export type SymbolicatedStackTrace = $ReadOnly<{|
+  stack: Array<StackFrame>,
+  codeFrame: ?CodeFrame,
+|}>;
+
 function isSourcedFromDisk(sourcePath: string): boolean {
   return !/^http/.test(sourcePath) && /[\\/]/.test(sourcePath);
 }
 
 async function symbolicateStackTrace(
   stack: Array<StackFrame>,
-): Promise<Array<StackFrame>> {
+): Promise<SymbolicatedStackTrace> {
   // RN currently lazy loads whatwg-fetch using a custom fetch module, which,
   // when called for the first time, requires and re-exports 'whatwg-fetch'.
   // However, when a dependency of the project tries to require whatwg-fetch
@@ -74,8 +88,7 @@ async function symbolicateStackTrace(
     method: 'POST',
     body: JSON.stringify({stack: stackCopy}),
   });
-  const json = await response.json();
-  return json.stack;
+  return await response.json();
 }
 
 module.exports = symbolicateStackTrace;
diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js
@@ -116,7 +116,7 @@ function reportException(e: ExtendedError, isFatal: boolean) {
       }
       const symbolicateStackTrace = require('./Devtools/symbolicateStackTrace');
       symbolicateStackTrace(stack)
-        .then(prettyStack => {
+        .then(({stack: prettyStack}) => {
           if (prettyStack) {
             NativeExceptionsManager.updateExceptionMessage(
               data.message,

diff --git a/Libraries/Core/__tests__/ExceptionsManager-test.js b/Libraries/Core/__tests__/ExceptionsManager-test.js
@@ -39,11 +39,13 @@ describe('ExceptionsManager', () => {
       };
     });
     // Make symbolication a no-op.
-    jest.mock('../Devtools/symbolicateStackTrace', () => {
-      return async function symbolicateStackTrace(stack) {
-        return stack;
-      };
-    });
+    jest.mock(
+      '../Devtools/symbolicateStackTrace',
+      () =>
+        async function symbolicateStackTrace(stack) {
+          return {stack};
+        },
+    );
     jest.spyOn(console, 'error').mockImplementation(() => {});
     ReactFiberErrorDialog = require('../ReactFiberErrorDialog');
     NativeExceptionsManager = require('../NativeExceptionsManager').default;

diff --git a/Libraries/LogBox/Data/LogBoxLog.js b/Libraries/LogBox/Data/LogBoxLog.js
@@ -82,13 +82,33 @@ class LogBoxLog {
     let aborted = false;
 
     if (this.symbolicated.status !== 'COMPLETE') {
-      const updateStatus = (error: ?Error, stack: ?Stack): void => {
+      const updateStatus = (
+        error: ?Error,
+        stack: ?Stack,
+        codeFrame: ?CodeFrame,
+      ): void => {
         if (error != null) {
-          this.symbolicated = {error, stack: null, status: 'FAILED'};
+          this.symbolicated = {
+            error,
+            stack: null,
+            status: 'FAILED',
+          };
         } else if (stack != null) {
-          this.symbolicated = {error: null, stack, status: 'COMPLETE'};
+          if (codeFrame) {
+            this.codeFrame = codeFrame;
+          }
+
+          this.symbolicated = {
+            error: null,
+            stack,
+            status: 'COMPLETE',
+          };
         } else {
-          this.symbolicated = {error: null, stack: null, status: 'PENDING'};
+          this.symbolicated = {
+            error: null,
+            stack: null,
+            status: 'PENDING',
+          };
         }
         if (!aborted) {
           if (callback != null) {
@@ -99,8 +119,8 @@ class LogBoxLog {
 
       updateStatus(null, null);
       LogBoxSymbolication.symbolicate(this.stack).then(
-        stack => {
-          updateStatus(null, stack);
+        data => {
+          updateStatus(null, data?.stack, data?.codeFrame);
         },
         error => {
           updateStatus(error, null);

diff --git a/Libraries/LogBox/Data/LogBoxSymbolication.js b/Libraries/LogBox/Data/LogBoxSymbolication.js
@@ -13,15 +13,19 @@
 import symbolicateStackTrace from '../../Core/Devtools/symbolicateStackTrace';
 
 import type {StackFrame} from '../../Core/NativeExceptionsManager';
+import type {SymbolicatedStackTrace} from '../../Core/Devtools/symbolicateStackTrace';
 
 export type Stack = Array<StackFrame>;
 
-const cache: Map<Stack, Promise<Stack>> = new Map();
+const cache: Map<Stack, Promise<SymbolicatedStackTrace>> = new Map();
 
 /**
  * Sanitize because sometimes, `symbolicateStackTrace` gives us invalid values.
  */
-const sanitize = (maybeStack: mixed): Stack => {
+const sanitize = ({
+  stack: maybeStack,
+  codeFrame,
+}: SymbolicatedStackTrace): SymbolicatedStackTrace => {
   if (!Array.isArray(maybeStack)) {
     throw new Error('Expected stack to be an array.');
   }
@@ -58,14 +62,14 @@ const sanitize = (maybeStack: mixed): Stack => {
       collapse,
     });
   }
-  return stack;
+  return {stack, codeFrame};
 };
 
 export function deleteStack(stack: Stack): void {
   cache.delete(stack);
 }
 
-export function symbolicate(stack: Stack): Promise<Stack> {
+export function symbolicate(stack: Stack): Promise<SymbolicatedStackTrace> {
   let promise = cache.get(stack);
   if (promise == null) {
     promise = symbolicateStackTrace(stack).then(sanitize);

diff --git a/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js b/Libraries/LogBox/Data/__tests__/LogBoxLog-test.js
@@ -12,6 +12,7 @@
 'use strict';
 
 import type {StackFrame} from '../../../Core/NativeExceptionsManager';
+import type {SymbolicatedStackTrace} from '../../../Core/Devtools/symbolicateStackTrace';
 
 jest.mock('../LogBoxSymbolication', () => {
   return {__esModule: true, symbolicate: jest.fn(), deleteStack: jest.fn()};
@@ -35,7 +36,7 @@ function getLogBoxLog() {
 function getLogBoxSymbolication(): {|
   symbolicate: JestMockFn<
     $ReadOnlyArray<Array<StackFrame>>,
-    Promise<Array<StackFrame>>,
+    Promise<SymbolicatedStackTrace>,
   >,
 |} {
   return (require('../LogBoxSymbolication'): any);
@@ -53,9 +54,10 @@ describe('LogBoxLog', () => {
   beforeEach(() => {
     jest.resetModules();
 
-    getLogBoxSymbolication().symbolicate.mockImplementation(async stack =>
-      createStack(stack.map(frame => `S(${frame.methodName})`)),
-    );
+    getLogBoxSymbolication().symbolicate.mockImplementation(async stack => ({
+      stack: createStack(stack.map(frame => `S(${frame.methodName})`)),
+      codeFrame: null,
+    }));
   });
 
   it('creates a LogBoxLog object', () => {

diff --git a/Libraries/YellowBox/Data/YellowBoxSymbolication.js b/Libraries/YellowBox/Data/YellowBoxSymbolication.js
@@ -13,6 +13,7 @@
 const symbolicateStackTrace = require('../../Core/Devtools/symbolicateStackTrace');
 
 import type {StackFrame} from '../../Core/NativeExceptionsManager';
+import type {SymbolicatedStackTrace} from '../../Core/Devtools/symbolicateStackTrace';
 
 type CacheKey = string;
 
@@ -45,7 +46,8 @@ const getCacheKey = (stack: Stack): CacheKey => {
 /**
  * Sanitize because sometimes, `symbolicateStackTrace` gives us invalid values.
  */
-const sanitize = (maybeStack: mixed): Stack => {
+const sanitize = (data: SymbolicatedStackTrace): Stack => {
+  const maybeStack = data?.stack;
   if (!Array.isArray(maybeStack)) {
     throw new Error('Expected stack to be an array.');
   }