From 5e702a1da3330620624bc86d3741341cdfce29d8 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Mon, 21 Oct 2019 10:02:47 -0700 Subject: [PATCH] Don't wrap console methods for metro logging in Chrome debugger (#26883) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/26883 This diff fixes an issue reported in https://github.com/facebook/react-native/issues/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 --- Libraries/Core/setUpDeveloperTools.js | 67 ++++++++++++++++++++------- Libraries/polyfills/console.js | 10 ++++ 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/Libraries/Core/setUpDeveloperTools.js b/Libraries/Core/setUpDeveloperTools.js index 7a9ff424892..6d3bc326fd1 100644 --- a/Libraries/Core/setUpDeveloperTools.js +++ b/Libraries/Core/setUpDeveloperTools.js @@ -11,6 +11,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. @@ -54,25 +58,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 index 1dab70cc264..384dd873ea0 100644 --- a/Libraries/polyfills/console.js +++ b/Libraries/polyfills/console.js @@ -557,6 +557,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. @@ -608,4 +613,9 @@ if (global.nativeLoggingHook) { debug: log, table: log, }; + + Object.defineProperty(console, '_isPolyfilled', { + value: true, + enumerable: false, + }); }