wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

Implement `Into<JsValue>` for `Vec`

Open Liamolucko opened this issue 2 years ago • 2 comments

This means that Vecs can now be returned from async functions.

Fixes #3155.

I had to add a new VectorIntoJsValue trait, since similarly to VectorIntoWasmAbi the orphan rule won't let us directly implement From<Vec<T>> for JsValue outside of wasm-bindgen.

I did some quick benchmarking, and it seems like the copy-into-a-new-Vec approach used to return Vecs from synchronous functions is faster:

cpu: Apple M1 Pro
runtime: deno 1.36.4 (aarch64-apple-darwin)

file:///Users/liam/src/wasm-bindgen-playground/bench.ts
benchmark             time (avg)        iter/s             (min … max)       p75       p99      p995
---------------------------------------------------------------------- -----------------------------
direct_enum_vec   121.87 µs/iter       8,205.6   (115.33 µs … 1.28 ms) 120.25 µs 189.42 µs 196.38 µs
jsvalue_enum_vec  359.78 µs/iter       2,779.5 (285.38 µs … 480.12 µs) 358.71 µs 444.92 µs 454.62 µs

So it might be a good idea to later change this to use a similar implementation. However I haven't checked if these results are the same in other JS runtimes so take that with a grain of salt.

Liamolucko avatar Sep 21 '23 10:09 Liamolucko

Is there any progress here? This would be a fantastic feature

benthecarman avatar Nov 28 '23 20:11 benthecarman

I had to add a new VectorIntoJsValue trait, since similarly to VectorIntoWasmAbi the orphan rule won't let us directly implement From<Vec<T>> for JsValue outside of wasm-bindgen.

Something I was always wondering about in wasm-bindgen is why JsValue conversions can't just reuse the ABI representation instead of adding lots of new intrinsics.

For example, it's already possible to implement any such conversions as

#[wasm_bindgen]
extern {
  #[wasm_bindgen(js_name = Object)] // using Object(x) as just an identity function for `x=>x`
  fn convert(values: Vec<u32>) -> JsValue;
}

It should be possible to generate similar stubs generically for arbitrary types, without resorting to Object(x) , Number(x) etc as identity functions, but having wasm-bindgen generate proper identity function whose entire job is to convert argument from one ABI and into another.

RReverser avatar Dec 09 '23 19:12 RReverser

Any updates on this?

G2-Games avatar Mar 24 '24 00:03 G2-Games

Any updates on this?

I was holding out on merging this until I got around to properly benchmarking whether the current approach or the copy-into-a-Vec-first approach was faster (i.e. across all browsers instead of just Deno), but obviously that still hasn't happened so I'm going to merge it as is. A slightly-suboptimal implementation is better than no implementation.

Liamolucko avatar Mar 25 '24 00:03 Liamolucko

@Liamolucko thank you so much for implementing this! I'm currently trying it out and can't seem to get it working. I wonder what I'm missing.

Here's my cargo:

[package]
name = "hui"
version = "0.1.0"
edition = "2021"

[lib]
crate-type = ["cdylib", "rlib"]

[dependencies]
wasm-bindgen = { git = "https://github.com/rustwasm/wasm-bindgen" }
wasm-bindgen-futures = "*"

and my lib

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub async fn async_number_vec() -> Vec<i32> {
    vec![1, -3, 7, 12]
}

And rust complains with

/t/hui (master|✔) [101]$ cargo check
    Checking hui v0.1.0 (/private/tmp/hui)
error[E0277]: the trait bound `{async block@src/lib.rs:3:1: 3:16}: futures::future::Future` is not satisfied
   --> src/lib.rs:3:1
    |
3   | #[wasm_bindgen]
    | ^^^^^^^^^^^^^^^ the trait `futures::future::Future` is not implemented for `{async block@src/lib.rs:3:1: 3:16}`
    |
    = help: the following other types implement trait `futures::future::Future`:
              JsFuture
              futures::future::flatten::Flatten<A>
              Box<F>
              futures::stream::concat::Concat2<S>
              futures::stream::concat::Concat<S>
              futures::sync::oneshot::SpawnHandle<T, E>
              futures::sync::oneshot::Execute<F>
              futures::sync::mpsc::Execute<S>
            and 44 others
note: required by a bound in `future_to_promise`
   --> /Users/terhechte/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wasm-bindgen-futures-0.2.19/src/lib.rs:204:14
    |
203 | pub fn future_to_promise<F>(future: F) -> Promise
    |        ----------------- required by a bound in this function
204 |     where F: Future<Item = JsValue, Error = JsValue> + 'static,
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `future_to_promise`
    = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `JsValue: From<js_sys::Promise>` is not satisfied
 --> src/lib.rs:3:1
  |
3 | #[wasm_bindgen]
  | ^^^^^^^^^^^^^^^ the trait `From<js_sys::Promise>` is not implemented for `JsValue`
  |
  = help: the following other types implement trait `From<T>`:
            <JsValue as From<bool>>
            <JsValue as From<isize>>
            <JsValue as From<i8>>
            <JsValue as From<i16>>
            <JsValue as From<i32>>
            <JsValue as From<i64>>
            <JsValue as From<i128>>
            <JsValue as From<usize>>
          and 39 others
  = note: required for `js_sys::Promise` to implement `Into<JsValue>`
  = note: this error originates in the attribute macro `wasm_bindgen` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `hui` (lib) due to 2 previous errors

terhechte avatar Mar 26 '24 10:03 terhechte

@Liamolucko thank you so much for implementing this! I'm currently trying it out and can't seem to get it working. I wonder what I'm missing.

The underlying problem going on there is that you're ending up depending on two different versions of wasm-bindgen: the git version you specified manually, and the version from crates.io depended on by wasm-bindgen-futures.

The proper way to switch to a git version of wasm-bindgen is using [patch]:

[package]
name = "hui"
version = "0.1.0"
edition = "2021"

[lib]
crate-type = ["cdylib", "rlib"]

[dependencies]
wasm-bindgen = "*"
wasm-bindgen-futures = "*"

[patch.crates-io]
wasm-bindgen = { git = "https://github.com/rustwasm/wasm-bindgen" }

As for why you got that error, it's due to a weird interaction with links. wasm-bindgen uses links to tell Cargo that you can't have more than one version of wasm-bindgen at once; but because you specified wasm-bindgen-futures = "*", cargo was able to sidestep that by depending on wasm-bindgen-futures 0.2.19, the last version that didn't use links. That version predates async-await and Future being added to the standard library, and so the Future trait it's using isn't actually std::future::Future and isn't implemented by async blocks.

Liamolucko avatar Mar 26 '24 11:03 Liamolucko

Thanks @Liamolucko! Awesome explanation and not so obvious. I wouldn't found that issue without your help. Works great!

terhechte avatar Mar 27 '24 10:03 terhechte

@Liamolucko If you could help me with this problem, it would be greatly appreciated. Could you help me understand why this fails to compile with wasm-pack build --target web. I am aware of the workarounds. Is there a way to implement the new Vector traits for arbitrary data? When returning Vec<Data>, it would return Vec<JsValue> automatically, without explicitly changing the function signature.

// this doesn't work
#[wasm_bindgen]
pub async fn test_wait_vec() -> Vec<Data>  //the trait `JsObject` is not implemented for `Data`, which is required by `Vec<Data>: wasm_bindgen::describe::WasmDescribe`
{
    vec![Data::new()]
}
// This works
#[wasm_bindgen]
pub async fn test_wait() -> Data {
    Data::new()
}

#[derive(Serialize, Deserialize, Tsify)]
#[into_wasm_abi]
pub struct Data {
    pub time: SystemTime,
    pub map: HashMap<Test, InnerData>,
}

impl Into<JsValue> for Data {
    fn into(self) -> JsValue {
        serde_wasm_bindgen::to_value(&self).unwrap()
    }
}

impl VectorIntoJsValue for Data {
    fn vector_into_jsvalue(vector: Box<[Data]>) -> JsValue {
        wasm_bindgen::__rt::js_value_vector_into_jsvalue(vector)
    }
}

The error is in the comment.

When I implement any combination of the following traits below...

impl WasmDescribeVector for Data {
    fn describe_vector() {
        inform(VECTOR);
        Data::describe();
    }
}

impl VectorIntoJsValue for Data {
    fn vector_into_jsvalue(vector: Box<[Data]>) -> JsValue {
        wasm_bindgen::__rt::js_value_vector_into_jsvalue(vector)
    }
}

impl VectorIntoWasmAbi for Data {
    type Abi = WasmSlice;

    fn vector_into_abi(vector: Box<[Self]>) -> WasmSlice {
        let jsvaluevec: Box<[JsValue]> = vector
            .iter()
            .map(|x| serde_wasm_bindgen::to_value(&x).unwrap())
            .collect();
        JsValue::vector_into_abi(jsvaluevec)
    }
}

I get a different error:

error: import of __wbindgen_array_new doesn't have an adapter listed Error: Running the wasm-bindgen CLI Caused by: Running the wasm-bindgen CLI

Thank you for your time! :)

samankittani avatar Jul 04 '24 13:07 samankittani

From a quick glance it looks like this would be related to #3919?

daxpedda avatar Jul 29 '24 09:07 daxpedda