rustwasm / wasm-bindgen

Facilitating high-level interactions between Wasm modules and JavaScript
https://rustwasm.github.io/docs/wasm-bindgen/
Apache License 2.0
7.77k stars 1.07k forks source link

Functions returning Result<T,E> result in incorrect typescript function output type `Array` #4207

Open wandomPewlin opened 1 week ago

wandomPewlin commented 1 week ago

Describe the Bug

In version 0.2.95, any function returning a Result<T,E> will cause the generated typescript function to return Array (note that it's not Array<T>, just Array). The problem doesn't occur in version 0.2.93.

Steps to Reproduce

Using the code from wasm-bindgen doc

use wasm_bindgen::prelude::*;

#[wasm_bindgen]
pub fn throwing_function() -> Result<(), JsError> {
    Err(JsError::new("message"))
}

Expected Behavior

in version 0.2.93, the generated function signature in typescript would be

export function throwing_function(a: number): void;

Actual Behavior

but in 0.2.95, it becomes.

export function throwing_function(): Array;

Additional Context

The problem doesn't seem to be JsError itself, I originally run into the problem while using serde-wasm-bindgen and the function returns Result<i32,JsValue>. The generated ts function also returns Array as output.

RunDevelopment commented 1 week ago

Cannot reproduce.

I just tested your code on main and the function returns void as expected (in JS and in the type definitions). So this is probably an issue with serde-wasm-bindgen.

wandomPewlin commented 1 week ago

Cannot reproduce.

I just tested your code on main and the function returns void as expected (in JS and in the type definitions). So this is probably an issue with serde-wasm-bindgen.

Huh, that's weird. Before opening this issue, I specifically started a fresh project without serde-wasm-bindgen, and the result still got turned into Array. The problem could only be resolved if I forced the dependency wasm-bindgen = "=0.2.93" in Cargo.toml.

davids-work commented 1 week ago

This is dependent on the rust version! Something has happened between Rust 1.81 and 1.82 that triggers this behavior.

Using the code from the original post, I can reproduce it like this using wasm-bindgen 0.2.95:

rustup toolchain add 1.82.0
rustup toolchain add 1.81.0
rustup override set 1.81.0
wasm-pack build --target web
cp pkg/array.d.ts array-1.81.d.ts
rustup override set 1.82.0
wasm-pack build --target web
diff -u pkg/array.d.ts array-1.81.d.ts

Gives me

 export interface InitOutput {
   readonly memory: WebAssembly.Memory;
-  readonly throwing_function: (a: number) => void;
-  readonly __wbindgen_add_to_stack_pointer: (a: number) => number;
+  readonly throwing_function: () => Array;
+  readonly __wbindgen_export_0: WebAssembly.Table;
+  readonly __externref_table_dealloc: (a: number) => void;
+  readonly __wbindgen_start: () => void;
 }

Note that I got this behavior for a lot of functions/method on another project, and it doesn't seem to be related to Result specifically.

I can not reproduce it on wasm-bindgen 0.2.93, with either Rust version

RunDevelopment commented 1 week ago

Now I understand what's going on here. Rust 1.82 enabled multivalue returns by default (related #4133). This allows wasm-bindgen to directly return multiple value instead of using a return pointer. WBG returns multiple values, because it needs a tag for the result (one value) and the error (another value) for Result<(), JsError>.

This is why the signature changed from (a: number) => void (where a should really be called returnPtr) to () => Array.

That said, I don't see why this is an issue. From my understanding, this is just wasm bindgen's internal ABI for those functions, and this ABI may change at any time and depending on configuration (e.g. compiler flags). The generated JS function of the same name should still be function throwing_function(): void as before.

davids-work commented 1 week ago

It is an issue because the type signature in the generated .d.ts bindings is invalid! Array needs a generic parameter.

I.e. Typescript applications that load these definition files won't compile.

(I'm not actually sure why these "internal" functions end up in the Typescript definitions in the first place, preferably I'd rather not have them there at all, but that's another topic)

RunDevelopment commented 1 week ago

It is an issue because the type signature in the generated .d.ts bindings is invalid! Array needs a generic parameter.

Oh, I didn't even notice that. I thought the issue was about the function return an array instead of void.

Well, replacing Array with any[] should be an easy fix.

(I'm not actually sure why these "internal" functions end up in the Typescript definitions in the first place, preferably I'd rather not have them there at all, but that's another topic)

I super agree. I don't know why there are exported at all.