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

Allow returning fixed-size tuples and arrays #122

Open
dbrgn opened this issue Apr 12, 2018 · 19 comments
Open

Allow returning fixed-size tuples and arrays #122

dbrgn opened this issue Apr 12, 2018 · 19 comments
Labels
more-types Adding support for more Rust types to cross the boundary

Comments

@dbrgn
Copy link
Contributor

dbrgn commented Apr 12, 2018

It appears that returning (f32, f32, f32, f32) or [f32; 4] in a #[wasm_bindgen] function does not currently work. Would be great if fixed size arrays and tuples (maybe up to length 32) were supported.

@alexcrichton alexcrichton added the more-types Adding support for more Rust types to cross the boundary label Apr 16, 2018
@alexcrichton
Copy link
Contributor

I think fixed-size-arrays are in theory easy enough (we'd probably pass them through allocated memory), tuples are a bit odder though. You're mostly looking for homogenous tuples, right? And both of these in JS would pop out as arrays?

@dbrgn
Copy link
Contributor Author

dbrgn commented Apr 17, 2018

Yes. In my case I'd like to pass a list of 2D-coordinates to JS. Using a fixed size array would work fine too of course.

@David-OConnor
Copy link
Member

David-OConnor commented Jul 10, 2018

Would love to see this as well, for both arrays and tuples. The output for either could be a JS array.

@rhysd
Copy link
Contributor

rhysd commented Jan 25, 2019

This would be great.

In my use case, I wanted to return RGB value as 3 elements tuple or array of u8. However, they are not yet supported so I needed to create a struct like

#[wasm_bindgen]
pub struct Rgb { pub r: u8, pub g: u8, pub b: u8 }

But it is not so efficient since in JavaScript, accessing each field causes interaction between wasm and JS.

console.log(color.r, color.g, color.b);

For this use case, I could finally use 0xrrggbb format of u32 values for RGB values. But it's a solution only for this use case.


I also think mapping tuples to JS array is fine and reasonable. TypeScript supports tuples and actually it is transpiled into array.

https://www.typescriptlang.org/docs/handbook/basic-types.html

@RReverser
Copy link
Member

As another use-case, I think tuples support could improve interfaces for js-sys iterators too.

In particular, .entries() on Map / Set / Array could produce an iterator over tuples of (JsValue, JsValue) right away instead of users having to repeat same boilerplate with JsArray + Reflect::get_u32(..., 0) + Reflect::get_u32(..., 1) to extract key and value on each iteration.

@RReverser
Copy link
Member

Just an update - I imagine this might become easier these days (or, at least, soon) by leveraging multi-value Wasm feature.

@josephrocca
Copy link

In case it's relevant to the discussion (e.g. RE wasm-bindgen backwards compatibility concerns, if there are any), and in case others are unaware, JS will probably get tuples in the next couple of years. The tc39 proposal repo has 1.4k stars (very high - up there with the decorators proposal) and the proposal is at stage 2 as of writing.

@ghost
Copy link

ghost commented Apr 25, 2021

@josephrocca, Tuples refer to Rust tuples, Wasm multi-value tuples return JavaScript arrays, and it would make the most sense to target the Wasm feature so that some cases won't need a wasm-bindgen shim at all.

@ForkKILLET
Copy link

Are we waiting for the TC39 tuple proposal?

@stegaBOB
Copy link

stegaBOB commented May 9, 2023

@alexcrichton

I think fixed-size-arrays are in theory easy enough (we'd probably pass them through allocated memory)

is this still the case? I'm not too familiar with the internals of wasm_bindgen (yet), but I'd be interested in taking a stab at this!

@daxpedda
Copy link
Collaborator

daxpedda commented May 9, 2023

@stegaBOB I'm not entirely sure how easy or hard to implement this really is, but I'm happy to review a PR!

@stegaBOB
Copy link

@daxpedda I think we're pretty close to getting something that's working & ready for a review! As of now we're focused on getting fixed size arrays working only for compiling w/ multi-value. Would proceeding like that and throwing a runtime error on wasm-bindgen-cli if fixed size arrays exist and multi value is off be an acceptable change?

@daxpedda
Copy link
Collaborator

Last time I checked multi-value doesn't currently work in Rust: rust-lang/rust#73755.

I wouldn't mind merging something that doesn't work for now, but I also don't want to merge something I can't even test.

@stegaBOB
Copy link

stegaBOB commented May 25, 2023

@daxpedda I meant multi value using the multi-value-xform wasm-bindgen crate, not natively multi value. We ran into that rust issue as well unfortunately, but the transformer seems to work fine (although we also made a couple modifications to that transformer too)

@daxpedda
Copy link
Collaborator

Would proceeding like that and throwing a runtime error on wasm-bindgen-cli if fixed size arrays exist and multi value is off be an acceptable change?

Sounds good then!

@lancenonce
Copy link

Any examples on where this is implemented? Couldn't get it working from current documentation

@stegaBOB
Copy link

Any examples on where this is implemented? Couldn't get it working from current documentation

We're currently doing the wasm-bindgen work here: https://github.com/staratlasmeta/wasm-bindgen. It's not in a working state at the moment and there's still work we'll need to do, but its getting there.

@stegaBOB
Copy link

stegaBOB commented Jun 4, 2023

We currently have fixed size arrays for most numeric primitives working when using the WASM_BINDGEN_MULTI_VALUE=true option! Still a bit of work to be done (testing, clean up, making it work when multi value is disabled, etc), but I think we're close! I've opened a draft PR in the meantime if anyone wants to take a look at what we have so far: #3458.

@lancenonce
Copy link

Thank you for the update! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-types Adding support for more Rust types to cross the boundary
Projects
None yet
Development

No branches or pull requests

10 participants