Skip to content

fix: refactor console.log implementation a bit #1741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 18, 2022
Merged

fix: refactor console.log implementation a bit #1741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion test-app/app/src/main/assets/app/mainpage.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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