[quickjs-wasm-rs] Should the closure passed to `wrap_callback` return `JSValueRef`?

[surma]

Currently, the closure passed to wrap_callback is impl FnMut(&Self, JSValueRef<'_>, &[JSValueRef<'_>]) -> Result<JSValue> + 'static.

This has worked well so far, however, this makes it impossible to return an already existing value and maintain referential equality. For example:

let ctx = JSContextRef::default();
// This is supposed to be a Rust-implementation of `x => x`.
let identity_func = ctx.wrap_callback(|ctx, _this, args| {
    let undef = ctx.undefined_value()?;
    // As far as I can tell, `from_qjs_value` is the only path from `JSValueRef` 
    // to `JSValue`, which deep-copies the value into a context-independent data structure.
    from_qjs_value(args.get(0).cloned().unwrap_or(undef))
})?;
let object = ctx.object_value()?;
let result = identity_func.call(&ctx.undefined_value()?, &[func.clone()])?;
// Will fail as these are different objects now.
assert_eq!(Into::<u64>::into(result), Into::<u64>::into(object));

As JSValue is a context-independent representation of a JS value, would it make sense for a callback to always return a context-specific value, which can be easily created from a given JSValue using to_qjs_value()?

(Side-note: Would be great to also implement PartialEq for JSValueRef.)

[saulecabrera]

I think we did it in this way initially assuming that all use-cases would want to return high-level Rust types from the callback, but taking a close look what you're suggesting, it doesn't seem like the right assumption and makes sense to me also because wrap_callback immediately converts the JSValue to a JSValueRef in which case if you want to pass an existing JSValueRef the process feels somewhat inefficient. I think that we could make it so that it works for both use cases, and let the caller decide what return type they are after depending on the use-case.

[jeffcharles]

Another example of where it would be useful to be able to return a JSValueRef would be if someone wants to return another function from their function. See this Zulip chat message for where this has come up.

[saulecabrera]

Now that https://github.com/bytecodealliance/javy/pull/618 is merged, this is no longer an issue. I'll go ahead and close this issue.