Implementing String Enums

[JohnGinger]

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?

[Liamolucko]

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'?)

[JohnGinger]

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.

[JohnGinger]

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,
}

[Liamolucko]

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.

[Liamolucko]

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.

[JohnGinger]

Thanks for being so responsive. I'm happy to help fix this, any pointers as to where I should start?

[Liamolucko]

String enums are parsed here:

wasm-bindgen/crates/macro-support/src/parser.rs

Lines 1168 to 1212 in f75a3f8

	fn import_enum(enum_: syn::ItemEnum, program: &mut ast::Program) -> Result<(), Diagnostic> {
	let mut variants = vec![];
	let mut variant_values = vec![];
	for v in enum_.variants.iter() {
	match v.fields {
	syn::Fields::Unit => (),
	_ => bail_span!(v.fields, "only C-Style enums allowed with #[wasm_bindgen]"),
	}
	let (_, expr) = match &v.discriminant {
	Some(pair) => pair,
	None => {
	bail_span!(v, "all variants must have a value");
	}
	};
	match get_expr(expr) {
	syn::Expr::Lit(syn::ExprLit {
	attrs: _,
	lit: syn::Lit::Str(str_lit),
	}) => {
	variants.push(v.ident.clone());
	variant_values.push(str_lit.value());
	}
	expr => bail_span!(
	expr,
	"enums with #[wasm_bindgen] cannot mix string and non-string values",
	),
	}
	}
	program.imports.push(ast::Import {
	module: None,
	js_namespace: None,
	kind: ast::ImportKind::Enum(ast::ImportEnum {
	vis: enum_.vis,
	name: enum_.ident,
	variants,
	variant_values,
	rust_attrs: enum_.attrs,
	}),
	});
	Ok(())
	}

Their AST is converted to the type passed from the macro to the CLI here:

wasm-bindgen/crates/backend/src/encode.rs

Lines 315 to 317 in f75a3f8

	fn shared_import_enum<'a>(_i: &'a ast::ImportEnum, _intern: &'a Interner) -> ImportEnum {
	ImportEnum {}
	}

That type is defined here (currently empty):

wasm-bindgen/crates/shared/src/lib.rs

Line 91 in f75a3f8

struct ImportEnum {}

ImportKind::Enum is matched against and ignored here:

wasm-bindgen/crates/cli-support/src/wit/mod.rs

Line 503 in f75a3f8

decode::ImportKind::Enum(_) => Ok(()),

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.

[Liamolucko]

Oh, and you'll want to put the list of string enums somewhere in this struct to pass to the JS generation stage:

wasm-bindgen/crates/cli-support/src/wit/nonstandard.rs

Lines 8 to 62 in f75a3f8

	/// A synthetic custom section which is not standardized, never will be, and
	/// cannot be serialized or parsed. This is synthesized from all of the
	/// compiler-emitted wasm-bindgen sections and then immediately removed to be
	/// processed in the JS generation pass.
	#[derive(Default, Debug)]
	pub struct WasmBindgenAux {
	/// Extra typescript annotations that should be appended to the generated
	/// TypeScript file. This is provided via a custom attribute in Rust code.
	pub extra_typescript: String,
	/// A map from identifier to the contents of each local module defined via
	/// the `#[wasm_bindgen(module = "/foo.js")]` import options.
	pub local_modules: HashMap<String, String>,
	/// A map from unique crate identifier to the list of inline JS snippets for
	/// that crate identifier.
	pub snippets: HashMap<String, Vec<String>>,
	/// A list of all `package.json` files that are intended to be included in
	/// the final build.
	pub package_jsons: HashSet<PathBuf>,
	/// A map from exported function id to where it's expected to be exported
	/// to.
	pub export_map: HashMap<AdapterId, AuxExport>,
	/// A map from imported function id to what it's expected to import.
	pub import_map: HashMap<AdapterId, AuxImport>,
	/// Small bits of metadata about imports.
	pub imports_with_catch: HashSet<AdapterId>,
	pub imports_with_variadic: HashSet<AdapterId>,
	pub imports_with_assert_no_shim: HashSet<AdapterId>,
	/// Auxiliary information to go into JS/TypeScript bindings describing the
	/// exported enums from Rust.
	pub enums: Vec<AuxEnum>,
	/// Auxiliary information to go into JS/TypeScript bindings describing the
	/// exported structs from Rust and their fields they've got exported.
	pub structs: Vec<AuxStruct>,
	/// Information about various internal functions used to manage the `externref`
	/// table, later used to process JS bindings.
	pub externref_table: Option<walrus::TableId>,
	pub function_table: Option<walrus::TableId>,
	pub externref_alloc: Option<walrus::FunctionId>,
	pub externref_drop: Option<walrus::FunctionId>,
	pub externref_drop_slice: Option<walrus::FunctionId>,
	/// Various intrinsics used for JS glue generation
	pub exn_store: Option<walrus::FunctionId>,
	pub shadow_stack_pointer: Option<walrus::GlobalId>,
	pub thread_destroy: Option<walrus::FunctionId>,
	}

And then actually generate the JS in here:

wasm-bindgen/crates/cli-support/src/js/mod.rs

Lines 2384 to 2415 in f75a3f8

	pub fn generate(&mut self) -> Result<(), Error> {
	self.prestore_global_import_identifiers()?;
	for (id, adapter) in crate::sorted_iter(&self.wit.adapters) {
	let instrs = match &adapter.kind {
	AdapterKind::Import { .. } => continue,
	AdapterKind::Local { instructions } => instructions,
	};
	self.generate_adapter(*id, adapter, instrs)?;
	}
	let mut pairs = self.aux.export_map.iter().collect::<Vec<_>>();
	pairs.sort_by_key(|(k, _)| *k);
	check_duplicated_getter_and_setter_names(&pairs)?;
	for e in self.aux.enums.iter() {
	self.generate_enum(e)?;
	}
	for s in self.aux.structs.iter() {
	self.generate_struct(s)?;
	}
	self.typescript.push_str(&self.aux.extra_typescript);
	for path in self.aux.package_jsons.iter() {
	self.process_package_json(path)?;
	}
	self.export_destructor();
	Ok(())
	}

[Solo-steven]

I would like to work on this bug ~ thanks ~

[JohnGinger]

Go for it, sorry I haven't got round to it

[SIGSTACKFAULT]

I tried to at least improve the situation by changing the type from any to string but couldn't even find the logic

[Liamolucko]

I tried to at least improve the situation by changing the type from any to string but 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:

wasm-bindgen/crates/backend/src/codegen.rs

Lines 1084 to 1088 in def9147

	impl #wasm_bindgen::describe::WasmDescribe for #name {
	fn describe() {
	<#wasm_bindgen::JsValue as #wasm_bindgen::describe::WasmDescribe>::describe()
	}
	}

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:

wasm-bindgen/src/convert/slices.rs

Lines 227 to 240 in def9147

	impl WasmDescribeVector for String {
	fn describe_vector() {
	inform(VECTOR);
	inform(NAMED_EXTERNREF);
	// Trying to use an actual loop for this breaks the wasm interpreter.
	inform(6);
	inform('s' as u32);
	inform('t' as u32);
	inform('r' as u32);
	inform('i' as u32);
	inform('n' as u32);
	inform('g' as u32);
	}
	}

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!