feat: WASM client via web-sys transport #648
Conversation
| let (err_tx, err_rx) = oneshot::channel(); | ||
| let max_notifs_per_subscription = self.max_notifs_per_subscription; | ||
|
|
||
| wasm_bindgen_futures::spawn_local(async move { |
There was a problem hiding this comment.
https://crates.io/crates/wasm-futures-executor is a candidate but doesn't compile for stable rust yet....
There was a problem hiding this comment.
I guess we could call the WASM client "experimental" and require nightly until the wasm-futures-executor stabilizes?
There was a problem hiding this comment.
it seems be fast enough for now, let's open a issue and investigate the performance gain for it instead....
client/transport/src/web_sys.rs
Outdated
|
|
||
| /// Sender. | ||
| #[derive(Debug)] | ||
| pub struct Sender(mpsc::UnboundedSender<WebSocketMessage>); |
There was a problem hiding this comment.
The JsValue is !Send and I had to use another channel here instead of having WebSocket here directly.
There was a problem hiding this comment.
Ah maybe this answers my question about not impling Send on the wasm Transport traits!
| _ = timeout => Ok(Err(Error::RequestTimeout)) | ||
| timeout: std::time::Duration, | ||
| rx: futures_channel::oneshot::Receiver<Result<T, Error>>, | ||
| ) -> Result<Result<T, Error>, futures_channel::oneshot::Canceled> { |
There was a problem hiding this comment.
changed to future::select to make it work for wasm
I wonder whether it's better to have a feature flag like "js"; just because it's compiled to wasm doesn't mean it'll be executed in a browser with access to the web-sys JS APIs (and I think some other crates use "js" but I'd have to go looking to check!) :) |
Does this have pluggable transpart (as in replaceable by a user)? If yes it makes sense to have a feature. If not: What is the point in compiling it for wasm without having access to the web APIs as there is no way to communicate? I would propose using |
That's a good point it's quite complicated to configure the client: the transport needs We might want to provide a thin wrapper crate that does that mess instead of the users. But yeah |
| "tracing", | ||
| "futures-timer", | ||
| ] | ||
| async-wasm-client = [ |
There was a problem hiding this comment.
we could reduce the features if the async client was extracted to a separate crate...
then we could do some target specific assertions but won't work here.
| - uses: actions/checkout@master | ||
|
|
||
| - name: Install | ||
| run: curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh |
There was a problem hiding this comment.
I've done cargo install wasm-pack in the past; is there an advantage to doing it this way?
There was a problem hiding this comment.
Aah; one advantage of the script by the looks of it is that it'll download pre-compiled binaries if they exist; that's good to know!
This PR adds
web-systransport to the jsonrpsee client in the browser (via WebSocket), it actually uses "std" currently but I'm not sure whether this will be an issue or not for users.I added another feature flag
async-wasm-clienttocoreto use a client to that spawns a background task usingwasm_bindgen_futures::spawn_localinstead oftokiosimilar to howasync-clientworks. We can probably bikeshed on the naming there.Another option is to go by `target.'cfg(target_arch = wasm32 )' instead to reduce the number of complicated feature flags to understand in this repo...
Closing #606