[Bug]: [Script] loosing properties of objects set via registerNativeObjects (methods and child objects are ok)

[benkuper]

Detailed steps on how to reproduce the bug

There seem to be a problem with retrieving properties on the C++ side for objects that have been registered via registerNativeObject.

On a minimal setup, i have this test structure, that is basically :

test:{
  value:"ok", 
  child:{
  value:"childOK"
  }
}

which gives this when registering in JUCE :

var testO(new DynamicObject());
testO.getDynamicObject()->setProperty("value", "ok");

var testC(new DynamicObject());
testC.getDynamicObject()->setProperty("value", "childOK");

testO.getDynamicObject()->setProperty("child", testC.getDynamicObject());

scriptEngine->registerNativeObject("test", testO.getDynamicObject());

Now for the fun part. My script is the following (script.log is explained at the end of this issue) :

script.log("Test : ", test);
script.log("Test value : ", test.value);
script.log("Test child : ", test.child);
script.log("Test child value : ", test.child.value);

var jsTest = {value:"jsOK"};
script.log("JS Test : ", jsTest);

and here is the result of it :

Test : {child:Object 0x2394f60}
Test value :  ok
Test child : {}
Test child value : childOK
JS Test : {value:jsOK}

As you can see, objects passed from registerNativeObject are not going the full way, even though their values are accessible in JS. Moreover, locally generated objects (jsTest) will have no problem being read with their properties on the C++ side.

I also tried looping through properties with a for(var p in test), and it gives me the same result, only showing the child object but not the "value" property.

The fact that script.log works is also showing that methods are well kept and usable, as "script" is an object registered via registerNativeObject. This fscript.log callback is just looping through the NativeFunctionArgs.arguments and listing the properties.

Another interesting fact is that the "thisObject" property of the NativeFunctionArgs seems to not have this problem (code is harder to extract from my actual software where I'm testing this).

What is the expected behaviour?

I would expect the properties of an objects passed through NativeFunctionArgs to still have properties, not just methods and child objects.

Operating systems

Windows

What versions of the operating systems?

Win 11

Architectures

64-bit

Stacktrace

No response

Plug-in formats (if applicable)

No response

Plug-in host applications (DAWs) (if applicable)

No response

Testing on the develop branch

The bug is present on the develop branch

Code of Conduct

• [X] I agree to follow the Code of Conduct

[benkuper]

Going further on this issue :

I tried to track down where is the property lost, and in my test case, in the tryQuickJSToJuce function, i should expect numProps to be 2 at line 375 of juce_Javascript.cpp > one for "value" and one for "child". But as I break there and check, numProps is only one, and only property here is "child". So it must happen before that.

Now just for testing purpose, I tried to fill the args using DynamicObjectWrapper instead of the quickJSToJuce function :

        Array<var> juceArgs;
        for (int i = 0; i < numArgs; i++)
        {
            auto& a = *static_cast<DynamicObjectWrapper*> (qjs::JS_GetOpaque2(ctx, args[i], getClassId()));
            juceArgs.add(a.object.get());
        }

This effectively works when providing objects as arguments. It crashes when providing non-object argument, quite logically. but this is advance :)

[benkuper]

I'm not posting a PR because I'm not sure this fix is the right solution, but removing JS_GPN_ENUM_ONLY in the JS_GetOwnPropertyNames at line 367 of juce_Javascript.cpp makes all properties included in the result.

the modified line is the following :

if (JS_GetOwnPropertyNames (ptr.context, &properties, &numProps, obj.get(), JS_GPN_STRING_MASK) != 0
    || properties == nullptr)

But the question now is : why is this property not marked as enumerable ? I'm not sure I can go further in the investigation without spending hours on this. I hope someone with this kind of knowledge will read this :)

[reuk]

It's a while since I last looked at this code, and I'm not super familiar with javascript, so I can't provide much insight without spending more time investigating.

I think the enumerable/non-enumerable flag is referring to a property of the JS language itself, where not all properties are enumerable depending on how they were created. It looks like we convert JUCE properties to JS by calling JS_SetPropertyStr, so perhaps this is creating non-enumerable properties that we then don't visit when converting back to JUCE types.

I tried printing JSON.stringify(test) after registering your demo native object, which produces {"child":{}}. JSON.stringify only visits enumerable properties, which makes me think that JUCE is creating non-enumerable properties when converting to JS types.

We'll have to consider the best solution here. The current non-symmetric behaviour definitely seems incorrect, but I don't know whether the JUCE->JS conversion should create enumerable properties, or whether the JS->JUCE conversion should include non-enumerable properties. Maybe this needs to be a user-facing option.

[benkuper]

Interesting !

I think there is one more valuable information : in my software, I have a method getChild() that returns child objects that should be the same as the ones provided in registerNativeObject

so calling root.parameters.myObject should give me a parameter that is a child of parameters, that is a child of root. This doesn't work, as described above (i.e., myObject will not show the "value" field for instance) But when I do root.getChild("parameters").myParameter, this works, it will show the value field !

which I think means that objects returned by c-function calls are not set up the same way as with registerNativeObject

[reuk]

Thanks for your patience. I've pushed a change that should fix this issue here: https://github.com/juce-framework/JUCE/commit/07c863ac2345f48db798918ffd08178f37448f9b

[benkuper]

Thank you, seems all good !