-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
Support enum variants with values #2407
Comments
It seems reasonable to me to try to support this, thanks for writing this up! This is likely a pretty significant feature to implement, however, and I don't have a ton of time to help with design details myself. I'd be happy to help review though! |
Nice to hear. I will also be a bit busy the coming weeks, but I'll get at it, eventually. Need this feature for a problem I'm working on. |
Awesome to see someone else had the idea before me! I do like Approach 1, though I'm not sure if one would get the proper hints by the IDE when calling the constructors. Still, perfectly reasonable and would be a huge improvement! |
That's a great feature that will surely help a lot in one of my projects. You're proposals are awesome, @joeriexelmans, but what do you think about following what the I'd be more than happy to work on this with you, and am also currently reading the |
From quickly reading the ReScript documentation, it seems to me that a downside is that matching a specific variant in JS is somewhat more complex, as a variant value may be either just a I'm still busy, until mid-March, so feel free to "take over" from me. I'll let you guys know soon as I've got time to work on this again. |
Hey @joeriexelmans Yes, the bindings are indeed more complex. I got the wrong understanding about how C-like enums were compiled in Sure, I'm pretty busy lately too, but I'll keep trying to understand the macro when I have free time. |
Hey @alexcrichton, how are you doing? |
Hi everyone, what would be needed to unlock this feature? |
Is #2631 still relevant? I'm interested in this and would like to play around with it but starting from scratch might be too much. Getting a couple of pointers would also work. |
A common error handling pattern is to wrap errors from other libraries in an enum variant. Would be great to be able to return a JsValue error inside of our library's error enum, rather than try to map every single one to a unique variant. If there's a better way to do this please let me know. |
@voxelstack Yes, kind of. It does work in that revision as far as I remember, so definitely your best starting point, but you'd have to rebase it onto master. Could be possible, could be a lot of work. |
Another option would be to treat non-c-style enums like structs, so they're wrapped in a |
I think at a minimum, // Externally tagged: enum variant is the key, enum content is the value
// Default representation
{"Request": {"id": "...", "method": "...", "params": {...}}}
// Internally tagged: the enum variant gets a key with the rest of the enum content
// `#[wasm_bindgen(tag = "type")]` on the Rust side
{"type": "Request", "id": "...", "method": "...", "params": {...}}
// Adjacently tagged: the enum content is represented within a separate key
// `#[wasm_bindgen(tag = "t", content = "c")]`
{"type": "Request", "c": {"id": "...", "method": "...", "params": {...}}}
// Untagged: no variant indicator, figure it out based on the content
// `#[wasm_bindgen(untagged)]`
{"id": "...", "method": "...", "params": {...}} I think that there needs to be a couple possible representations, and mimicking the serde style gives a familiar (and proven) way to do that. In any case, exporting the enum variant constructors as js functions (e.g. For anyone who needs a fast workaround, using |
The last one (untagged) would not work for enum where some variants have values and others not and the first one might lead. The first one (internally tagged) may lead to problems with field name conflicts and as per serde documentation does not work with enums containing tuple variants. So I think the second one (adjacently tagged) or the default which you did not mention (externally tagged) is best suited. |
I did miss externally tagged, thanks for pointing that out. The point of my comment was more that I don't think wasm_bindgen does need to pick the "best" representation - instead it needs options, since no single representation is good in every case. |
Although I agree that multiple options would be the best, I think we need to focus on getting the feature working first. TS/JS representation is something we can do solely on the JS side, which I believe is easier to modify than actually passing the necessary data back and forth. I, unfortunately, still have no time to continue working on my PR, but would love to be involved if someone can. |
Is there a way to see if a JsValue is of an enum member declared in the rust code as of now or is everything like that to-do? I have an enum that is used in the js code and then passed to the rust code on several other occasions, but I can't use the enum in the functions argument. |
What’s the status on this? I think I need this implemented, unless there’s other alternatives/workarounds |
@ThatXliner as a general rule, if there are no other comments on an issue, the status hasn't changed. A comment like "What's the status?" is rarely productive and notifies everyone in the conversation, causing noise and distraction, please consider not posting comments like that in the future in highly active repositories. If you think someone should see this issue, send it to them or mention them here, ideally with an explanation. If you want to express your support or desire for this feature, upvote the original post. If you want to show how this feature is useful in a way that hasn't been considered in this thread yet, feel free to post a minimal example. Something like this can be useful for generating test cases. Thank you. |
Correct me if I am wrong, but I believe this is just waiting on somebody to implement it (reads this as: it could be you!). It doesn’t seem like it would all that much work, especially since SerDe provides a good reference. |
Probably needs a reviewer, I'm afraid this will be too big for me to find time to review. |
True, but so far nobody has done it. There was an attempt in #2631, but the original author had no time to work on it further and it was never merged. |
Hey all, I've recently come across a need for this feature and I'm also between jobs and so have some spare time to work on something like this. However I'm not sure I understand what benefits this approach would provide over the one outlined in the documentation for communicating arbitrary data via Especially since Rust structs exported to JS must be Copy https://rustwasm.github.io/wasm-bindgen/reference/types/exported-rust-types.html so any enum with String is out of the question and the The following example appears to work just fine, and since the implementations of #[derive(Serialize, Deserialize)]
#[serde(tag = "type", content = "data")]
pub enum TestEnum {
Foo { number: i32 },
Bar { string: String },
}
impl From<TestEnum> for JsValue {
fn from(val: TestEnum) -> Self {
serde_wasm_bindgen::to_value(&val).unwrap()
}
}
impl wasm_bindgen::describe::WasmDescribe for TestEnum {
fn describe() {
JsValue::describe()
}
}
impl wasm_bindgen::convert::IntoWasmAbi for TestEnum {
type Abi = <JsValue as IntoWasmAbi>::Abi;
fn into_abi(self) -> Self::Abi {
serde_wasm_bindgen::to_value(&self).unwrap().into_abi()
}
}
impl wasm_bindgen::convert::FromWasmAbi for TestEnum {
type Abi = <JsValue as FromWasmAbi>::Abi;
unsafe fn from_abi(js: Self::Abi) -> Self {
serde_wasm_bindgen::from_value(JsValue::from_abi(js)).unwrap()
}
}
#[wasm_bindgen(js_name = "newTestEnum")]
pub fn new_test_enum() -> TestEnum {
TestEnum::Foo { number: 42 }
}
#[wasm_bindgen(js_name = "printTestEnum")]
pub fn print_test_enum(test_enum: TestEnum) {
match test_enum {
TestEnum::Foo { number } => console_log!("Foo encountered with value: {}", number),
TestEnum::Bar { string } => console_log!("Bar encountered with value: {}", string),
}
} The main limitation I can think of is that it won't produce Typescript type definitions for the Enum, you can define them manually https://rustwasm.github.io/wasm-bindgen/reference/attributes/on-js-imports/typescript_type.html, but keeping them in sync would be a pain. However I'm not sure this limitation warrants the effort. Maybe it'd be enough to outline this approach for enums in the documentation? |
Here's a procedural macro for it, the macro attributes get passed directly to // lib.rs
use proc_macro::TokenStream;
mod serde_wasm_bindgen;
#[proc_macro_attribute]
pub fn serde_wasm_bindgen(attr: TokenStream, item: TokenStream) -> TokenStream {
serde_wasm_bindgen::expand_macro(attr.into(), item.into())
.unwrap_or_else(syn::Error::into_compile_error)
.into()
} // serde_wasm_bindgen.rs
use proc_macro2::TokenStream;
use quote::{quote, ToTokens};
use syn::{spanned::Spanned, Error, Item};
pub fn expand_macro(serde_attr: TokenStream, tokens: TokenStream) -> syn::Result<TokenStream> {
let item = syn::parse2::<Item>(tokens)?;
match item {
Item::Struct(it) => inner_macro(it.ident.clone(), serde_attr, it.to_token_stream()),
Item::Enum(it) => inner_macro(it.ident.clone(), serde_attr, it.to_token_stream()),
item => Err(Error::new(
item.span(),
"serde_wasm_bindgen macro can only be applied to structs or enums",
)),
}
}
fn inner_macro(
ident: proc_macro2::Ident,
serde_attr: TokenStream,
tokens: TokenStream,
) -> Result<TokenStream, Error> {
let pound = syn::Token![#](tokens.span()).to_token_stream();
Ok(quote! {
#pound[derive(Serialize, Deserialize)]
#pound[serde(#serde_attr)]
#tokens
impl From<#ident> for JsValue {
fn from(val: #ident) -> Self {
serde_wasm_bindgen::to_value(&val).unwrap()
}
}
impl TryFrom<JsValue> for #ident {
type Error = serde_wasm_bindgen::Error;
fn try_from(value: JsValue) -> Result<Self, Self::Error> {
serde_wasm_bindgen::from_value(value)
}
}
impl wasm_bindgen::describe::WasmDescribe for #ident {
fn describe() {
JsValue::describe()
}
}
impl wasm_bindgen::convert::IntoWasmAbi for #ident {
type Abi = <JsValue as IntoWasmAbi>::Abi;
fn into_abi(self) -> Self::Abi {
Into::<JsValue>::into(self).into_abi()
}
}
impl wasm_bindgen::convert::FromWasmAbi for #ident {
type Abi = <JsValue as FromWasmAbi>::Abi;
unsafe fn from_abi(js: Self::Abi) -> Self {
JsValue::from_abi(js).try_into().unwrap()
}
}
})
} #[serde_wasm_bindgen(tag = "type", content = "data")]
pub enum TestEnum {
Foo { number: i32 },
Bar { string: String },
}
#[wasm_bindgen(js_name = "printTestEnum")]
pub fn print_test_enum(test_enum: TestEnum) {
match test_enum {
TestEnum::Foo { number } => console_log!("Foo encountered with value: {}", number),
TestEnum::Bar { string } => console_log!("Bar encountered with value: {}", string),
}
} I made |
@DylanRJohnston I've created a macro that does this for me. It generates TypeScript based on serde and uses |
FWIW there's also tsify, which does this as well. |
FWIW tsify seems to be a little abandoned right now, but there's an active and updated fork called tsify-next. |
Hi folks! This feature was discussed in the past in other issues, but it's been quiet for a while and there hasn't been an issue dedicated specifically to this feature, hence this:
Motivation
Right now, only C-style enums are supported. A C-style enum like this
is now converted to a mapping from tag to integer values:
In your code you can just use
CStyleEnum.A
, like you would in Rust.Rust's support for Algebraic Data Types (ADTs) is an awesome feature that I love and use intensely. It would be nice if JavaScript bindings could be generated for an enum like this:
Proposed Solutions
JavaScript does not have builtin support for "tagged unions" like Rust does, so a decision has to be made on how we "model" this. There was discussion on this in the earlier C-style enum issue, with 2 proposed solutions. I'll summarize them here.
Solution 1: Tag + array of values
This is the most straightforward solution. It reflects Rust's memory layout for enums.
Typescript definition:
I like how currently, for C-style enums, a mapping from enum identifier to an integer is created. We could further generalize this by creating a mapping from enum identifier to enum variant constructor:
and use it as follows:
JavaScript allows any function to be called with any list of parameters, so there's no way to prevent e.g. forgetting the number parameter for variant B, doing something like
new MyEnum.B()
, but that's just the way it is, in a dynamically typed language.To "match" an enum variant, one can use JS switch-statements:
if-else:
or use a mapping:
Alternatives
Solution 2: Keep the tag as a key in the object
The following was also proposed:
I'm personally not in favor, because one is forced to add a dummy value for variant 'A', which is inefficient:
And this dummy value has to be truthy, in order for
to match.
Also, I think checking if an object contains a key is less efficient in (non-optimized?) JavaScript, compared to just checking the value of an integer field ("tag").
To "match" a variant, to my knowledge, one can only use if-statements.
Additional info
I'm willing to put some work into this. Currently looking at the source of the wasm_bindgen macro to see if I can implement it myself.
The text was updated successfully, but these errors were encountered: