Merge pull request #855 from overte-org/fix/script_require_bg

Fix require behavior for modules
overte-org · Mar 5, 2024 · e9e63ef · e9e63ef
2 parents 22faf21 + 931b222
commit e9e63ef
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 1 deletion.
diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp
@@ -664,6 +664,11 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure,
             }
             // "Script" API is context-dependent, so it needs to be recreated for each new context
             registerGlobalObject("Script", new ScriptManagerScriptingInterface(_manager), ScriptEngine::ScriptOwnership);
+            auto Script = globalObject().property("Script");
+            auto require = Script.property("require");
+            auto resolve = Script.property("_requireResolve");
+            require.setProperty("resolve", resolve, ScriptValue::ReadOnly | ScriptValue::Undeletable);
+            globalObject().setProperty("require", require, ScriptValue::ReadOnly | ScriptValue::Undeletable);
 
             // Script.require properties need to be copied, since that's where the Script.require cache is
             // Get source and destination Script.require objects

diff --git a/tests/script-engine/src/ScriptEngineNetworkedTests.cpp b/tests/script-engine/src/ScriptEngineNetworkedTests.cpp
@@ -120,7 +120,7 @@ ScriptManagerPointer ScriptEngineNetworkedTests::makeManager(const QString &scri
     return sm;
 }
 
-void ScriptEngineNetworkedTests::testRequire() {
+void ScriptEngineNetworkedTests::testScriptRequire() {
     auto sm = makeManager(
         "print(\"Starting\");"
         "Script.require('./tests/c.js');"
@@ -153,6 +153,38 @@ void ScriptEngineNetworkedTests::testRequire() {
     }
 }
 
+void ScriptEngineNetworkedTests::testRequire() {
+    auto sm = makeManager(
+        "print(\"Starting\");"
+        "require('./tests/c_require.js');"
+        "print(\"Done\");"
+        "Script.stop(true);", "testRequire.js");
+    QStringList printed;
+    QStringList expected {"Starting", "Value from A: 6", "Value from B: 6", "Done"};
+
+
+    QVERIFY(!sm->isRunning());
+    QVERIFY(!sm->isStopped());
+    QVERIFY(!sm->isFinished());
+
+    connect(sm.get(), &ScriptManager::printedMessage, [&printed](const QString& message, const QString& engineName){
+        printed.append(message);
+    });
+
+
+    qInfo() << "About to run script";
+    sm->run();
+
+    QVERIFY(!sm->isRunning());
+    QVERIFY(!sm->isStopped());
+    QVERIFY(sm->isFinished());
+
+    QVERIFY(printed.length() == expected.length());
+    for(int i=0;i<printed.length();i++) {
+        QString nomatch = QString("Result '%1' didn't match expected '%2'").arg(printed[i]).arg(expected[i]);
+        QVERIFY2(printed[i] == expected[i], qPrintable(nomatch));
+    }
+}
 
 
 void ScriptEngineNetworkedTests::testRequireInfinite() {

diff --git a/tests/script-engine/src/ScriptEngineNetworkedTests.h b/tests/script-engine/src/ScriptEngineNetworkedTests.h
@@ -27,6 +27,7 @@ class ScriptEngineNetworkedTests : public QObject {
 private slots:
     void initTestCase();
     void testRequire();
+    void testScriptRequire();
     void testRequireInfinite();
 
 

diff --git a/tests/script-engine/src/tests/b_require.js b/tests/script-engine/src/tests/b_require.js
@@ -0,0 +1,6 @@
+
+// b.js
+var a = require('./a.js');
+a.value += 1;
+console.log('message from b');
+module.exports = a.value;
diff --git a/tests/script-engine/src/tests/c_require.js b/tests/script-engine/src/tests/c_require.js
@@ -0,0 +1,6 @@
+
+// c.js
+var a = require('./a.js');
+var b = require('./b_require.js');
+print("Value from A: " + a.value);
+print("Value from B: " + b);