-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Allow creating Vec
s of and implement TryFrom<JsValue>
for strings and exported Rust types
#3554
Merged
Liamolucko
merged 24 commits into
rustwasm:main
from
Liamolucko:more-vec-types-and-jsvalue-to-rust
Sep 4, 2023
Merged
Allow creating Vec
s of and implement TryFrom<JsValue>
for strings and exported Rust types
#3554
Liamolucko
merged 24 commits into
rustwasm:main
from
Liamolucko:more-vec-types-and-jsvalue-to-rust
Sep 4, 2023
Commits on Aug 26, 2023
-
Enable passing String vectors and boxed slices across ABI
This is accomplished via conversion of the Strings to/from JsValues.
Configuration menu - View commit details
-
Copy full SHA for 9f51d64 - Browse repository at this point
Copy the full SHA 9f51d64View commit details -
Enable passing custom struct vectors over ABI
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.
Configuration menu - View commit details
-
Copy full SHA for 5d26952 - Browse repository at this point
Copy the full SHA 5d26952View commit details -
Configuration menu - View commit details
-
Copy full SHA for 9752b81 - Browse repository at this point
Copy the full SHA 9752b81View commit details -
Configuration menu - View commit details
-
Copy full SHA for 734eef4 - Browse repository at this point
Copy the full SHA 734eef4View commit details -
Configuration menu - View commit details
-
Copy full SHA for 905c1c3 - Browse repository at this point
Copy the full SHA 905c1c3View commit details -
Configuration menu - View commit details
-
Copy full SHA for c8a327f - Browse repository at this point
Copy the full SHA c8a327fView commit details -
Configuration menu - View commit details
-
Copy full SHA for cd89028 - Browse repository at this point
Copy the full SHA cd89028View commit details -
Throw on invalid array elements instead of silently removing them
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?
Configuration menu - View commit details
-
Copy full SHA for 55b7c3e - Browse repository at this point
Copy the full SHA 55b7c3eView commit details -
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.
Configuration menu - View commit details
-
Copy full SHA for dba751d - Browse repository at this point
Copy the full SHA dba751dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 8443320 - Browse repository at this point
Copy the full SHA 8443320View commit details -
Configuration menu - View commit details
-
Copy full SHA for 0fc2ff9 - Browse repository at this point
Copy the full SHA 0fc2ff9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7254342 - Browse repository at this point
Copy the full SHA 7254342View commit details -
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.
Configuration menu - View commit details
-
Copy full SHA for 263f635 - Browse repository at this point
Copy the full SHA 263f635View commit details -
Don't require manual
OptionVector{From,Into}WasmAbi
implsI 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.
Configuration menu - View commit details
-
Copy full SHA for 761dc4a - Browse repository at this point
Copy the full SHA 761dc4aView commit details -
Configuration menu - View commit details
-
Copy full SHA for 58667b2 - Browse repository at this point
Copy the full SHA 58667b2View commit details -
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`.
Configuration menu - View commit details
-
Copy full SHA for 8c96579 - Browse repository at this point
Copy the full SHA 8c96579View commit details -
This is the nitpickiest of nitpicks, but this is my PR goddammit and I can do what I want :)
Configuration menu - View commit details
-
Copy full SHA for af06b34 - Browse repository at this point
Copy the full SHA af06b34View commit details -
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?
Configuration menu - View commit details
-
Copy full SHA for a5c9fad - Browse repository at this point
Copy the full SHA a5c9fadView commit details -
Configuration menu - View commit details
-
Copy full SHA for 60f74d6 - Browse repository at this point
Copy the full SHA 60f74d6View commit details -
Configuration menu - View commit details
-
Copy full SHA for efc958e - Browse repository at this point
Copy the full SHA efc958eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 68f2d8f - Browse repository at this point
Copy the full SHA 68f2d8fView commit details -
Configuration menu - View commit details
-
Copy full SHA for 5f1808a - Browse repository at this point
Copy the full SHA 5f1808aView commit details -
Configuration menu - View commit details
-
Copy full SHA for 8516726 - Browse repository at this point
Copy the full SHA 8516726View commit details
Commits on Sep 4, 2023
-
Configuration menu - View commit details
-
Copy full SHA for e6df778 - Browse repository at this point
Copy the full SHA e6df778View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.