Don't wrap console methods for metro logging in Chrome debugger (#26883)

Summary: Pull Request resolved: #26883 This diff fixes an issue reported in #26788 where logs in the Chrome console were showing a different location than previous versions. In the change here, we stop wrapping the console functions to attach the metro websocket in any environment that isn't a native app environment. We do this by checking `global.nativeLoggingHook` which is bound only by native apps and not environments like the Chrome DevTools. Changelog: [General][Fixed] - Fix wrong lines logging in Chrome debugger Reviewed By: cpojer, gaearon Differential Revision: D17951707 fbshipit-source-id: f045ea9abaa8aecc6afb8eca7db9842146a3d872
facebook · Oct 21, 2019 · 42ac240 · gaearon · Oct 31, 2023 · 42ac240
1 parent 50c59aa
commit 42ac240
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 17 deletions.
diff --git a/Libraries/Core/setUpDeveloperTools.js b/Libraries/Core/setUpDeveloperTools.js
@@ -12,6 +12,10 @@
 
 import Platform from '../Utilities/Platform';
 
+declare var console: typeof console & {
+  _isPolyfilled: boolean,
+};
+
 /**
  * Sets up developer tools for React Native.
  * You can use this module directly, or just require InitializeCore.
@@ -60,25 +64,54 @@ if (__DEV__) {
       JSInspector.registerAgent(require('../JSInspector/NetworkAgent'));
     }
 
+    // Note we can't check if console is "native" because it would appear "native" in JSC and Hermes.
+    // We also can't check any properties that don't exist in the Chrome worker environment.
+    // So we check a navigator property that's set to a particular value ("Netscape") in all real browsers.
+    const isLikelyARealBrowser =
+      global.navigator != null &&
+      /*              _
+       *             | |
+       *   _ __   ___| |_ ___  ___ __ _ _ __   ___
+       *  | '_ \ / _ \ __/ __|/ __/ _` | '_ \ / _ \
+       *  | | | |  __/ |_\__ \ (_| (_| | |_) |  __/
+       *  |_| |_|\___|\__|___/\___\__,_| .__/ \___|
+       *                               | |
+       *                               |_|
+       */
+      global.navigator.appName === 'Netscape'; // Any real browser
+
     if (!Platform.isTesting) {
       const HMRClient = require('../Utilities/HMRClient');
-      [
-        'trace',
-        'info',
-        'warn',
-        'log',
-        'group',
-        'groupCollapsed',
-        'groupEnd',
-        'debug',
-      ].forEach(level => {
-        const originalFunction = console[level];
-        // $FlowFixMe Overwrite console methods
-        console[level] = function(...args) {
-          HMRClient.log(level, args);
-          originalFunction.apply(console, args);
-        };
-      });
+
+      if (console._isPolyfilled) {
+        // We assume full control over the console and send JavaScript logs to Metro.
+        [
+          'trace',
+          'info',
+          'warn',
+          'log',
+          'group',
+          'groupCollapsed',
+          'groupEnd',
+          'debug',
+        ].forEach(level => {
+          const originalFunction = console[level];
+          // $FlowFixMe Overwrite console methods
+          console[level] = function(...args) {
+            HMRClient.log(level, args);
+            originalFunction.apply(console, args);
+          };
+        });
+      } else {
+        // We assume the environment has a real rich console (like Chrome), and don't hijack it to log to Metro.
+        // It's likely the developer is using rich console to debug anyway, and hijacking it would
+        // lose the filenames in console.log calls: https://github.com/facebook/react-native/issues/26788.
+        HMRClient.log('log', [
+          `JavaScript logs will appear in your ${
+            isLikelyARealBrowser ? 'browser' : 'environment'
+          } console`,
+        ]);
+      }
     }
 
     require('./setUpReactRefresh');

diff --git a/Libraries/polyfills/console.js b/Libraries/polyfills/console.js
@@ -569,6 +569,11 @@ if (global.nativeLoggingHook) {
     assert: consoleAssertPolyfill,
   };
 
+  Object.defineProperty(console, '_isPolyfilled', {
+    value: true,
+    enumerable: false,
+  });
+
   // If available, also call the original `console` method since that is
   // sometimes useful. Ex: on OS X, this will let you see rich output in
   // the Safari Web Inspector console.
@@ -620,4 +625,9 @@ if (global.nativeLoggingHook) {
     debug: log,
     table: log,
   };
+
+  Object.defineProperty(console, '_isPolyfilled', {
+    value: true,
+    enumerable: false,
+  });
 }