Skip to content

Commit

Permalink
Don't wrap console methods for metro logging in Chrome debugger (#26883)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rickhanlonii authored and grabbou committed Nov 4, 2019
1 parent bd12ca1 commit 5e702a1
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 17 deletions.
67 changes: 50 additions & 17 deletions Libraries/Core/setUpDeveloperTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');
Expand Down
10 changes: 10 additions & 0 deletions Libraries/polyfills/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -608,4 +613,9 @@ if (global.nativeLoggingHook) {
debug: log,
table: log,
};

Object.defineProperty(console, '_isPolyfilled', {
value: true,
enumerable: false,
});
}

0 comments on commit 5e702a1

Please sign in to comment.