Allow creating `Vec`s of and implement `TryFrom<JsValue>` for strings and exported Rust types #3554

Liamolucko · 2023-08-10T12:29:28Z

This is a rebase of #2734 along with some fixes and tweaks. Sorry it took me so long to get to it!

This PR:

Adds support for passing Vecs of Rust structs to/from JS.
Adds support for passing Vec<String>s to/from JS.
Implements TryFrom<JsValue> for Rust structs and strings.

The Vec impls are little inefficient, since they work by passing Vec<JsValue>s to/from JS like normal, and then translating them to the desired Vec with TryFrom / Into<JsValue>, which requires throwing away the first allocation and making a second one.

Fixes #111
Fixes #2452
Fixes #168
Fixes #2231

This also mostly supersedes #3088. That PR does support slightly more functionality (getting access to &Struct instead of forcing you to take ownership of it and invalidate JS's handle), but it also does so by exposing the internal Anchor type used by RefFromWasmAbi, which I'm not a huge fan of.

I haven't bumped the schema version, since the actual schema hasn't changed and so older versions of the CLI are mostly still compatible, although they do give this cryptic error message if you try and use the new TryFrom<JsValue> impl for Rust structs:

error: import of `__wbg_foo_unwrap` doesn't have an adapter listed

(where foo is the name of the struct.)

So maybe it's worth bumping it anyway, not sure.

daxpedda

I did a quick review and it LGTM.
Maybe I will find some time to make a proper review, but feel free to just merge if you are feeling confident about this.

ranile · 2023-08-11T15:36:36Z

Does this allow returning Vec<_> or such from externs? That's been a big limitation in my use of ducktor for complicated types

Liamolucko · 2023-08-13T11:50:00Z

Does this allow returning Vec<_> or such from externs? That's been a big limitation in my use of ducktor for complicated types

If it's a Vec of #[wasm_bindgen]'d structs or `Strings, yes.

This is accomplished via conversion of the Strings to/from JsValues.

This was done by converting the structs to/from JsValues. It was necessary to change the way relevant traits (e.g. WasmDescribe, IntoWasmAbi etc.) are implemented. It's impossible to implement these for `Box<[#name]>` in codegen.rs because implementing traits on generic types is only allowed in the crate in which the trait is defined. Naively adding a blanket implementation on the wasm-bindgen side doesn't work either because it conflicts with the implementation for JsObjects. The solution was to create traits like VectorIntoWasmAbi etc. that are defined on the concrete type and contain the implementation for IntoWasmAbi etc. for vectors of that type. JsObjects are blanket implemented as before, and struct types are implemented in codegen.rs. Due to the way these traits are defined, Rust requires implementing types to be Sized, so they can't be used for the existing String implementations. Converting structs from JsValues was accomplished by adding an unwrap function to the generated JavaScript class, and calling that from Rust.

I put some work into making sure that you can tell what function the error message is coming from. You still have to dig into the call stack of the error message to see it, but hopefully that's not too big an ask?

The main reason for this, which I didn't mention before, is that I found it really confusing when I was originally reviewing this PR what the difference was between `JsValueVector` and `Vector{From,Into}WasmAbi`, since it really looks like another conversion trait at first glance.

…mpls to JsValue

I moved the `TryFrom<JsValue>` impl out of convert/slices.rs, it doesn't really belong there, and also got rid of the `js_value_vectors!` macro in favour of just implementing it for `String` directly; there's not much point in a macro you only use for one type.

I noticed that strings and rust structs weren't implementing `OptionVectorFromWasmAbi`, so I tried to make a failing test and... it worked. That threw me for a loop for a bit until I realised that it was because I'd used `Vec<T>`, and `Vec<T>`'s impls of `Option{From,Into}WasmAbi` didn't actually rely on `Box<[T]>` implementing the traits: they just required that it implemented `{From,Into}WasmAbi` with an ABI of `WasmSlice`, since from there the element type doesn't matter. So then I thought 'well, why not do that for `Box<[T]>` too? so that's how this commit's pile of new tests came to be.

Since vecs of strings and rust structs were describing themselves as `Box<[JsValue]>`, they got typed as such - as `any[]`. This fixes that by using `NAMED_EXTERNREF` instead of just plain `EXTERNREF` with the type we want. This is maybe _slightly_ sketchy, since `NAMED_EXTERNREF` is meant for imported JS types, but ehhh it's fine. You can already put arbitrary TypeScript in there with `typescript_type`.

This is the nitpickiest of nitpicks, but this is my PR goddammit and I can do what I want :)

