From e786fd48645a75ee2b8aff9e8d9d247fc608acf6 Mon Sep 17 00:00:00 2001 From: ksuprynowicz Date: Sun, 15 Jan 2023 20:44:03 +0100 Subject: [PATCH] Fixed Script.require --- libraries/script-engine/src/ScriptManager.cpp | 5 ++- .../script-engine/src/v8/ScriptEngineV8.cpp | 41 ++++++++++++++++--- tests/script-engine/src/tests/004_require.js | 4 +- .../src/tests/004b_require_module.js | 9 ++++ 4 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 tests/script-engine/src/tests/004b_require_module.js diff --git a/libraries/script-engine/src/ScriptManager.cpp b/libraries/script-engine/src/ScriptManager.cpp index b66da2b0db..3df0f26cbe 100644 --- a/libraries/script-engine/src/ScriptManager.cpp +++ b/libraries/script-engine/src/ScriptManager.cpp @@ -1341,7 +1341,7 @@ ScriptValue ScriptManager::newModule(const QString& modulePath, const ScriptValu // module.require is a bound version of require that always resolves relative to that module's path auto boundRequire = _engine->evaluate("(function(id) { return Script.require(Script.require.resolve(id, this.filename)); })", "(boundRequire)"); module.setProperty("require", boundRequire, READONLY_PROP_FLAGS); - qDebug() << "Module object contents" << _engine->scriptValueDebugListMembers(module); + //qDebug() << "Module object contents" << _engine->scriptValueDebugListMembers(module); return module; } @@ -1421,7 +1421,7 @@ ScriptValue ScriptManager::instantiateModule(const ScriptValue& module, const QS closure.setProperty("require", module.property("require")); closure.setProperty("__filename", modulePath, READONLY_HIDDEN_PROP_FLAGS); closure.setProperty("__dirname", QString(modulePath).replace(QRegExp("/[^/]*$"), ""), READONLY_HIDDEN_PROP_FLAGS); - _engine->scriptValueDebugDetails(module); + //_engine->scriptValueDebugDetails(module); result = _engine->evaluateInClosure(closure, _engine->newProgram( sourceCode, modulePath )); } _engine->maybeEmitUncaughtException(__FUNCTION__); @@ -1531,6 +1531,7 @@ ScriptValue ScriptManager::require(const QString& moduleId) { qCDebug(scriptengine_module) << "//ScriptManager::require(" << moduleId << ")"; _engine->maybeEmitUncaughtException(__FUNCTION__); + qCDebug(scriptengine_module) << "Exports: " << _engine->scriptValueDebugDetails(module.property("exports")); return module.property("exports"); } diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index 0d9071891a..8110314a6d 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -760,6 +760,7 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, v8::Local closureGlobal; ScriptValueV8Wrapper* unwrappedClosure; ScriptProgramV8Wrapper* unwrappedProgram; + v8::Local oldContext = getContext(); { v8::Context::Scope contextScope(_v8Context.Get(_v8Isolate)); @@ -792,6 +793,10 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, return nullValue(); } closureObject = v8::Local::Cast(closure.constGet()); + qDebug() << "Closure object members:" << scriptValueDebugListMembersV8(closure); + v8::Local testObject = v8::Object::New(_v8Isolate); + testObject->Set(getContext(), v8::String::NewFromUtf8(_v8Isolate, "test_value").ToLocalChecked(), closureObject); + qDebug() << "Test object members:" << scriptValueDebugListMembersV8(V8ScriptValue(_v8Isolate, testObject)); if (!closureObject->Get(closure.constGetContext(), v8::String::NewFromUtf8(_v8Isolate, "global").ToLocalChecked()) .ToLocal(&closureGlobal)) { @@ -806,7 +811,7 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, //oldGlobal = _v8Context.Get(_v8Isolate)->Global(); v8::Local closureContext; - // V8TODO + // V8TODO V8 cannot use arbitrary objects as global objects /*if (closureGlobal->IsObject()) { #ifdef DEBUG_JS qCDebug(shared) << " setting global = closure.global" << shortName; @@ -817,7 +822,7 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, } else { closureContext = v8::Context::New(_v8Isolate); }*/ - closureContext = v8::Context::New(_v8Isolate, nullptr, v8::Local(), closureObject); + closureContext = v8::Context::New(_v8Isolate); ScriptValue result; //auto context = pushContext(); @@ -861,8 +866,33 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, #endif { v8::TryCatch tryCatch(getIsolate()); - //qDebug(scriptengine) << "Closure before run:" << scriptValueDebugDetailsV8(closure); - auto maybeResult = program.constGet()->Run(closureContext); + // Since V8 cannot use arbitrary object as global object, objects from main global need to be copied to closure's global object + auto oldGlobalMemberNames = oldContext->Global()->GetPropertyNames(oldContext).ToLocalChecked(); + for (int i = 0; i < oldGlobalMemberNames->Length(); i++) { + auto name = oldGlobalMemberNames->Get(closureContext, i).ToLocalChecked(); + closureContext->Global()->Set(closureContext, name, oldContext->Global()->Get(oldContext, name).ToLocalChecked()); + } + // Objects from closure need to be copied to global object too + auto closureMemberNames = closureObject->GetPropertyNames(closureContext).ToLocalChecked(); + for (int i = 0; i < closureMemberNames->Length(); i++) { + auto name = closureMemberNames->Get(closureContext, i).ToLocalChecked(); + closureContext->Global()->Set(closureContext, name, closureObject->Get(closureContext, name).ToLocalChecked()); + } + // List members of closure global object + QString membersString(""); + if (closureContext->Global()->IsObject()) { + v8::Local membersStringV8; + v8::Local object = v8::Local::Cast(closureContext->Global()); + auto names = object->GetPropertyNames(closureContext).ToLocalChecked(); + if (v8::JSON::Stringify(closureContext, names).ToLocal(&membersStringV8)) { + membersString = QString(*v8::String::Utf8Value(_v8Isolate, membersStringV8)); + } + membersString = QString(*v8::String::Utf8Value(_v8Isolate, membersStringV8)); + } else { + membersString = QString(" Is not an object"); + } + qDebug(scriptengine) << "Closure global before run:" << membersString; + auto maybeResult = program.constGet()->GetUnboundScript()->BindToCurrentContext()->Run(closureContext); //qDebug(scriptengine) << "Closure after run:" << scriptValueDebugDetailsV8(closure); v8::Local v8Result; if (!maybeResult.ToLocal(&v8Result)) { @@ -897,7 +927,6 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, setGlobalObject(oldGlobal); }*/ - closureContext->DetachGlobal(); _evaluatingCounter--; return result; } @@ -1546,7 +1575,7 @@ QString ScriptEngineV8::scriptValueDebugListMembersV8(const V8ScriptValue &v8Val v8::Local membersStringV8; v8::Local object = v8::Local::Cast(v8Value.constGet()); auto names = object->GetPropertyNames(getContext()).ToLocalChecked(); - if (v8::JSON::Stringify(getContext(), object).ToLocal(&membersStringV8)) { + if (v8::JSON::Stringify(getContext(), names).ToLocal(&membersStringV8)) { membersString = QString(*v8::String::Utf8Value(_v8Isolate, membersStringV8)); } membersString = QString(*v8::String::Utf8Value(_v8Isolate, membersStringV8)); diff --git a/tests/script-engine/src/tests/004_require.js b/tests/script-engine/src/tests/004_require.js index 6ed23d4a9e..10345ef596 100644 --- a/tests/script-engine/src/tests/004_require.js +++ b/tests/script-engine/src/tests/004_require.js @@ -5,5 +5,7 @@ Script.require.test_prop = "test property"; print(JSON.stringify("Script.require.test_prop after defining: " + Script.require.test_prop)); print("Before require"); // TODO: find correct local path for the test module -Script.require("http://oaktown.pl/scripts/001_test_print.js"); +requireTest = Script.require("http://oaktown.pl/scripts/004b_require_module.js"); print("After require"); +requireTest.moduleFunction1(); +requireTest.moduleFunction2(); diff --git a/tests/script-engine/src/tests/004b_require_module.js b/tests/script-engine/src/tests/004b_require_module.js new file mode 100644 index 0000000000..186746c9d1 --- /dev/null +++ b/tests/script-engine/src/tests/004b_require_module.js @@ -0,0 +1,9 @@ +print("Print inside module script"); +module.exports = { + moduleFunction1: function () { + print("moduleFunction1 works"); + }, + moduleFunction2: function () { + print("moduleFunction2 works"); + } +}; \ No newline at end of file