Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Into<JsValue> for Vec #3630

Merged
merged 6 commits into from
Mar 25, 2024
Merged

Conversation

Liamolucko
Copy link
Collaborator

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.

@benthecarman
Copy link

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

@RReverser
Copy link
Member

RReverser commented Dec 9, 2023

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.

@G2-Games
Copy link
Contributor

Any updates on this?

This means that `Vec`s can now be returned from `async` functions.

Fixes rustwasm#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`.
@Liamolucko
Copy link
Collaborator Author

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 Liamolucko merged commit 88efe46 into rustwasm:main Mar 25, 2024
23 checks passed
@Liamolucko Liamolucko deleted the vec-into-js-value branch March 25, 2024 00:10
@terhechte
Copy link

@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

@Liamolucko
Copy link
Collaborator Author

@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.

@terhechte
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Returning Result<Vec<u8>, JsError> from an async function doesn't work
5 participants