From 606a7bc552b0737d2dee8feca3a203f167c33662 Mon Sep 17 00:00:00 2001
From: reuk <reuk@users.noreply.github.com>
Date: Tue, 2 Jul 2024 15:23:50 +0100
Subject: [PATCH] Javascript: Update implementation to preserve methods when
 converting between JUCE and QJS types

---
 .../juce_core/javascript/juce_Javascript.cpp  | 800 +++++++++++++-----
 1 file changed, 568 insertions(+), 232 deletions(-)

diff --git a/modules/juce_core/javascript/juce_Javascript.cpp b/modules/juce_core/javascript/juce_Javascript.cpp
index bfd9a9eb6b..b440b8f20c 100644
--- a/modules/juce_core/javascript/juce_Javascript.cpp
+++ b/modules/juce_core/javascript/juce_Javascript.cpp
@@ -55,177 +55,361 @@ static int64_t fromJuceInt64 (const T& convertible) { return (int64_t) (int64) c
 template <typename T>
 static int64_t toJuceInt64   (const T& convertible) { return (int64) (int64_t) convertible; }
 
-template<>
-struct VariantConverter<choc::value::Value>
-{
-    static choc::value::Value fromVar (const var& variant)
-    {
-        if (variant.isInt())
-            return choc::value::Value { (int) variant };
-
-        if (variant.isInt64())
-            return choc::value::Value { fromJuceInt64 (variant) };
-
-        if (variant.isDouble())
-            return choc::value::Value { (double) variant };
-
-        if (variant.isInt())
-            return choc::value::Value { (int) variant };
-
-        if (variant.isBool())
-            return choc::value::Value { (bool) variant };
-
-        if (variant.isString())
-            return choc::value::Value { variant.toString().toStdString() };
-
-        if (variant.isArray())
-        {
-            choc::value::Value value { choc::value::Type::createEmptyArray() };
-            const auto& array = *variant.getArray();
-
-            for (int i = 0; i < array.size(); ++i)
-                value.addArrayElement (fromVar (array[i]));
-
-            return value;
-        }
-
-        if (variant.isObject())
-        {
-            if (auto* dynamicObject = dynamic_cast<DynamicObject*> (variant.getObject()))
-            {
-                choc::value::Value value { choc::value::Type::createObject ("") };
-
-                for (const auto& [name, prop] : dynamicObject->getProperties())
-                    value.setMember (name.toString().toRawUTF8(), fromVar (prop));
-
-                return value;
-            }
-        }
-
-        if (variant.isUndefined())
-            return {};
-
-        jassertfalse;
-        return {};
-    }
-
-    static var toVar (const choc::value::Value& value)
-    {
-        if (value.isVoid())
-            return {};
-
-        if (value.isInt32())
-            return { value.getInt32() };
-
-        if (value.isInt64())
-            return { (int64) value.getInt64() };
-
-        if (value.isFloat32())
-            return { value.getFloat32() };
-
-        if (value.isFloat64())
-            return { value.getFloat64() };
-
-        if (value.isBool())
-            return { value.getBool() };
-
-        if (value.isString())
-        {
-            const auto tmp = value.toString();
-            return { String { CharPointer_UTF8 { tmp.c_str() } } };
-        }
-
-        if (value.isVector() || value.isArray())
-        {
-            var variant { Array<var>{} };
-
-            for (uint32_t i = 0; i < value.size(); ++i)
-            {
-                jassert (i < (uint32_t) std::numeric_limits<int>::max());
-                variant.insert ((int) i, toVar (choc::value::Value { value[i] }));
-            }
-
-            return variant;
-        }
-
-        if (value.isObject())
-        {
-            auto dynamicObject = std::make_unique<DynamicObject>();
-
-            for (uint32_t i = 0; i < value.size(); ++i)
-            {
-                const auto& [name, type] = value.getObjectMemberAt (i);
-                dynamicObject->setProperty (name, toVar (choc::value::Value { type }));
-            }
-
-            return { dynamicObject.release() };
-        }
-
-        jassertfalse;
-        return {};
-    }
-};
-
 //==============================================================================
 namespace qjs = choc::javascript::quickjs;
 
-static choc::value::Value quickJSToChoc (const qjs::QuickJSContext::ValuePtr& ptr, Result* result = nullptr)
+using VarOrError = std::variant<var, String>;
+
+static var discardError (VarOrError variant)
 {
-    if (result != nullptr)
-        *result = Result::ok();
-
-    try
-    {
-        return ptr.toChocValue();
-    }
-    catch (const choc::javascript::Error& error)
-    {
-        if (result != nullptr)
-            *result = Result::fail (error.what());
-    }
-
-    return {};
+    const auto* v = std::get_if<var> (&variant);
+    return v != nullptr ? *v : var::undefined();
 }
 
-/* Does not release the passed in JSValue. */
-static choc::value::Value quickJSToChoc (qjs::JSValue value, qjs::JSContext* ctx, Result* result = nullptr)
-{
-    qjs::QuickJSContext::ValuePtr valuePtr { value, ctx };
-    ScopeGuard releaseJSValue { [&] { valuePtr.release(); } };
-
-    return quickJSToChoc (valuePtr, result);
-}
-
-/*  Returns a new JSValue object with a reference count of 1. This can be passed into most QuickJS
-    functions such as JS_SetPropertyStr(), which take ownership of this value and will decrement
-    its reference count. When used without passing to QuickJS use ValuePtr to wrap, ensuring
-    cleanup when the ValuePtr goes out of scope.
-*/
-static qjs::JSValue chocToQuickJS (choc::value::Value value, qjs::JSContext* ctx)
-{
-    auto* context = static_cast<qjs::QuickJSContext*> (qjs::JS_GetContextOpaque (ctx));
-    return context->valueToJS (value).release();
-}
-
-static var quickJSToJuce (qjs::JSValueConst value, qjs::JSContext* ctx)
-{
-    return VariantConverter<choc::value::Value>::toVar (quickJSToChoc (value, ctx));
-}
+static VarOrError quickJSToJuce (const qjs::QuickJSContext::ValuePtr& ptr);
 
 static std::vector<var> quickJSToJuce (Span<qjs::JSValueConst> args, qjs::JSContext* ctx)
 {
     std::vector<var> argList;
+    argList.reserve (args.size());
 
     for (const auto& arg : args)
-        argList.push_back (quickJSToJuce (arg, ctx));
+        argList.push_back (discardError (quickJSToJuce ({ qjs::JS_DupValue (ctx, arg), ctx })));
 
     return argList;
 }
 
-/*  See chocToQuickJS() for a discussion about ownership of the returned value. */
-static qjs::JSValue juceToQuickJs (var variant, qjs::JSContext* ctx)
+static qjs::JSValue juceToQuickJs (const var& v, qjs::JSContext* ctx)
 {
-    return chocToQuickJS (VariantConverter<choc::value::Value>::fromVar (variant), ctx);
+    using namespace qjs;
+
+    if (v.isInt())
+        return JS_NewInt32   (ctx, static_cast<int> (v));
+
+    if (v.isInt64())
+        return JS_NewInt64   (ctx, static_cast<int64> (v));
+
+    if (v.isDouble())
+        return JS_NewFloat64 (ctx, static_cast<double>  (v));
+
+    if (v.isBool())
+        return JS_NewBool    (ctx, static_cast<bool>    (v));
+
+    if (v.isString())
+    {
+        const String x = v;
+        return JS_NewStringLen (ctx, x.toRawUTF8(), x.getNumBytesAsUTF8());
+    }
+
+    if (auto fn = v.getNativeFunction())
+    {
+        using Fn = var::NativeFunction;
+        static constexpr auto size = sizeof (fn);
+
+        const auto cb = [] (JSContext* localContext,
+                            JSValueConst thisVal,
+                            int argc,
+                            JSValueConst* argv,
+                            int,
+                            JSValue* funcData) -> JSValue
+        {
+            if (funcData == nullptr)
+            {
+                jassertfalse;
+                return {};
+            }
+
+            size_t bufferSize{};
+            void* buffer = qjs::JS_GetArrayBuffer (localContext, &bufferSize, *funcData);
+
+            if (buffer == nullptr || bufferSize != size)
+            {
+                jassertfalse;
+                return {};
+            }
+
+            const auto thisConverted = discardError (quickJSToJuce ({ qjs::JS_DupValue (localContext, thisVal), localContext }));
+            const auto argsConverted = quickJSToJuce ({ argv, (size_t) argc }, localContext);
+            const var::NativeFunctionArgs args { thisConverted, argsConverted.data(), (int) argsConverted.size() };
+
+            const auto resultVar = (*static_cast<Fn*> (buffer)) (args);
+
+            return juceToQuickJs (resultVar, localContext);
+        };
+
+        const auto free = [] (JSRuntime*, void*, void* buffer)
+        {
+            auto* localFn = static_cast<Fn*> (buffer);
+            localFn->~Fn();
+            delete[] static_cast<uint8_t*> (buffer);
+        };
+
+        std::unique_ptr<uint8_t[]> storage { new uint8_t[size] };
+        new (storage.get()) Fn { std::move (fn) };
+
+        qjs::QuickJSContext::ValuePtr callbackAsData { qjs::JS_NewArrayBuffer (ctx,
+                                                                               storage.release(),
+                                                                               size,
+                                                                               free,
+                                                                               nullptr,
+                                                                               false),
+                                                       ctx };
+        return JS_NewCFunctionData (ctx, cb, 0, 0, 1, &callbackAsData.value);
+    }
+
+    if (auto* array = v.getArray())
+    {
+        auto result = JS_NewArray (ctx);
+
+        for (const auto [index, value] : enumerate (*array, uint32_t{}))
+            JS_SetPropertyUint32 (ctx, result, index, juceToQuickJs (value, ctx));
+
+        return result;
+    }
+
+    if (auto* obj = v.getDynamicObject())
+    {
+        auto result = JS_NewObject (ctx);
+
+        for (const auto& pair : obj->getProperties())
+        {
+            const auto name = pair.name.toString();
+            JS_SetPropertyStr (ctx, result, name.toRawUTF8(), juceToQuickJs (pair.value, ctx));
+        }
+
+        return result;
+    }
+
+    jassert (v.isVoid());
+    return JS_UNDEFINED;
+}
+
+//==============================================================================
+JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wsubobject-linkage")
+struct JSFunctionArguments
+{
+    explicit JSFunctionArguments (qjs::JSContext* contextIn)
+        : context (contextIn)
+    {
+    }
+
+    JSFunctionArguments (qjs::JSContext* contextIn, const var::NativeFunctionArgs& args)
+        : JSFunctionArguments (contextIn, Span { args.arguments, (size_t) args.numArguments })
+    {
+    }
+
+    JSFunctionArguments (qjs::JSContext* contextIn, Span<const var> args)
+        : context (contextIn)
+    {
+        values.reserve (args.size());
+
+        for (const auto& arg : args)
+            values.push_back (juceToQuickJs (arg, context));
+    }
+
+    ~JSFunctionArguments()
+    {
+        for (const auto& value : values)
+            qjs::JS_FreeValue (context, value);
+    }
+
+    void add (const var& arg)
+    {
+        values.push_back (juceToQuickJs (arg, context));
+    }
+
+    qjs::JSValue* getArguments()
+    {
+        return values.data();
+    }
+
+    int getSize() const
+    {
+        return (int) values.size();
+    }
+
+private:
+    qjs::JSContext* context;
+    std::vector<qjs::JSValue> values;
+
+    JUCE_DECLARE_NON_COPYABLE (JSFunctionArguments)
+    JUCE_DECLARE_NON_MOVEABLE (JSFunctionArguments)
+};
+JUCE_END_IGNORE_WARNINGS_GCC_LIKE
+
+//==============================================================================
+// Throws on failure
+static var tryQuickJSToJuce (const qjs::QuickJSContext::ValuePtr& ptr,
+                             const qjs::JSValue* parent = nullptr)
+{
+    using namespace qjs;
+
+    jassert (ptr.context != nullptr);
+
+    if (JS_IsUndefined (ptr.value))
+        return var::undefined();
+
+    if (JS_IsNull (ptr.value))
+        return var{};
+
+    if (JS_IsNumber (ptr.value))
+    {
+        double d = 0;
+        JS_ToFloat64 (ptr.context, std::addressof (d), ptr.value);
+        return d;
+    }
+
+    if (JS_IsBool (ptr.value))
+        return JS_ToBool (ptr.context, ptr.value) != 0;
+
+    if (JS_IsString (ptr.value))
+    {
+        size_t len = 0;
+        const auto* s = JS_ToCStringLen2 (ptr.context, std::addressof (len), ptr.value, false);
+        const ScopeGuard scope { [&] { JS_FreeCString (ptr.context, s); } };
+        return String::fromUTF8 (s, (int) len);
+    }
+
+    if (JS_IsArray (ptr.context, ptr.value))
+    {
+        const auto lengthProp = ptr["length"];
+        uint32_t len = 0;
+        JS_ToUint32 (ptr.context, &len, lengthProp.get());
+
+        Array<var> result;
+        result.ensureStorageAllocated ((int) len);
+
+        for (auto i = decltype (len){}; i < len; ++i)
+            result.add (tryQuickJSToJuce (ptr[i], &ptr.value));
+
+        return result;
+    }
+
+    if (JS_IsFunction (ptr.context, ptr.value))
+    {
+        // ValuePtr is move-only, so can't be captured into a std::function.
+        // Use a custom copyable callable instead.
+        struct Callable
+        {
+            Callable (JSContext* ctxIn, JSValue fnIn, JSValue selfIn)
+                : ctx (ctxIn),
+                  fn (JS_DupValue (ctx, fnIn)),
+                  self (JS_DupValue (ctx, selfIn))
+            {
+            }
+
+            Callable (const Callable& other)
+                : ctx (other.ctx),
+                  fn (JS_DupValue (ctx, other.fn)),
+                  self (JS_DupValue (ctx, other.self))
+            {
+            }
+
+            Callable& operator= (const Callable& other)
+            {
+                Callable { other }.swap (*this);
+                return *this;
+            }
+
+            ~Callable()
+            {
+                JS_FreeValue (ctx, fn);
+                JS_FreeValue (ctx, self);
+            }
+
+            void swap (Callable& other) noexcept
+            {
+                std::swap (other.ctx, ctx);
+                std::swap (other.fn, fn);
+                std::swap (other.self, self);
+            }
+
+            var operator() (const var::NativeFunctionArgs& args) const
+            {
+                JSFunctionArguments convertedArgs { ctx, args };
+
+                const qjs::QuickJSContext::ValuePtr result { qjs::JS_Call (ctx,
+                                                                           fn,
+                                                                           self,
+                                                                           (int) convertedArgs.getSize(),
+                                                                           convertedArgs.getArguments()),
+                                                             ctx };
+
+                return discardError (quickJSToJuce (result));
+            }
+
+            JSContext* ctx{};
+            JSValue fn, self;
+        };
+
+        const qjs::QuickJSContext::ValuePtr parentToUse { parent != nullptr ? JS_DupValue (ptr.context, *parent)
+                                                                            : JS_GetGlobalObject (ptr.context),
+                                                          ptr.context };
+
+        return var::NativeFunction { Callable { ptr.context, ptr.value, parentToUse.value } };
+    }
+
+    if (JS_IsObject (ptr.value))
+    {
+        std::vector<std::string> propNames;
+
+        for (auto obj = ptr.takeValue (JS_DupValue (ptr.context, ptr.value));;)
+        {
+            JSPropertyEnum* properties = nullptr;
+            uint32_t numProps = 0;
+
+            if (JS_GetOwnPropertyNames (ptr.context, &properties, &numProps, obj.get(), JS_GPN_STRING_MASK | JS_GPN_ENUM_ONLY) != 0
+                || properties == nullptr)
+            {
+                return new DynamicObject;
+            }
+
+            const ScopeGuard scope { [&] { js_free (ptr.context, properties); } };
+
+            propNames.reserve (numProps);
+
+            for (uint32_t i = 0; i < numProps; ++i)
+            {
+                const auto* name = JS_AtomToCString (ptr.context, properties[i].atom);
+                std::string nameString (name);
+
+                if (nameString != QuickJSContext::objectNameAttribute)
+                    propNames.push_back (std::move (nameString));
+
+                JS_FreeCString (ptr.context, name);
+                JS_FreeAtom (ptr.context, properties[i].atom);
+            }
+
+            auto proto = ptr.takeValue (JS_GetPrototype (ptr.context, obj.get()));
+
+            if (! JS_IsObject (proto.get()))
+                break;
+
+            obj = std::move (proto);
+        }
+
+        DynamicObject::Ptr result = new DynamicObject;
+
+        for (auto& propName : propNames)
+            result->setProperty (String (propName), tryQuickJSToJuce (ptr[propName.c_str()], &ptr.value));
+
+        return result.get();
+    }
+
+    ptr.throwIfError();
+    return {};
+}
+
+static VarOrError quickJSToJuce (const qjs::QuickJSContext::ValuePtr& ptr)
+{
+    try
+    {
+        return tryQuickJSToJuce (ptr);
+    }
+    catch (const choc::javascript::Error& error)
+    {
+        return String (error.what());
+    }
 }
 
 //==============================================================================
@@ -317,7 +501,7 @@ static qjs::JSClassID createClassId()
 */
 struct DynamicObjectWrapper
 {
-    DynamicObjectWrapper (detail::QuickJSWrapper& engineIn, DynamicObject* objectIn)
+    DynamicObjectWrapper (detail::QuickJSWrapper& engineIn, DynamicObject::Ptr objectIn)
         : engine (engineIn), object (objectIn)
     {
         getDynamicObjects().insert (this);
@@ -388,7 +572,7 @@ struct DynamicObjectWrapper
                                        int ordinal)
     {
         auto& self = *static_cast<DynamicObjectWrapper*> (qjs::JS_GetOpaque2 (ctx, thisVal, getClassId()));
-        self.object->setProperty (self.getIdentifier (ordinal), quickJSToJuce (val, ctx));
+        self.object->setProperty (self.getIdentifier (ordinal), discardError (quickJSToJuce ({ qjs::JS_DupValue (ctx, val), ctx })));
 
         // In case there is a problem we could return e.g. `JS_EXCEPTION` or
         // `JS_ThrowRangeError(ctx, "invalid precision");` here.
@@ -434,7 +618,7 @@ public:
     }
 
     void registerNativeObject (const Identifier& name,
-                               DynamicObject* dynamicObject,
+                               DynamicObject::Ptr dynamicObject,
                                std::optional<qjs::JSValue> parent = std::nullopt)
     {
         auto wrapper  = std::make_unique<DynamicObjectWrapper> (engine, dynamicObject);
@@ -491,7 +675,7 @@ public:
         }
         else
         {
-            auto globalObject = ValuePtr { qjs::JS_GetGlobalObject (ctx), ctx };
+            ValuePtr globalObject { qjs::JS_GetGlobalObject (ctx), ctx };
             qjs::JS_SetPropertyStr (ctx, globalObject.get(), jsObjectName, jsObject);
         }
 
@@ -514,16 +698,14 @@ public:
         if (errorMessage != nullptr)
             *errorMessage = Result::ok();
 
-        try
-        {
-            auto result = engine.getContext().evaluate (code.toStdString());
-            return VariantConverter<choc::value::Value>::toVar (result);
-        }
-        catch (const choc::javascript::Error& error)
-        {
+        const auto result = quickJSToJuce ({ JS_Eval (engine.getQuickJSContext(), code.toRawUTF8(), code.getNumBytesAsUTF8(), "", JS_EVAL_TYPE_GLOBAL), engine.getQuickJSContext() });
+
+        if (auto* v = std::get_if<var> (&result))
+            return *v;
+
+        if (auto* e = std::get_if<String> (&result))
             if (errorMessage != nullptr)
-                *errorMessage = Result::fail (error.what());
-        }
+                *errorMessage = Result::fail (*e);
 
         return var::undefined();
     }
@@ -537,26 +719,29 @@ public:
 
     var callFunction (const Identifier& function, const var::NativeFunctionArgs& args, Result* errorMessage)
     {
-        std::vector<choc::value::Value> argList;
-        argList.reserve ((size_t) args.numArguments);
+        auto* ctx = engine.getQuickJSContext();
+        const auto functionStr = function.toString();
 
-        for (int i = 0; i < args.numArguments; ++i)
-            argList.emplace_back (VariantConverter<choc::value::Value>::fromVar (args.arguments[i]));
+        const auto fn = qjs::JS_NewAtomLen (ctx, functionStr.toRawUTF8(), functionStr.getNumBytesAsUTF8());
+
+        JSFunctionArguments argList { ctx, args };
+
+        qjs::QuickJSContext::ValuePtr global { JS_GetGlobalObject (ctx), ctx };
+        qjs::QuickJSContext::ValuePtr returnVal { JS_Invoke (ctx, global.get(), fn, argList.getSize(), argList.getArguments()), ctx };
+
+        JS_FreeAtom (ctx, fn);
 
         if (errorMessage != nullptr)
             *errorMessage = Result::ok();
 
-        try
-        {
-            auto& ctx = engine.getContext();
-            return VariantConverter<choc::value::Value>::toVar (ctx.invokeWithArgList (function.toString().toRawUTF8(),
-                                                                                       argList));
-        }
-        catch (const choc::javascript::Error& error)
-        {
+        const auto result = quickJSToJuce (returnVal);
+
+        if (auto* v = std::get_if<var> (&result))
+            return *v;
+
+        if (auto* e = std::get_if<String> (&result))
             if (errorMessage != nullptr)
-                *errorMessage = Result::fail (error.what());
-        }
+                *errorMessage = Result::fail (*e);
 
         return var::undefined();
     }
@@ -651,42 +836,6 @@ static uint32_t toUint32 (int64 value)
 
 //==============================================================================
 JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wsubobject-linkage")
-struct JSFunctionArguments
-{
-    explicit JSFunctionArguments (qjs::JSContext* contextIn) : context (contextIn)
-    {
-    }
-
-    ~JSFunctionArguments()
-    {
-        for (const auto& value : values)
-            qjs::JS_FreeValue (context, value);
-    }
-
-    void add (const var& arg)
-    {
-        values.push_back (juceToQuickJs (arg, context));
-    }
-
-    qjs::JSValue* getArguments()
-    {
-        return values.data();
-    }
-
-    int getSize() const
-    {
-        return (int) values.size();
-    }
-
-private:
-    qjs::JSContext* context;
-    std::vector<qjs::JSValue> values;
-
-    JUCE_DECLARE_NON_COPYABLE (JSFunctionArguments)
-    JUCE_DECLARE_NON_MOVEABLE (JSFunctionArguments)
-};
-
-//==============================================================================
 class JSObject::Impl
 {
 public:
@@ -744,10 +893,11 @@ public:
             if (DynamicObjectWrapper::getDynamicObjects().count (opaque) != 0)
                 return { static_cast<DynamicObjectWrapper*> (opaque)->object.get() };
 
-        return quickJSToJuce (valuePtr.get(), engine->getQuickJSContext());
+        auto* ctx = engine->getQuickJSContext();
+        return discardError (quickJSToJuce ({ qjs::JS_DupValue (ctx, valuePtr.get()), ctx }));
     }
 
-    var invokeMethod (const Identifier& methodName, Span<const var> args, Result* result) const
+    VarOrError invokeMethod (const Identifier& methodName, Span<const var> args) const
     {
         if (! hasProperty (methodName))
         {
@@ -759,10 +909,7 @@ public:
         const auto methodAtom = JS_NewAtom (ctx, methodName.toString().toRawUTF8());
         ScopeGuard scope { [&] { qjs::JS_FreeAtom (ctx, methodAtom); } };
 
-        JSFunctionArguments arguments { ctx };
-
-        for (const auto& arg : args)
-            arguments.add (arg);
+        JSFunctionArguments arguments { ctx, args };
 
         ValuePtr returnVal { qjs::JS_Invoke (ctx,
                                              valuePtr.get(),
@@ -771,7 +918,7 @@ public:
                                              arguments.getArguments()),
                              ctx };
 
-        return VariantConverter<choc::value::Value>::toVar (quickJSToChoc (returnVal, result));
+        return quickJSToJuce (returnVal);
     }
 
     NamedValueSet getProperties() const
@@ -779,16 +926,16 @@ public:
         NamedValueSet result;
 
         auto* ctx = engine->getQuickJSContext();
-        auto names = ValuePtr { qjs::JS_GetOwnPropertyNames2 (ctx,
-                                                              valuePtr.get(),
-                                                              qjs::JS_GPN_ENUM_ONLY | qjs::JS_GPN_STRING_MASK,
-                                                              qjs::JS_ITERATOR_KIND_KEY),
-                                ctx };
+        ValuePtr names { qjs::JS_GetOwnPropertyNames2 (ctx,
+                                                       valuePtr.get(),
+                                                       qjs::JS_GPN_ENUM_ONLY | qjs::JS_GPN_STRING_MASK,
+                                                       qjs::JS_ITERATOR_KIND_KEY),
+                         ctx };
 
 
-        if (const auto propertyNames = quickJSToChoc (names); propertyNames.isArray())
+        if (const auto* propertyNames = discardError (quickJSToJuce (names)).getArray())
         {
-            for (const auto& name : propertyNames)
+            for (const auto& name : *propertyNames)
             {
                 if (name.isString())
                 {
@@ -913,7 +1060,15 @@ var JSObject::invokeMethod (const Identifier& methodName,
                             Span<const var> args,
                             Result* result) const
 {
-    return impl->invokeMethod (methodName, args, result);
+    const auto varOrError = impl->invokeMethod (methodName, args);
+
+    if (result != nullptr)
+    {
+        const auto* e = std::get_if<String> (&varOrError);
+        *result = e != nullptr ? Result::fail (*e) : Result::ok();
+    }
+
+    return discardError (varOrError);
 }
 
 NamedValueSet JSObject::getProperties() const
@@ -1276,6 +1431,187 @@ public:
 
             expectEquals ((int) cursor.get(), 6);
         }
+
+        beginTest ("Native functions returning objects with native functions work as expected");
+        {
+            JavascriptEngine temporaryEngine;
+
+            temporaryEngine.registerNativeObject ("ObjGetter", [&]
+            {
+                auto* objGetter = new DynamicObject();
+
+                objGetter->setMethod ("getObj", [&] (const auto&)
+                {
+                    auto* obj = new DynamicObject();
+
+                    obj->setMethod ("getVal", [] (const auto&)
+                    {
+                        return 42;
+                    });
+
+                    return obj;
+                });
+
+                return objGetter;
+            }());
+
+            auto res = juce::Result::fail ("");
+            const auto val = temporaryEngine.evaluate ("let objGetter = ObjGetter; let obj = objGetter.getObj(); obj.getVal();", &res);
+            expect (res.wasOk());
+            expect (static_cast<int> (val) == 42);
+        }
+
+        beginTest ("Methods of javascript objects can be called from C++");
+        {
+            JavascriptEngine temporaryEngine;
+            auto res = juce::Result::fail ("");
+            const auto val = temporaryEngine.evaluate ("var result = { bar: 5, foo (a) { return a + this.bar; } }; result;", &res);
+            expect (res.wasOk());
+
+            auto* obj = val.getDynamicObject();
+
+            if (obj == nullptr)
+            {
+                expect (false);
+                return;
+            }
+
+            expect (obj->hasMethod ("foo"));
+            expect (obj->hasProperty ("bar"));
+
+            expect (obj->getProperty ("bar") == var (5));
+
+            const var a[] { var { 10 } };
+            const auto aResult = obj->invokeMethod ("foo", { val, std::data (a), (int) std::size (a) });
+            expect (aResult == var (15));
+
+            temporaryEngine.evaluate ("result.bar = -5;", &res);
+            expect (res.wasOk());
+
+            const var b[] { var { -10 } };
+            const auto bResult = obj->invokeMethod ("foo", { val, std::data (b), (int) std::size (b) });
+            expect (bResult == var (-15));
+        }
+
+        beginTest ("Destructors of custom callables are called, eventually");
+        {
+            struct CustomCallable
+            {
+                explicit CustomCallable (int& instances)
+                    : liveInstances (instances)
+                {
+                    ++liveInstances;
+                }
+
+                CustomCallable (const CustomCallable& other)
+                    : liveInstances (other.liveInstances)
+                {
+                    ++liveInstances;
+                }
+
+                CustomCallable (CustomCallable&& other) noexcept
+                    : liveInstances (other.liveInstances)
+                {
+                    ++liveInstances;
+                }
+
+                ~CustomCallable()
+                {
+                    --liveInstances;
+                }
+
+                CustomCallable& operator= (const CustomCallable&) = delete;
+                CustomCallable& operator= (CustomCallable&&) noexcept = delete;
+
+                var operator() (const var::NativeFunctionArgs&) const { return "hello world"; }
+
+                int& liveInstances;
+            };
+
+            int methodInstances = 0;
+
+            {
+                JavascriptEngine temporaryEngine;
+
+                temporaryEngine.registerNativeObject ("ObjGetter", [&]
+                {
+                    auto* objGetter = new DynamicObject();
+
+                    objGetter->setMethod ("getObj", [&] (const auto&)
+                    {
+                        auto* obj = new DynamicObject;
+                        obj->setMethod ("getVal", CustomCallable { methodInstances });
+                        return obj;
+                    });
+
+                    return objGetter;
+                }());
+
+                auto res = juce::Result::fail ("");
+                const auto value = temporaryEngine.evaluate ("ObjGetter.getObj().getVal();", &res);
+                expect (res.wasOk());
+                expect (value == "hello world");
+            }
+
+            expect (methodInstances == 0);
+        }
+
+        beginTest ("null and undefined return values are distinctly represented");
+        {
+            JavascriptEngine temporaryEngine;
+            auto res = juce::Result::fail ("");
+            const auto val = temporaryEngine.evaluate ("var result = { returnsNull (a) { return null; }, returnsUndefined (a) { 5 + 2; } }; result;", &res);
+            expect (res.wasOk());
+
+            auto* obj = val.getDynamicObject();
+
+            if (obj == nullptr)
+            {
+                expect (false);
+                return;
+            }
+
+            expect (obj->hasMethod ("returnsNull"));
+            const auto aResult = obj->invokeMethod ("returnsNull", { val, nullptr, 0 });
+            expect (aResult.isVoid());
+
+            expect (obj->hasMethod ("returnsUndefined"));
+            const auto bResult = obj->invokeMethod ("returnsUndefined", { val, nullptr, 0 });
+            expect (bResult.isUndefined());
+        }
+
+        beginTest ("calling a C function that returns void is converted correctly");
+        {
+            int numCalls = 0;
+
+            JavascriptEngine temporaryEngine;
+
+            temporaryEngine.registerNativeObject ("Obj", [&]
+            {
+                auto* objGetter = new DynamicObject();
+
+                objGetter->setMethod ("getObj", [&] (const auto&)
+                {
+                    auto* obj = new DynamicObject;
+
+                    obj->setMethod ("mutate", [&] (const auto&)
+                    {
+                        ++numCalls;
+                        return var{};
+                    });
+
+                    return obj;
+                });
+
+                return objGetter;
+            }());
+
+            auto res = juce::Result::fail ("");
+            const auto val = temporaryEngine.evaluate ("let foo = Obj.getObj(); foo.mutate(); foo.mutate();", &res);
+            expect (res.wasOk());
+
+            expect (numCalls == 2);
+        }
     }
 };