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

Return types are very weak #110

Open
richvdh opened this issue Jun 26, 2023 · 6 comments
Open

Return types are very weak #110

richvdh opened this issue Jun 26, 2023 · 6 comments

Comments

@richvdh
Copy link
Member

richvdh commented Jun 26, 2023

Currently, matrix-sdk-crypto-js defines lots of functions which return types like Promise, rather than using typescript's support for generics to declare what that promise will contain.

For example: OlmMachine.getMissingSessions could be defined to return Promise<KeysClaimRequest | undefined>.

Failure to declare these types correctly (a) makes development against the library much harder, because application developers have to add their own type annotations; (b) increases the risk of a regression due to an unnoticed change in return value

@richvdh
Copy link
Member Author

richvdh commented Jun 26, 2023

(I think this is blocked on better support in wasm-bindgen ?)

@Hywan
Copy link
Member

Hywan commented Jun 28, 2023

Yeah, it should be patched on wasm-bindgen directly.

@richvdh
Copy link
Member Author

richvdh commented Jun 28, 2023

@Hywan is there a particular issue in the wasm-bindgen repo which tracks the features we need here?

@Hywan
Copy link
Member

Hywan commented Jun 29, 2023

I don't think so. The best I can find is rustwasm/wasm-bindgen#1197.

@poljar poljar transferred this issue from matrix-org/matrix-rust-sdk Mar 21, 2024
@poljar poljar changed the title Return types in Javascript bindings are very weak Return types are very weak Mar 21, 2024
@richvdh
Copy link
Member Author

richvdh commented Jun 3, 2024

FWIW with modern wasm-bindgen, we can improve on this considerably: wasm-bindgen now understands async functions and will automatically turn async fn foo() -> Bar into javascript-side functions that return Promise<Bar>, provided Bar has an Into<JsValue> impl (cf https://rustwasm.github.io/wasm-bindgen/reference/js-promises-and-rust-futures.html#return-values-of-async-fn).

We should go through and fix the existing methods that explicitly call future_to_promise, and stop adding new ones.

@Hywan
Copy link
Member

Hywan commented Jun 5, 2024

Oh nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants