Skip to content

Commit

Permalink
fix: refactor console.log implementation a bit (#1741)
Browse files Browse the repository at this point in the history
  • Loading branch information
vmutafov authored Nov 18, 2022
1 parent 1c0214a commit d3c52cb
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 54 deletions.
3 changes: 2 additions & 1 deletion test-app/app/src/main/assets/app/mainpage.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,5 @@ require("./tests/kotlin/enums/testEnumsSupport");
require("./tests/kotlin/access/testInternalLanguageFeaturesSupport");
require("./tests/testPackagePrivate");
require("./tests/kotlin/properties/testPropertiesSupport.js");
require('./tests/testNativeTimers');
require('./tests/testNativeTimers');
require("./tests/console/logTests.js");
31 changes: 31 additions & 0 deletions test-app/app/src/main/assets/app/tests/console/logTests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
describe("Test JSONObject conversions", () => {
it("console.log with number param should not crash", () => {
console.log(123);
});

it("console.log with string param should not crash", () => {
console.log("123");
});

it("console.log with object param should not crash", () => {
console.log({ num: 123 });
});

it("console.log with function param should not crash", () => {
console.log(function() {});
});

it("console.log with arrow function param should not crash", () => {
console.log(() => {});
});

it("console.log with primitive array param should not crash", () => {
console.log([1, 2, 3]);
});

it("console.log with object array param should not crash", () => {
console.log([{
num: 123
}]);
});
});
12 changes: 8 additions & 4 deletions test-app/runtime/src/main/cpp/CallbackHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1091,16 +1091,18 @@ CallbackHandlers::WorkerObjectPostMessageCallback(const v8::FunctionCallbackInfo
ArgConverter::ConvertToV8String(isolate, "workerId"),
jsId);

Local<String> msg = tns::JsonStringifyObject(isolate, args[0], false);
auto context = isolate->GetCurrentContext();
auto objToStringify = args[0]->ToObject(context).ToLocalChecked();
std::string msg = tns::JsonStringifyObject(isolate, objToStringify, false);

// get worker's ID that is associated on the other side - in Java
auto id = jsId->Int32Value(context).ToChecked();

JEnv env;
auto mId = env.GetStaticMethodID(RUNTIME_CLASS, "sendMessageFromMainToWorker",
"(ILjava/lang/String;)V");

auto jmsg = ArgConverter::ConvertToJavaString(msg);
jstring jmsg = env.NewStringUTF(msg.c_str());
JniLocalRef jmsgRef(jmsg);

env.CallStaticVoidMethod(RUNTIME_CLASS, mId, id, (jstring) jmsgRef);
Expand Down Expand Up @@ -1190,13 +1192,15 @@ CallbackHandlers::WorkerGlobalPostMessageCallback(const v8::FunctionCallbackInfo
return;
}

Local<String> msg = tns::JsonStringifyObject(isolate, args[0], false);
auto context = isolate->GetCurrentContext();
auto objToStringify = args[0]->ToObject(context).ToLocalChecked();
std::string msg = tns::JsonStringifyObject(isolate, objToStringify, false);

JEnv env;
auto mId = env.GetStaticMethodID(RUNTIME_CLASS, "sendMessageFromWorkerToMain",
"(Ljava/lang/String;)V");

auto jmsg = ArgConverter::ConvertToJavaString(msg);
auto jmsg = env.NewStringUTF(msg.c_str());
JniLocalRef jmsgRef(jmsg);

env.CallStaticVoidMethod(RUNTIME_CLASS, mId, (jstring) jmsgRef);
Expand Down
17 changes: 10 additions & 7 deletions test-app/runtime/src/main/cpp/V8GlobalHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ Local<Function> GetSmartJSONStringifyFunction(Isolate* isolate) {
return smartStringifyPersistentFunction->Get(isolate);
}

Local<String> tns::JsonStringifyObject(Isolate* isolate, Handle<v8::Value> value, bool handleCircularReferences) {
std::string tns::JsonStringifyObject(Isolate* isolate, v8::Local<v8::Object> value, bool handleCircularReferences) {
if (value.IsEmpty()) {
return String::Empty(isolate);
return "";
}

auto context = isolate->GetCurrentContext();
Expand All @@ -80,25 +80,28 @@ Local<String> tns::JsonStringifyObject(Isolate* isolate, Handle<v8::Value> value
v8::Local<v8::Value> resultValue;
v8::TryCatch tc(isolate);

Local<Value> args[] = { value->ToObject(context).ToLocalChecked() };
auto success = smartJSONStringifyFunction->Call(context, Undefined(isolate), 1, args).ToLocal(&resultValue);
Local<Value> args[] = { value };
auto success = smartJSONStringifyFunction
->Call(context, Undefined(isolate), 1, args)
.ToLocal(&resultValue);

if (success && !tc.HasCaught()) {
return resultValue->ToString(context).ToLocalChecked();
auto res = resultValue.As<v8::String>();
return ArgConverter::ConvertToString(res);
}
}
}
}

v8::Local<v8::String> resultString;
v8::TryCatch tc(isolate);
auto success = v8::JSON::Stringify(context, value->ToObject(context).ToLocalChecked()).ToLocal(&resultString);
auto success = v8::JSON::Stringify(context, value).ToLocal(&resultString);

if (!success && tc.HasCaught()) {
throw NativeScriptException(tc);
}

return resultString;
return ArgConverter::ConvertToString(resultString);
}

bool tns::V8GetPrivateValue(Isolate* isolate, const Local<Object>& obj, const Local<String>& propName, Local<Value>& out) {
Expand Down
2 changes: 1 addition & 1 deletion test-app/runtime/src/main/cpp/V8GlobalHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <map>

namespace tns {
v8::Local<v8::String> JsonStringifyObject(v8::Isolate* isolate, v8::Handle<v8::Value> value, bool handleCircularReferences = true);
std::string JsonStringifyObject(v8::Isolate* isolate, v8::Handle<v8::Object> value, bool handleCircularReferences = true);

bool V8GetPrivateValue(v8::Isolate* isolate, const v8::Local<v8::Object>& obj, const v8::Local<v8::String>& propName, v8::Local<v8::Value>& out);

Expand Down
64 changes: 23 additions & 41 deletions test-app/runtime/src/main/cpp/console/Console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,105 +72,89 @@ void Console::sendToDevToolsFrontEnd(v8::Isolate* isolate, const std::string& me
}
}

const v8::Local<v8::String> transformJSObject(v8::Isolate* isolate, v8::Local<v8::Object> object) {
std::string transformJSObject(v8::Isolate* isolate, v8::Local<v8::Object> object) {
auto context = isolate->GetCurrentContext();
auto objToString = object->ToString(context).ToLocalChecked();
v8::Local<v8::String> resultString;
auto objToCppString = ArgConverter::ConvertToString(objToString);

auto hasCustomToStringImplementation = ArgConverter::ConvertToString(objToString).find("[object Object]") == std::string::npos;
auto hasCustomToStringImplementation = objToCppString.find("[object Object]") == std::string::npos;

if (hasCustomToStringImplementation) {
resultString = objToString;
return objToCppString;
} else {
v8::HandleScope scope(isolate);
resultString = JsonStringifyObject(isolate, object);
return JsonStringifyObject(isolate, object);
}

return resultString;
}

const v8::Local<v8::String> buildStringFromArg(v8::Isolate* isolate, const v8::Local<v8::Value>& val) {
v8::Local<v8::String> argString;
std::string buildStringFromArg(v8::Isolate* isolate, const v8::Local<v8::Value>& val) {
if (val->IsFunction()) {
val->ToDetailString(isolate->GetCurrentContext()).ToLocal(&argString);
auto v8FunctionString = val->ToDetailString(isolate->GetCurrentContext()).ToLocalChecked();
return ArgConverter::ConvertToString(v8FunctionString);
} else if (val->IsArray()) {
auto context = isolate->GetCurrentContext();
auto cachedSelf = val;
auto array = val->ToObject(context).ToLocalChecked();
auto arrayEntryKeys = array->GetPropertyNames(isolate->GetCurrentContext()).ToLocalChecked();

auto arrayLength = arrayEntryKeys->Length();

argString = ArgConverter::ConvertToV8String(isolate, "[");
std::string argString = "[";

for (int i = 0; i < arrayLength; i++) {
auto propertyName = arrayEntryKeys->Get(context, i).ToLocalChecked();

auto propertyValue = array->Get(context, propertyName).ToLocalChecked();

// avoid bottomless recursion with cyclic reference to the same array
if (propertyValue->StrictEquals(cachedSelf)) {
argString = v8::String::Concat(isolate, argString, ArgConverter::ConvertToV8String(isolate, "[Circular]"));
argString.append("[Circular]");
continue;
}

auto objectString = buildStringFromArg(isolate, propertyValue);

argString = v8::String::Concat(isolate, argString, objectString);
argString.append(objectString);

if (i != arrayLength - 1) {
argString = v8::String::Concat(isolate, argString, ArgConverter::ConvertToV8String(isolate, ", "));
argString.append(", ");
}
}

argString = v8::String::Concat(isolate, argString, ArgConverter::ConvertToV8String(isolate, "]"));
return argString.append("]");
} else if (val->IsObject()) {
v8::Local<v8::Object> obj = val.As<v8::Object>();

argString = transformJSObject(isolate, obj);
return transformJSObject(isolate, obj);
} else {
val->ToDetailString(isolate->GetCurrentContext()).ToLocal(&argString);
auto v8DefaultToString = val->ToDetailString(isolate->GetCurrentContext()).ToLocalChecked();
return ArgConverter::ConvertToString(v8DefaultToString);
}

return argString;
}

const std::string buildLogString(const v8::FunctionCallbackInfo<v8::Value>& info, int startingIndex = 0) {
std::string buildLogString(const v8::FunctionCallbackInfo<v8::Value>& info, int startingIndex = 0) {
auto isolate = info.GetIsolate();

v8::HandleScope scope(isolate);

std::stringstream ss;

auto argLen = info.Length();
if (argLen) {
for (int i = startingIndex; i < argLen; i++) {
v8::Local<v8::String> argString;

argString = buildStringFromArg(isolate, info[i]);

// separate args with a space
if (i != 0) {
ss << " ";
}

ss << ArgConverter::ConvertToString(argString);

std::string argString = buildStringFromArg(isolate, info[i]);
ss << argString;
}
} else {
ss << std::endl;
}

std::string stringResult = ss.str();

return stringResult;
return ss.str();
}

void Console::assertCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
try {
auto isolate = info.GetIsolate();

auto argLen = info.Length();
auto context = isolate->GetCurrentContext();
auto expressionPasses = argLen && info[0]->BooleanValue(isolate);

if (!expressionPasses) {
Expand Down Expand Up @@ -307,13 +291,11 @@ void Console::dirCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
if (propIsFunction) {
ss << "()";
} else if (propertyValue->IsArray()) {
auto stringResult = buildStringFromArg(isolate, propertyValue);
std::string jsonStringifiedArray = ArgConverter::ConvertToString(stringResult);
std::string jsonStringifiedArray = buildStringFromArg(isolate, propertyValue);
ss << ": " << jsonStringifiedArray;
} else if (propertyValue->IsObject()) {
auto obj = propertyValue->ToObject(context).ToLocalChecked();
auto objString = transformJSObject(isolate, obj);
std::string jsonStringifiedObject = ArgConverter::ConvertToString(objString);
auto jsonStringifiedObject = transformJSObject(isolate, obj);
// if object prints out as the error string for circular references, replace with #CR instead for brevity
if (jsonStringifiedObject.find("circular structure") != std::string::npos) {
jsonStringifiedObject = "#CR";
Expand Down

0 comments on commit d3c52cb

Please sign in to comment.