From b7c1bd4e89d7eb1e6371f51909df209575c75606 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 20 Mar 2024 06:31:34 -0700 Subject: [PATCH] Remove console logs from Metro when native debugger console is available Summary: Changelog: [Internal] Remove console logs from Metro when native Fusebox debugger console is available React Native currently sends all `console.log` messages to Metro, which prints them to the terminal. This feature has been in place since 2019 (D15559151) but is pretty limited when compared to what's available in modern browsers. Most of the limitations can't really be fixed within the constraints of Metro's relatively simple terminal infrastructure. With the new React Native debugger (codenamed "Fusebox") we aim to fundamentally elevate the debugging experience by shipping a well-tested version of Chrome DevTools with React Native. Chrome DevTools has a rich, interactive console, as well as a host of other debugging features we want developers to notice and use. To that end, we plan to **strongly nudge users towards the Fusebox console from day 1**. Specifically, if we detect that Fusebox is available and is using an engine which implements CDP `console` support ( = Hermes only for now), we'll no longer send logs to Metro, and will instead display an explanatory message directing users to Fusebox. Reviewed By: huntie Differential Revision: D54829811 fbshipit-source-id: 2b1cdb666094f901ff4e7f42b123271be4ce7d10 --- .../Libraries/Core/setUpDeveloperTools.js | 4 ++- .../Libraries/Utilities/HMRClient.js | 28 +++++++++++++++++++ .../Libraries/Utilities/HMRClientProdShim.js | 1 + .../__snapshots__/public-api-test.js.snap | 1 + .../chrome/HermesRuntimeTargetDelegate.cpp | 8 ++++++ .../chrome/HermesRuntimeTargetDelegate.h | 2 ++ .../FallbackRuntimeTargetDelegate.cpp | 4 +++ .../FallbackRuntimeTargetDelegate.h | 2 ++ .../jsinspector-modern/RuntimeTarget.h | 6 ++++ .../RuntimeTargetConsole.cpp | 11 +++++++- .../jsinspector-modern/tests/InspectorMocks.h | 1 + 11 files changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/react-native/Libraries/Core/setUpDeveloperTools.js b/packages/react-native/Libraries/Core/setUpDeveloperTools.js index 67227191a8b..5cc39eae9c1 100644 --- a/packages/react-native/Libraries/Core/setUpDeveloperTools.js +++ b/packages/react-native/Libraries/Core/setUpDeveloperTools.js @@ -42,7 +42,9 @@ if (__DEV__) { if (!Platform.isTesting) { const HMRClient = require('../Utilities/HMRClient'); - if (console._isPolyfilled) { + if (global.__FUSEBOX_HAS_FULL_CONSOLE_SUPPORT__) { + HMRClient.unstable_notifyFuseboxConsoleEnabled(); + } else if (console._isPolyfilled) { // We assume full control over the console and send JavaScript logs to Metro. [ 'trace', diff --git a/packages/react-native/Libraries/Utilities/HMRClient.js b/packages/react-native/Libraries/Utilities/HMRClient.js index 03ae2874fff..27c52dad1f5 100644 --- a/packages/react-native/Libraries/Utilities/HMRClient.js +++ b/packages/react-native/Libraries/Utilities/HMRClient.js @@ -26,6 +26,7 @@ let hmrUnavailableReason: string | null = null; let currentCompileErrorMessage: string | null = null; let didConnect: boolean = false; let pendingLogs: Array<[LogLevel, $ReadOnlyArray]> = []; +let pendingFuseboxConsoleNotification = false; type LogLevel = | 'trace' @@ -51,6 +52,7 @@ export type HMRClientNativeInterface = {| isEnabled: boolean, scheme?: string, ): void, + unstable_notifyFuseboxConsoleEnabled(): void, |}; /** @@ -140,6 +142,29 @@ const HMRClient: HMRClientNativeInterface = { } }, + unstable_notifyFuseboxConsoleEnabled() { + if (!hmrClient) { + pendingFuseboxConsoleNotification = true; + return; + } + hmrClient.send( + JSON.stringify({ + type: 'log', + level: 'info', + data: [ + '\n' + + '\x1b[7m' + + ' \x1b[1mJavaScript logs have moved!\x1b[22m They will now appear in the debugger console. ' + + 'Tip: Type \x1b[1mj\x1b[22m in the terminal to open the debugger (requires Google Chrome ' + + 'or Microsoft Edge).' + + '\x1b[27m' + + '\n', + ], + }), + ); + pendingFuseboxConsoleNotification = false; + }, + // Called once by the bridge on startup, even if Fast Refresh is off. // It creates the HMR client but doesn't actually set up the socket yet. setup( @@ -316,6 +341,9 @@ function flushEarlyLogs(client: MetroHMRClient) { pendingLogs.forEach(([level, data]) => { HMRClient.log(level, data); }); + if (pendingFuseboxConsoleNotification) { + HMRClient.unstable_notifyFuseboxConsoleEnabled(); + } } finally { pendingLogs.length = 0; } diff --git a/packages/react-native/Libraries/Utilities/HMRClientProdShim.js b/packages/react-native/Libraries/Utilities/HMRClientProdShim.js index 4d36db2bc52..c4492b84545 100644 --- a/packages/react-native/Libraries/Utilities/HMRClientProdShim.js +++ b/packages/react-native/Libraries/Utilities/HMRClientProdShim.js @@ -25,6 +25,7 @@ const HMRClientProdShim: HMRClientNativeInterface = { disable() {}, registerBundle() {}, log() {}, + unstable_notifyFuseboxConsoleEnabled() {}, }; module.exports = HMRClientProdShim; diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index 290704597d7..482356444d6 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -8417,6 +8417,7 @@ export type HMRClientNativeInterface = {| isEnabled: boolean, scheme?: string ): void, + unstable_notifyFuseboxConsoleEnabled(): void, |}; declare const HMRClient: HMRClientNativeInterface; declare module.exports: HMRClient; diff --git a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.cpp b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.cpp index f83da054d60..0d762a4248f 100644 --- a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.cpp +++ b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.cpp @@ -122,6 +122,10 @@ class HermesRuntimeTargetDelegate::Impl final : public RuntimeTargetDelegate { HermesConsoleMessage{message.timestamp, type, std::move(message.args)}); } + bool supportsConsole() const override { + return true; + } + private: HermesRuntimeTargetDelegate& delegate_; std::shared_ptr runtime_; @@ -173,6 +177,10 @@ void HermesRuntimeTargetDelegate::addConsoleMessage( impl_->addConsoleMessage(runtime, std::move(message)); } +bool HermesRuntimeTargetDelegate::supportsConsole() const { + return impl_->supportsConsole(); +} + #ifdef HERMES_ENABLE_DEBUGGER CDPDebugAPI& HermesRuntimeTargetDelegate::getCDPDebugAPI() { return impl_->getCDPDebugAPI(); diff --git a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.h b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.h index d442505dbf6..8efc96c46f7 100644 --- a/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.h +++ b/packages/react-native/ReactCommon/hermes/inspector-modern/chrome/HermesRuntimeTargetDelegate.h @@ -48,6 +48,8 @@ class HermesRuntimeTargetDelegate : public RuntimeTargetDelegate { void addConsoleMessage(jsi::Runtime& runtime, ConsoleMessage message) override; + bool supportsConsole() const override; + private: // We use the private implementation idiom to ensure this class has the same // layout regardless of whether HERMES_ENABLE_DEBUGGER is defined. The net diff --git a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.cpp b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.cpp index f7d36d415f9..41cd6132ac4 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.cpp @@ -32,4 +32,8 @@ void FallbackRuntimeTargetDelegate::addConsoleMessage( // TODO: Best-effort printing (without RemoteObjects) } +bool FallbackRuntimeTargetDelegate::supportsConsole() const { + return false; +} + } // namespace facebook::react::jsinspector_modern diff --git a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.h b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.h index fa859f0fced..c2fcb833bb9 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/FallbackRuntimeTargetDelegate.h @@ -34,6 +34,8 @@ class FallbackRuntimeTargetDelegate : public RuntimeTargetDelegate { void addConsoleMessage(jsi::Runtime& runtime, ConsoleMessage message) override; + bool supportsConsole() const override; + private: std::string engineDescription_; }; diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h index a5f42b8fa1f..ef34e995907 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTarget.h @@ -67,6 +67,12 @@ class RuntimeTargetDelegate { virtual void addConsoleMessage( jsi::Runtime& runtime, ConsoleMessage message) = 0; + + /** + * \returns true if the runtime supports reporting console API calls over CDP. + * \c addConsoleMessage MAY be called even if this method returns false. + */ + virtual bool supportsConsole() const = 0; }; /** diff --git a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp index 6ac0b02fad9..d0734bf96ce 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp +++ b/packages/react-native/ReactCommon/jsinspector-modern/RuntimeTargetConsole.cpp @@ -102,8 +102,10 @@ double getTimestampMs() { } // namespace void RuntimeTarget::installConsoleHandler() { + auto delegateSupportsConsole = delegate_.supportsConsole(); jsExecutor_([selfWeak = weak_from_this(), - selfExecutor = executorFromThis()](jsi::Runtime& runtime) { + selfExecutor = executorFromThis(), + delegateSupportsConsole](jsi::Runtime& runtime) { jsi::Value consolePrototype = jsi::Value::null(); auto originalConsoleVal = runtime.global().getProperty(runtime, "console"); std::shared_ptr originalConsole; @@ -441,6 +443,13 @@ void RuntimeTarget::installConsoleHandler() { } runtime.global().setProperty(runtime, "console", console); + if (delegateSupportsConsole) { + // NOTE: If the delegate doesn't report console support, we'll still + // install the console handler for consistency of the runtime environment, + // but not claim that it has full console support. + runtime.global().setProperty( + runtime, "__FUSEBOX_HAS_FULL_CONSOLE_SUPPORT__", true); + } }); } diff --git a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h index f0b73ae9dca..7e74771f0a5 100644 --- a/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h +++ b/packages/react-native/ReactCommon/jsinspector-modern/tests/InspectorMocks.h @@ -141,6 +141,7 @@ class MockRuntimeTargetDelegate : public RuntimeTargetDelegate { addConsoleMessage, (jsi::Runtime & runtime, ConsoleMessage message), (override)); + MOCK_METHOD(bool, supportsConsole, (), (override, const)); }; class MockRuntimeAgentDelegate : public RuntimeAgentDelegate {