I didn't actually bump the schema version because it didn't change. If you don't use the `TryFrom<JsValue>` impl for Rust structs (or pass a `Vec` of them from JS to Rust), using an old CLI version will work fine; if you do though, you get a bit of a cryptic error message: ``` error: import of `__wbg_foo_unwrap` doesn't have an adapter listed ``` (That's from trying to pass a `Vec<Foo>` from JS to Rust.) So idk, maybe that's worth bumping the schema version over anyway?

sinking-point · 2023-09-04T09:42:50Z

Thank you @Liamolucko for sorting this out. It's so good to see my work being put to use.

cschramm · 2023-09-09T10:09:40Z

I think this only partially fixes #111 and #168 (depending on their scopes).

Is it expected that it does not work for returning Vecs from async functions? For both String and #[wasm_bindgen]d structs I get an error that From<Vec<_>> is not implemented for JsValue.

Minimal example:

#[wasm_bindgen]
pub async fn f() -> Vec<String> {
    unimplemented!()
}

I can imagine that it might be an easy target, though.

Side node: Passing Vecs to async fns works fine.

Also implements `TryFrom<JsValue>` and `Into<JsValue>` for them. Enums were left behind in rustwasm#3554. This adds the missing bits.

aspect · 2023-11-04T13:08:18Z

As this came into 0.2.88 it just broke all our framework bindings - #3685

TLDR; this implements a trait on types created by the developer who is using the wasm-bindgen framework, preventing the developer from creating his own TryFrom<JsValue> implementations, which can have many use cases.

Liamolucko changed the title ~~Allow creating Vecs of and implement TryFrom<JsValue> for strings and exported Rust types.~~ Allow creating Vecs of and implement TryFrom<JsValue> for strings and exported Rust types Aug 10, 2023

daxpedda reviewed Aug 11, 2023

View reviewed changes

sinking-point and others added 22 commits August 26, 2023 17:23

Enable passing String vectors and boxed slices across ABI

9f51d64

This is accomplished via conversion of the Strings to/from JsValues.

Remove unneeded require

9752b81

Move uses out of if_std

734eef4

Add documentation

905c1c3

Move incorrect use statements

c8a327f

Fix mistake in comment

cd89028

Throw on invalid array elements instead of silently removing them

55b7c3e

I put some work into making sure that you can tell what function the error message is coming from. You still have to dig into the call stack of the error message to see it, but hopefully that's not too big an ask?

Respect the configured wasm_bindgen crate path

8443320

Change the error type for String and rust structs' TryFrom<JsValue> i…

0fc2ff9

…mpls to JsValue

test string vecs too

7254342

Refactor String impls

263f635

I moved the `TryFrom<JsValue>` impl out of convert/slices.rs, it doesn't really belong there, and also got rid of the `js_value_vectors!` macro in favour of just implementing it for `String` directly; there's not much point in a macro you only use for one type.

fix clippy

58667b2

reorder some impls

af06b34

This is the nitpickiest of nitpicks, but this is my PR goddammit and I can do what I want :)

undo some unnecessary refactors

60f74d6

don't pointlessly use assert.deepStrictEqual for numbers

efc958e

Update the guide

68f2d8f

update reference tests

5f1808a

Liamolucko force-pushed the more-vec-types-and-jsvalue-to-rust branch from cdceeca to 5f1808a Compare August 26, 2023 07:24

add WASI check

8516726

H-Plus-Time mentioned this pull request Aug 31, 2023

Streaming Writes implementation kylebarron/parquet-wasm#305

Merged

daxpedda mentioned this pull request Sep 3, 2023

Support enum variants with values #2407

Open

Extremely nitpicky tweaks

e6df778

Liamolucko merged commit 4d4851d into rustwasm:main Sep 4, 2023
25 checks passed

Liamolucko deleted the more-vec-types-and-jsvalue-to-rust branch September 4, 2023 07:38

Liamolucko mentioned this pull request Sep 7, 2023

Make wasm-bindgen compatible with the standard C ABI #3595

Merged

Liamolucko added a commit to Liamolucko/wasm-bindgen that referenced this pull request Sep 7, 2023

Add missing changelog entries for rustwasm#3554

bdd85a6

Liamolucko added a commit that referenced this pull request Sep 7, 2023

Add missing changelog entries for #3554 (#3599)

0cbc0c9

cschramm added a commit to cschramm/wasm-bindgen that referenced this pull request Sep 19, 2023

Allow creating Vecs of exported enums

e3643cd

Also implements `TryFrom<JsValue>` and `Into<JsValue>` for them. Enums were left behind in rustwasm#3554. This adds the missing bits.

cschramm mentioned this pull request Sep 19, 2023

Allow creating Vecs of exported enums #3624

Merged

cschramm added a commit to cschramm/wasm-bindgen that referenced this pull request Sep 19, 2023

Allow creating Vecs of exported enums

618a3ec

Also implements `TryFrom<JsValue>` and `Into<JsValue>` for them. Enums were left behind in rustwasm#3554. This adds the missing bits.

daxpedda mentioned this pull request Sep 20, 2023

Enable passing String and custom struct boxed slices across ABI and implement TryFrom<JsValue> for custom structs #2734

Closed

Liamolucko mentioned this pull request Sep 20, 2023

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

Closed

ef4 mentioned this pull request Sep 30, 2023

parse result should have more specific type embroider-build/content-tag#30

Closed

fjarri mentioned this pull request Oct 6, 2023

Wasm bindings for user to participate in DKG and signing protocols entropyxyz/entropy-core#414

Merged

6 tasks

Hywan mentioned this pull request Nov 2, 2023

Downcast doesn't play nicely with minification matrix-org/matrix-rust-sdk-crypto-wasm#51

Closed

aspect mentioned this pull request Nov 4, 2023

Lock wasm-bindgen version due to upstream compatibility issues kaspanet/rusty-kaspa#315

Merged

Liamolucko mentioned this pull request Nov 30, 2023

Implementing String Enums #3057

Closed

This was referenced Feb 3, 2024

Returning Vec<T> where T: IntoWasmAbi madonoharu/tsify#49

Open

It's not possible to return Vec<T> if T only implements IntoWasmAbi #3824

Closed

eneoli mentioned this pull request Aug 1, 2024

Added support for VectorIntoWasmAbi and VectorFromWasmAbi siefkenj/tsify#41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow creating `Vec`s of and implement `TryFrom<JsValue>` for strings and exported Rust types #3554

Allow creating `Vec`s of and implement `TryFrom<JsValue>` for strings and exported Rust types #3554

Liamolucko commented Aug 10, 2023

daxpedda left a comment

ranile commented Aug 11, 2023

Liamolucko commented Aug 13, 2023

sinking-point commented Sep 4, 2023

cschramm commented Sep 9, 2023

aspect commented Nov 4, 2023

Allow creating Vecs of and implement TryFrom<JsValue> for strings and exported Rust types #3554

Allow creating Vecs of and implement TryFrom<JsValue> for strings and exported Rust types #3554

Conversation

Liamolucko commented Aug 10, 2023

daxpedda left a comment

Choose a reason for hiding this comment

ranile commented Aug 11, 2023

Liamolucko commented Aug 13, 2023

sinking-point commented Sep 4, 2023

cschramm commented Sep 9, 2023

aspect commented Nov 4, 2023

Allow creating `Vec`s of and implement `TryFrom<JsValue>` for strings and exported Rust types #3554

Allow creating `Vec`s of and implement `TryFrom<JsValue>` for strings and exported Rust types #3554