matrix-org / matrix-rust-sdk-crypto-wasm

Apache License 2.0
12 stars 7 forks source link

Return types are very weak #110

Open richvdh opened 1 year ago

richvdh commented 1 year ago

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 commented 1 year ago

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

Hywan commented 1 year ago

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

richvdh commented 1 year ago

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

Hywan commented 1 year ago

I don't think so. The best I can find is https://github.com/rustwasm/wasm-bindgen/issues/1197.

richvdh commented 4 months ago

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 commented 3 months ago

Oh nice!