Support for Option arguments of user types

[fjarri]

Currently, having a binding that takes an optional argument of a custom struct

#[wasm_bindgen]
pub fn func(&self, param: Option<&MyStruct>) -> ...

requires one to implement FromWasmAbi and OptionFromWasmAbi for &MyStruct, which are not very well documented, and require unsafe code and a good understanding of wasm-bindgen internals. It looks like something that can be derived automatically.

Alternatively, one would think that the following would work:

#[wasm_bindgen]
pub fn func(&self, param: Option<MyStruct>) -> ...

This compiles, but leads to unexpected behavior on the JS side (I couldn't find a mention of it in the documentation, which, in fact, states that Option<T> parameters are not supported): the first invocation of the binding nullifies the pointer of the JS struct:

let param = new MyStruct();
console.log(param);
obj.func(param);
console.log(param);

Output:

MyStruct {ptr: <some_nonzero_number>}
MyStruct {ptr: 0}

[RReverser]

For clarity, this issue should probably be rephrased that it's specifically for passing Option with references. By-value consumption and returns are already implemented and work as expected (as your last example shows).

[fjarri]

I must admit I certainly did not expect this behavior on JS side, but I understand how it makes sense if you want to make your JS code behave like Rust (how much sense that makes is a different question). Also, I couldn't find any warning about it in the docs (in fact, Option<T> parameters are explicitly marked as not supported)

[RReverser]

@fjarri That table is for Result<T, JsValue> specifically. Like for other types in that section, what it means is that Option<Result<T, JsValue>> is not supported in params (I admit, it's confusing though, due to usage of T in both).

[RReverser]

That said, here it should certainly be marked as supported in both params and returns.

[fjarri]

Like for other types in that section, what it means is that Option<Result<T, JsValue>> is not supported in params (I admit, it's confusing though, due to usage of T in both).

Ah, I see, my mistake.

[alexcrichton]

FWIW Option<T> "taking" the value is respecting the Rust-semantics where you're acquiring ownership of a value so it's effectively free'd afterwards, but the only way we have to reflect that in JS is to zero out the pointer. In any case thanks for the issue!

[fjarri]

Yes, I figured that. But honestly, I don't see a point in bringing that specific bit of Rust semantics into JS which misses all the other parts that work together with it, like references or the borrow checker.

[RReverser]

But honestly, I don't see a point in bringing that specific bit of Rust semantics into JS which misses all the other parts that work together with it, like references or the borrow checker.

Well it has to do that, otherwise this would be UB on the Rust part. Rust expects that if a parameter is marked to take something by-value, then it won't be accessible again - you're violating that if you still allow access to the same structure on the JS side after the call.

[fjarri]

On the other hand, you're violating the JS calling conventions by invalidating the object, so it's rather a question of priorities. I do not mind a Rust object inside the bindings not being accessible anymore, but why should it affect the JS object?

[RReverser]

Because JS object is merely a wrapper for Rust one. It doesn't have a copy of the struct or something like that, it's just a pointer to the Rust side. When Rust frees the data behind such pointer, it would be worse if JS kept allowing access to the "garbage" that now happens to reside there in memory.

[RReverser]

If you want JS side to have proper JS objects that are merely copies of the Rust ones, it might be that you want Serde integration instead (e.g. check out https://github.com/cloudflare/serde-wasm-bindgen).

[fjarri]

Because JS object is merely a wrapper for Rust one.

That's where I would disagree. A JS object is a wrapper over a WASM one, that's the reality that a user sees. Rust is merely an implementation detail. There's no call for leaking its semantics into JS, especially those parts of it that don't work well by themselves.

But I agree that this discussion is only tangential to this issue. If Option<&> is supported, that will solve the problems that I'm having.

[fjarri]

Bump. Any news on this? At the moment the only way to have optional arguments that behave as expected on the JS side (that is, don't get nullified on use) seems to be to use a hacky proc-macro (#2231 (comment)). I think it would be a very useful feature to a lot of people to be able to have Option<&...> args out of the box.

[fjarri]

For anyone who bumps into the same issue: a workaround is possible, see https://docs.rs/wasm-bindgen-derive/latest/wasm_bindgen_derive/#optional-arguments

[tamaroning]

Hello,
There are some hacky solutions provided by #2370 (comment) and #2231 (comment).
However, I hope wasm-bindgen supports Option<&T> as a parameter in a safe way.

FWIW Option "taking" the value is respecting the Rust-semantics where you're acquiring ownership of a value so it's effectively free'd afterwards, but the only way we have to reflect that in JS is to zero out the pointer. In any case thanks for the issue!
#2370 (comment)

@alexcrichton @RReverser
I think it is OK not to free arguments if they are of type Option<&T>. Could you clarify why it does not work?