From 8dedaecde3cd988c2a1773e1f62218bc7fcb60a5 Mon Sep 17 00:00:00 2001 From: Michael Schmidt Date: Wed, 2 Oct 2024 16:17:38 +0200 Subject: [PATCH 1/3] Improved string enum to WASM conversion --- crates/cli-support/src/js/binding.rs | 105 +++++++++++--------------- crates/cli/tests/reference/enums.d.ts | 10 +++ crates/cli/tests/reference/enums.js | 18 +++++ crates/cli/tests/reference/enums.rs | 22 ++++++ crates/cli/tests/reference/enums.wat | 4 + 5 files changed, 97 insertions(+), 62 deletions(-) diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs index f3d60ccbbd3..cef04592571 100644 --- a/crates/cli-support/src/js/binding.rs +++ b/crates/cli-support/src/js/binding.rs @@ -576,6 +576,40 @@ fn instruction( log_error: &mut bool, constructor: &Option, ) -> Result<(), Error> { + fn wasm_to_string_enum(variant_values: &[String], index: &str) -> String { + // e.g. ["a","b","c"][someIndex] + let mut enum_val_expr = String::new(); + enum_val_expr.push('['); + for variant in variant_values { + enum_val_expr.push_str(&format!("\"{variant}\",")); + } + enum_val_expr.push(']'); + enum_val_expr.push('['); + enum_val_expr.push_str(index); + enum_val_expr.push(']'); + enum_val_expr + } + fn string_enum_to_wasm(variant_values: &[String], invalid: u32, enum_val: &str) -> String { + // e.g. (["a","b","c"].indexOf(someEnumVal) + 1 || 4) - 1 + // | + // invalid + 1 + // + // The idea is that `indexOf` returns -1 if someEnumVal is invalid, + // and with +1 we get 0 which is falsey, so we can use || to + // substitute invalid+1. Finally, we just do -1 to get the correct + // values for everything. + let mut enum_val_expr = String::new(); + enum_val_expr.push_str("(["); + for variant in variant_values.iter() { + enum_val_expr.push_str(&format!("\"{variant}\",")); + } + enum_val_expr.push_str(&format!( + "].indexOf({enum_val}) + 1 || {invalid}) - 1", + invalid = invalid + 1 + )); + enum_val_expr + } + match instr { Instruction::ArgGet(n) => { let arg = js.arg(*n).to_string(); @@ -670,43 +704,15 @@ fn instruction( Instruction::WasmToStringEnum { variant_values } => { let index = js.pop(); - - // e.g. ["a","b","c"][someIndex] - let mut enum_val_expr = String::new(); - enum_val_expr.push('['); - for variant in variant_values { - enum_val_expr.push_str(&format!("\"{variant}\",")); - } - enum_val_expr.push(']'); - enum_val_expr.push('['); - enum_val_expr.push_str(&index); - enum_val_expr.push(']'); - - js.push(enum_val_expr) + js.push(wasm_to_string_enum(variant_values, &index)) } - Instruction::OptionWasmToStringEnum { - variant_values, - hole, - } => { + Instruction::OptionWasmToStringEnum { variant_values, .. } => { + // Since hole is currently variant_count+1 and the lookup is + // ["a","b","c"][index], the lookup will implicitly return map + // the hole to undefined, because OOB indexes will return undefined. let index = js.pop(); - - let mut enum_val_expr = String::new(); - enum_val_expr.push('['); - for variant in variant_values { - enum_val_expr.push_str(&format!("\"{variant}\",")); - } - enum_val_expr.push(']'); - enum_val_expr.push('['); - enum_val_expr.push_str(&index); - enum_val_expr.push(']'); - - // e.g. someIndex === 4 ? undefined : (["a","b","c"][someIndex]) - // | - // currently, hole = variant_count + 1 - js.push(format!( - "{index} === {hole} ? undefined : ({enum_val_expr})" - )) + js.push(wasm_to_string_enum(variant_values, &index)) } Instruction::StringEnumToWasm { @@ -714,22 +720,7 @@ fn instruction( invalid, } => { let enum_val = js.pop(); - - // e.g. {"a":0,"b":1,"c":2}[someEnumVal] ?? 3 - // | - // currently, invalid = variant_count - let mut enum_val_expr = String::new(); - enum_val_expr.push('{'); - for (i, variant) in variant_values.iter().enumerate() { - enum_val_expr.push_str(&format!("\"{variant}\":{i},")); - } - enum_val_expr.push('}'); - enum_val_expr.push('['); - enum_val_expr.push_str(&enum_val); - enum_val_expr.push(']'); - enum_val_expr.push_str(&format!(" ?? {invalid}")); - - js.push(enum_val_expr) + js.push(string_enum_to_wasm(variant_values, *invalid, &enum_val)) } Instruction::OptionStringEnumToWasm { @@ -738,19 +729,9 @@ fn instruction( hole, } => { let enum_val = js.pop(); + let enum_val_expr = string_enum_to_wasm(variant_values, *invalid, &enum_val); - let mut enum_val_expr = String::new(); - enum_val_expr.push('{'); - for (i, variant) in variant_values.iter().enumerate() { - enum_val_expr.push_str(&format!("\"{variant}\":{i},")); - } - enum_val_expr.push('}'); - enum_val_expr.push('['); - enum_val_expr.push_str(&enum_val); - enum_val_expr.push(']'); - enum_val_expr.push_str(&format!(" ?? {invalid}")); - - // e.g. someEnumVal == undefined ? 4 : ({"a":0,"b":1,"c":2}[someEnumVal] ?? 3) + // e.g. someEnumVal == undefined ? 4 : (string_enum_to_wasm(someEnumVal)) // | // double equals here in case it's null js.push(format!( diff --git a/crates/cli/tests/reference/enums.d.ts b/crates/cli/tests/reference/enums.d.ts index 14d117668b7..324cfe91af6 100644 --- a/crates/cli/tests/reference/enums.d.ts +++ b/crates/cli/tests/reference/enums.d.ts @@ -11,6 +11,16 @@ export function enum_echo(color: Color): Color; */ export function option_enum_echo(color?: Color): Color | undefined; /** +* @param {Color} color +* @returns {ColorName} +*/ +export function get_name(color: Color): ColorName; +/** +* @param {ColorName | undefined} [color] +* @returns {ColorName} +*/ +export function option_string_enum_echo(color?: ColorName): ColorName; +/** */ export enum Color { Green = 0, diff --git a/crates/cli/tests/reference/enums.js b/crates/cli/tests/reference/enums.js index c7446efaa24..28e0b501378 100644 --- a/crates/cli/tests/reference/enums.js +++ b/crates/cli/tests/reference/enums.js @@ -44,6 +44,24 @@ export function option_enum_echo(color) { return ret === 3 ? undefined : ret; } +/** +* @param {Color} color +* @returns {ColorName} +*/ +export function get_name(color) { + const ret = wasm.get_name(color); + return ["green","yellow","red",][ret]; +} + +/** +* @param {ColorName | undefined} [color] +* @returns {ColorName} +*/ +export function option_string_enum_echo(color) { + const ret = wasm.option_string_enum_echo(color == undefined ? 4 : ((["green","yellow","red",].indexOf(color) + 1 || 4) - 1)); + return ["green","yellow","red",][ret]; +} + /** */ export const Color = Object.freeze({ Green:0,"0":"Green",Yellow:1,"1":"Yellow",Red:2,"2":"Red", }); diff --git a/crates/cli/tests/reference/enums.rs b/crates/cli/tests/reference/enums.rs index 48a58d455ca..e4cc8e9794c 100644 --- a/crates/cli/tests/reference/enums.rs +++ b/crates/cli/tests/reference/enums.rs @@ -17,3 +17,25 @@ pub fn enum_echo(color: Color) -> Color { pub fn option_enum_echo(color: Option) -> Option { color } + +#[wasm_bindgen] +#[derive(PartialEq, Debug)] +pub enum ColorName { + Green = "green", + Yellow = "yellow", + Red = "red", +} + +#[wasm_bindgen] +pub fn get_name(color: Color) -> ColorName { + match color { + Color::Red => ColorName::Red, + Color::Green => ColorName::Green, + Color::Yellow => ColorName::Yellow, + } +} + +#[wasm_bindgen] +pub fn option_string_enum_echo(color: Option) -> Option { + color +} diff --git a/crates/cli/tests/reference/enums.wat b/crates/cli/tests/reference/enums.wat index 4c9fd212451..479c1c3ec89 100644 --- a/crates/cli/tests/reference/enums.wat +++ b/crates/cli/tests/reference/enums.wat @@ -2,10 +2,14 @@ (type (;0;) (func (param i32) (result i32))) (func $enum_echo (;0;) (type 0) (param i32) (result i32)) (func $option_enum_echo (;1;) (type 0) (param i32) (result i32)) + (func $get_name (;2;) (type 0) (param i32) (result i32)) + (func $option_string_enum_echo (;3;) (type 0) (param i32) (result i32)) (memory (;0;) 17) (export "memory" (memory 0)) (export "enum_echo" (func $enum_echo)) (export "option_enum_echo" (func $option_enum_echo)) + (export "get_name" (func $get_name)) + (export "option_string_enum_echo" (func $option_string_enum_echo)) (@custom "target_features" (after code) "\02+\0fmutable-globals+\08sign-ext") ) From 44a3428b73a1f900d305a38a19d504a7f31e5928 Mon Sep 17 00:00:00 2001 From: Michael Schmidt Date: Wed, 2 Oct 2024 16:18:17 +0200 Subject: [PATCH 2/3] Fixed type of optional output string enums --- crates/cli-support/src/wit/outgoing.rs | 2 +- crates/cli/tests/reference/enums.d.ts | 4 ++-- crates/cli/tests/reference/enums.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/cli-support/src/wit/outgoing.rs b/crates/cli-support/src/wit/outgoing.rs index b79a7b68cc6..6bc647cf9c6 100644 --- a/crates/cli-support/src/wit/outgoing.rs +++ b/crates/cli-support/src/wit/outgoing.rs @@ -305,7 +305,7 @@ impl InstructionBuilder<'_, '_> { variant_values: variant_values.to_vec(), hole: *hole, }, - &[AdapterType::StringEnum(String::from(name))], + &[AdapterType::StringEnum(String::from(name)).option()], ); } Descriptor::RustStruct(name) => { diff --git a/crates/cli/tests/reference/enums.d.ts b/crates/cli/tests/reference/enums.d.ts index 324cfe91af6..1ffd4249837 100644 --- a/crates/cli/tests/reference/enums.d.ts +++ b/crates/cli/tests/reference/enums.d.ts @@ -17,9 +17,9 @@ export function option_enum_echo(color?: Color): Color | undefined; export function get_name(color: Color): ColorName; /** * @param {ColorName | undefined} [color] -* @returns {ColorName} +* @returns {ColorName | undefined} */ -export function option_string_enum_echo(color?: ColorName): ColorName; +export function option_string_enum_echo(color?: ColorName): ColorName | undefined; /** */ export enum Color { diff --git a/crates/cli/tests/reference/enums.js b/crates/cli/tests/reference/enums.js index 28e0b501378..07d1cf32427 100644 --- a/crates/cli/tests/reference/enums.js +++ b/crates/cli/tests/reference/enums.js @@ -55,7 +55,7 @@ export function get_name(color) { /** * @param {ColorName | undefined} [color] -* @returns {ColorName} +* @returns {ColorName | undefined} */ export function option_string_enum_echo(color) { const ret = wasm.option_string_enum_echo(color == undefined ? 4 : ((["green","yellow","red",].indexOf(color) + 1 || 4) - 1)); From 00c19fe834ee22bf23952f9bb80be5baff63d309 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Mon, 7 Oct 2024 11:35:56 +0200 Subject: [PATCH 3/3] Add changelog entry --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ee3a7d10fb..266ae464d33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,12 @@ * Fixed emitted `package.json` structure to correctly specify its dependencies [#4091](https://github.com/rustwasm/wasm-bindgen/pull/4091) +* Fixed returning `Option` now correctly has the `| undefined` type in TS bindings. + [#4137](https://github.com/rustwasm/wasm-bindgen/pull/4137) + +* Fixed enum variant name collisions with object prototype fields. + [#4137](https://github.com/rustwasm/wasm-bindgen/pull/4137) + -------------------------------------------------------------------------------- ## [0.2.93](https://github.com/rustwasm/wasm-bindgen/compare/0.2.92...0.2.93)