LogBox - Minor error message formatting fixes

Summary:
Adds better handling of component stacks for warnings and clarifies why we ignore `(ADVICE)` warnings.

Changelog: [Internal]

Reviewed By: cpojer

Differential Revision: D18575693

fbshipit-source-id: f4eacbf74c62cd079f2c441951849fb03e0941dd

Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js

@@ -108,6 +108,33 @@ describe('parseLogBoxLog', () => {
     });
   });
 
+  it('detects a component stack in an interpolated warning', () => {
+    expect(
+      parseLogBoxLog([
+        'Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s%s',
+        '\n\nCheck the render method of `Container(Component)`.',
+        '\n    in MyComponent (at filename.js:1)\n    in MyOtherComponent (at filename2.js:1)',
+      ]),
+    ).toEqual({
+      componentStack: [
+        {component: 'MyComponent', location: 'filename.js:1'},
+        {component: 'MyOtherComponent', location: 'filename2.js:1'},
+      ],
+      category:
+        'Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s',
+      message: {
+        content:
+          'Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?\n\nCheck the render method of `Container(Component)`.',
+        substitutions: [
+          {
+            length: 52,
+            offset: 129,
+          },
+        ],
+      },
+    });
+  });
+
   it('detects a component stack in the second argument', () => {
     expect(
       parseLogBoxLog([

Libraries/LogBox/Data/parseLogBoxLog.js

@@ -187,15 +187,27 @@ export function parseLogBoxLog(
   category: Category,
   message: Message,
 |} {
-  // This detects a very narrow case of a simple log string,
-  // with a component stack appended by React DevTools.
-  // In this case, we extract the component stack,
-  // because LogBox formats those pleasantly.
-  // If there are other substitutions or formatting,
-  // this could potentially corrupt the data, but there
-  // are currently no known cases of that happening.
-  let componentStack = [];
+  const message = args[0];
   let argsWithoutComponentStack = [];
+  let componentStack = [];
+
+  // Extract component stack from warnings like "Some warning%s".
+  if (
+    typeof message === 'string' &&
+    message.slice(-2) === '%s' &&
+    args.length > 0
+  ) {
+    const lastArg = args[args.length - 1];
+    // Does it look like React component stack? "   in ..."
+    if (typeof lastArg === 'string' && /\s{4}in/.test(lastArg)) {
+      argsWithoutComponentStack = args.slice(0, -1);
+      argsWithoutComponentStack[0] = message.slice(0, -2);
+      componentStack = parseComponentStack(lastArg);
+    }
+  }
+
+  if (componentStack.length === 0) {
+    // Try finding the component stack elsewhere.
     for (const arg of args) {
       if (typeof arg === 'string' && /^\n {4}in/.exec(arg)) {
         componentStack = parseComponentStack(arg);
@@ -203,6 +215,7 @@ export function parseLogBoxLog(
         argsWithoutComponentStack.push(arg);
       }
     }
+  }
 
   return {
     ...parseCategory(argsWithoutComponentStack),

Libraries/LogBox/LogBox.js

@@ -150,10 +150,19 @@ if (__DEV__) {
     }
   };
 
+  const isRCTLogAdviceWarning = (...args) => {
+    // RCTLogAdvice is a native logging function designed to show users
+    // a message in the console, but not show it to them in Logbox.
+    return typeof args[0] === 'string' && args[0].startsWith('(ADVICE)');
+  };
+
+  const isWarningModuleWarning = (...args) => {
+    return typeof args[0] === 'string' && args[0].startsWith('Warning: ');
+  };
+
   const registerWarning = (...args): void => {
     try {
-      // This is carried over from the old YellowBox, but it is not clear why.
-      if (typeof args[0] !== 'string' || !args[0].startsWith('(ADVICE)')) {
+      if (!isRCTLogAdviceWarning(...args)) {
         const {category, message, componentStack} = parseLogBoxLog(args);
 
         if (!LogBoxData.isMessageIgnored(message.content)) {
@@ -177,8 +186,8 @@ if (__DEV__) {
 
   const registerError = (...args): void => {
     try {
+      if (!isWarningModuleWarning(...args)) {
         // Only show LogBox for the `warning` module, otherwise pass through and skip.
-      if (typeof args[0] !== 'string' || !args[0].startsWith('Warning: ')) {
         error.call(console, ...args);
         return;
       }