From 3c3f8ca270028fc0b6c20eba4d95561298a9d26f Mon Sep 17 00:00:00 2001
From: Riley Dulin <dulinr@fb.com>
Date: Tue, 30 Jun 2020 21:13:53 -0700
Subject: [PATCH] Add a special case for assert in installConsoleFunction

Summary:
The Hermes inspector always logged the message for `console.assert`.
It instead should check the first argument, and only if that argument
is false should it log.

Also remove the polyfill code that tried to avoid this situation.

Changelog: [Internal]

Reviewed By: mhorowitz

Differential Revision: D22299186

fbshipit-source-id: cdf4f8957d4db3d171d6673a82c7fc32b7152af3
---
 Libraries/polyfills/console.js             | 10 +----
 ReactCommon/hermes/inspector/Inspector.cpp | 52 +++++++++++++++++++---
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/Libraries/polyfills/console.js b/Libraries/polyfills/console.js
index 962e52b5e9..f82fb4d05b 100644
--- a/Libraries/polyfills/console.js
+++ b/Libraries/polyfills/console.js
@@ -580,15 +580,7 @@ if (global.nativeLoggingHook) {
       const reactNativeMethod = console[methodName];
       if (originalConsole[methodName]) {
         console[methodName] = function() {
-          // TODO(T43930203): remove this special case once originalConsole.assert properly checks
-          // the condition
-          if (methodName === 'assert') {
-            if (!arguments[0]) {
-              originalConsole.assert(...arguments);
-            }
-          } else {
-            originalConsole[methodName](...arguments);
-          }
+          originalConsole[methodName](...arguments);
           reactNativeMethod.apply(console, arguments);
         };
       }
diff --git a/ReactCommon/hermes/inspector/Inspector.cpp b/ReactCommon/hermes/inspector/Inspector.cpp
index c2b5871211..14cb76cad3 100644
--- a/ReactCommon/hermes/inspector/Inspector.cpp
+++ b/ReactCommon/hermes/inspector/Inspector.cpp
@@ -136,6 +136,29 @@ Inspector::~Inspector() {
   debugger_.setEventObserver(nullptr);
 }
 
+static bool toBoolean(jsi::Runtime &runtime, const jsi::Value &val) {
+  // Based on Operations.cpp:toBoolean in the Hermes VM.
+  if (val.isUndefined() || val.isNull()) {
+    return false;
+  }
+  if (val.isBool()) {
+    return val.getBool();
+  }
+  if (val.isNumber()) {
+    double m = val.getNumber();
+    return m != 0 && !std::isnan(m);
+  }
+  if (val.isSymbol() || val.isObject()) {
+    return true;
+  }
+  if (val.isString()) {
+    std::string s = val.getString(runtime).utf8(runtime);
+    return !s.empty();
+  }
+  assert(false && "All cases should be covered");
+  return false;
+}
+
 void Inspector::installConsoleFunction(
     jsi::Object &console,
     std::shared_ptr<jsi::Object> &originalConsole,
@@ -169,11 +192,30 @@ void Inspector::installConsoleFunction(
             }
 
             if (auto inspector = weakInspector.lock()) {
-              jsi::Array argsArray(runtime, count);
-              for (size_t index = 0; index < count; ++index)
-                argsArray.setValueAtIndex(runtime, index, args[index]);
-              inspector->logMessage(
-                  ConsoleMessageInfo{chromeType, std::move(argsArray)});
+              if (name != "assert") {
+                // All cases other than assert just log a simple message.
+                jsi::Array argsArray(runtime, count);
+                for (size_t index = 0; index < count; ++index)
+                  argsArray.setValueAtIndex(runtime, index, args[index]);
+                inspector->logMessage(
+                    ConsoleMessageInfo{chromeType, std::move(argsArray)});
+                return jsi::Value::undefined();
+              }
+              // console.assert needs to check the first parameter before
+              // logging.
+              if (count == 0) {
+                // No parameters, throw a blank assertion failed message.
+                inspector->logMessage(
+                    ConsoleMessageInfo{chromeType, jsi::Array(runtime, 0)});
+              } else if (!toBoolean(runtime, args[0])) {
+                // Shift the message array down by one to not include the
+                // condition.
+                jsi::Array argsArray(runtime, count - 1);
+                for (size_t index = 1; index < count; ++index)
+                  argsArray.setValueAtIndex(runtime, index, args[index]);
+                inspector->logMessage(
+                    ConsoleMessageInfo{chromeType, std::move(argsArray)});
+              }
             }
 
             return jsi::Value::undefined();