Skip to content
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

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

Closed
wandomPewlin opened this issue Oct 17, 2024 · 6 comments · Fixed by #4210 or #4229
Closed
Labels

Comments

@wandomPewlin
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link

davids-work commented Oct 18, 2024

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
Copy link
Contributor

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
Copy link

davids-work commented Oct 18, 2024

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
Copy link
Contributor

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.

john-h-kastner-aws added a commit to cedar-policy/cedar that referenced this issue Oct 23, 2024
john-h-kastner-aws added a commit to cedar-policy/cedar that referenced this issue Oct 23, 2024
john-h-kastner-aws added a commit to cedar-policy/cedar that referenced this issue Oct 23, 2024
john-h-kastner-aws added a commit to cedar-policy/cedar that referenced this issue Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants