wasm-bindgen
wasm-bindgen copied to clipboard
Implementing String Enums
Motivation
At the moment number enums are supported. e.g.
#[wasm_bindgen]
pub enum Enum {
A = 10,
B = 20,
}
I would like to support string enums as well e.g.
#[wasm_bindgen]
pub enum Enum {
A = "A",
B = "B",
}
Proposed Solution
I would be happy to implement this, is it something that a PR would be accepted for?
String enums are already supported; see https://rustwasm.github.io/wasm-bindgen/reference/types/imported-js-types.html.
(I'm not sure why they're in that section; surely they should be under 'Exported Rust Types'?)
They are in the docs, but if I try to use them they don't output any typescript types.
If you have a mixed enum, e.g.
#[wasm_bindgen]
#[derive(Copy, Clone, Debug)]
pub enum StringEnum {
Foo = 1,
Bar = "bar",
Qux = "qux",
}
You get the error 'enums with #[wasm_bindgen] may only have number literal values' - which is from here. https://github.com/rustwasm/wasm-bindgen/blob/main/crates/macro-support/src/parser.rs#L1288
I was assuming the docs were wrong - but if there is a way to make this work that would be great.
I've put a minimal reproducible example at https://github.com/JohnGinger/string-enum-example. The exported types are only
export enum NumberEnum {
Foo,
Bar,
Qux,
}
They are in the docs, but if I try to use them they don't output any typescript types.
That's a bug, the types should be emitted.
If you have a mixed enum, e.g.
#[wasm_bindgen] #[derive(Copy, Clone, Debug)] pub enum StringEnum { Foo = 1, Bar = "bar", Qux = "qux", }You get the error 'enums with #[wasm_bindgen] may only have number literal values' - which is from here. https://github.com/rustwasm/wasm-bindgen/blob/main/crates/macro-support/src/parser.rs#L1288
That's a bad error message - it should be saying something like 'enums must have either all string or all int payloads'.
In fact, it does give a better error message is you swap the order of the variants around - this:
#[wasm_bindgen]
#[derive(Copy, Clone, Debug)]
pub enum StringEnum {
Bar = "bar",
Qux = "qux",
Foo = 1,
}
gives this error message:
error: enums with #[wasm_bindgen] cannot mix string and non-string values
--> src/lib.rs:8:11
|
8 | Foo = 1,
| ^
So, it should be fixed to always give that message.
That's a bug, the types should be emitted.
Okay, it's slightly more complicated than I first thought - unlike regular enums, string enums are allowed to be private, and string enums defined deep in the dependency tree shouldn't be getting top-level type declarations. So, some logic will be needed to only add the type declarations if they're exported from the crate root.
Thanks for being so responsive. I'm happy to help fix this, any pointers as to where I should start?
String enums are parsed here: https://github.com/rustwasm/wasm-bindgen/blob/f75a3f8986c56d752b687cebe47b53232f403ba7/crates/macro-support/src/parser.rs#L1168-L1212
Their AST is converted to the type passed from the macro to the CLI here: https://github.com/rustwasm/wasm-bindgen/blob/f75a3f8986c56d752b687cebe47b53232f403ba7/crates/backend/src/encode.rs#L315-L317
That type is defined here (currently empty): https://github.com/rustwasm/wasm-bindgen/blob/f75a3f8986c56d752b687cebe47b53232f403ba7/crates/shared/src/lib.rs#L91
ImportKind::Enum is matched against and ignored here: https://github.com/rustwasm/wasm-bindgen/blob/f75a3f8986c56d752b687cebe47b53232f403ba7/crates/cli-support/src/wit/mod.rs#L503
I think that's most of the places you'd have to change, so hopefully that's helpful!
Also, I already wrote a fix for the bad error message in 6ed8f5b, if you want to start from there.
Oh, and you'll want to put the list of string enums somewhere in this struct to pass to the JS generation stage: https://github.com/rustwasm/wasm-bindgen/blob/f75a3f8986c56d752b687cebe47b53232f403ba7/crates/cli-support/src/wit/nonstandard.rs#L8-L62
And then actually generate the JS in here: https://github.com/rustwasm/wasm-bindgen/blob/f75a3f8986c56d752b687cebe47b53232f403ba7/crates/cli-support/src/js/mod.rs#L2384-L2415
I would like to work on this bug ~ thanks ~
Go for it, sorry I haven't got round to it
I tried to at least improve the situation by changing the type from any to string but couldn't even find the logic
I tried to at least improve the situation by changing the type from
anytostringbut couldn't even find the logic
The type is any because string enums currently work by converting the enums to/from JsValues before passing them to/from JS, and their WasmDescribe impls describe them as such, which the CLI translates to any. The code that generates their WasmDescribe impls is here:
https://github.com/rustwasm/wasm-bindgen/blob/def914721caf434c0f14fa9eb5df07d07d5113d4/crates/backend/src/codegen.rs#L1084-L1088
It's possible to keep them as JsValues but change the TypeScript type by describing them as NAMED_EXTERNREFs instead - I did the exact same thing in #3554 for Vec<String>s:
https://github.com/rustwasm/wasm-bindgen/blob/def914721caf434c0f14fa9eb5df07d07d5113d4/src/convert/slices.rs#L227-L240
It's a bit weird that they're passed as JsValues in the first place, though. Really they should just be passed to/from JS as &strs/Strings directly, which would also change the TypeScript type to string. So, that would be the preferable way of changing the TypeScript type to string and I don't think should be too difficult to implement; but changing the WasmDescribe impl would still be an improvement!