JUCE 8 JavascriptEngine var::isMethod() Incompatibility

In the process of updating to JUCE 8 we’ve noticed that all of our keyboard shortcuts have broken which makes the app pretty unusable. We use the JS engine extensively.
I’ve got two issues, one which I have a work around for so I’ll post that separately.

The problem is best described by this failing test:

void runObjectTest()
{
    auto createObj = []
    {
        auto obj = new juce::DynamicObject();
        obj->setMethod ("getVal",
                        [] (const auto&) { return 42; });
        return obj;
    };

    auto createObjGetter = [&]
    {
        auto objGetter = new juce::DynamicObject();
        objGetter->setMethod ("getObj",
                              [&] (const auto&) { return createObj(); });
        return objGetter;
    };

    auto createEngine = [&]
    {
        auto engine = std::make_unique<juce::JavascriptEngine>();
        engine->registerNativeObject ("Obj", createObj());
        engine->registerNativeObject ("ObjGetter", createObjGetter());

        return engine;
    };

    {
        auto engine = createEngine();
        auto res = juce::Result::fail ("");
        auto val = engine->evaluate ("let objGetter = ObjGetter; \
                                      let obj = objGetter.getObj(); \
                                      obj.getVal();",
                                     &res);
        jassert (res.wasOk());
        jassert (static_cast<int> (val) == 42);
    }
}

The problem seems to be that the js calls a C++ function that returns an object that has a function member. It seems like there’s no provision for this when converting from juce::var to choc::value::Value in VariantConverter<choc::value::Value>::fromVar.

I don’t actually think this can be handled by choc so it might need to be a special case in the calling juceToQuickJs function.

The above example looks contrived but it’s a distilled problem we face as we have lots of C++ objects represented in JS that call in to native code exactly like this.

Hello,
nice to know I’m not alone :slight_smile:
I had posted on Github issue, and posted here as well because I hadn’t found your post while searching for this…

My take is that choc::Value would need to be updated to support functions. Otherwise, a lot needs to be done in to work around it and the data flow would not be as elegant and consistent as now

Link to the other post :
(JUCE 8) QuickJS implementation missing methods - General JUCE discussion - JUCE

1 Like

Thanks for seconding this.

I imagine the juce team is working through the backlog that grew whilst J8 was being finalised. This is a show stopper for use at the moment as well though so hopefully they’ll get to it soon.

1 Like

Same here… this is a big one though. I guess there are other big ones, but it definitely renders the whole JavascriptEngine unusable…

Thanks for your patience, both. This ended up being quite an involved fix:

Please try it out and let us know if you encounter any further issues.

Hello, thank you for the time spent on this, this seems indeed like a big one :slight_smile:
However, I’m sad to say it’s not fully over… when using getRootObjectProperties, I get a crash. I explained it on the closed issue where you asked for further testing :
[Bug]: (JUCE8) variant.isVoid() not handled in VariantConverter fromVar() · Issue #1399 · juce-framework/JUCE (github.com)

To explain my case :
I created a system to list all declared functions in the script so I can filter out if they exist or not in the script before calling them.
In the previous engine, it proved to optimize a huge deal when calling inexistant functions at high rate.
Is the new engine handling that good or should I keep my internal handling ? If it’s good with the new one, at least I could go further on the juce8 upgrade, otherwise I’d wait :slight_smile: (and anyway, the bug would still be there)

Adding further (and that was a PR from me at JUCE 7 era), having a public function in JavascriptEngine list all methods (not all properties because then they are not distinguishable from declared object variables and objects registered via registerNativeObject) would be great for this !

Thank you, we can see the issue. A fix is arriving shortly.