-
Notifications
You must be signed in to change notification settings - Fork 296
typespec_client_core: New web_runtime
to support wasm32-unknown-unknown
#2770
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
2ab872d
c92b92b
5b5607b
6fa709a
411faaf
6069a4f
55ce372
23692dc
019ef31
5ecd47f
349b3c3
32a292f
1ef0cc3
79bbd58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,7 +150,7 @@ impl AsyncRuntime for StdRuntime { | |
/// Uses a simple thread based implementation for sleep. A more efficient | ||
/// implementation is available by using the `tokio` crate feature. | ||
#[cfg_attr(target_arch = "wasm32", allow(unused_variables))] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems this line is no longer needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we want to do #2770 (comment), this is still needed to allow a user to select the |
||
fn sleep(&self, duration: Duration) -> Pin<Box<dyn Future<Output = ()> + Send + 'static>> { | ||
fn sleep(&self, duration: Duration) -> TaskFuture { | ||
#[cfg(target_arch = "wasm32")] | ||
{ | ||
panic!("sleep is not supported on wasm32") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
use super::{AsyncRuntime, SpawnedTask, TaskFuture}; | ||
use crate::time::Duration; | ||
use std::pin::Pin; | ||
|
||
/// An [`AsyncRuntime`] using `tokio` based APIs. | ||
pub(crate) struct WebRuntime; | ||
|
||
impl AsyncRuntime for WebRuntime { | ||
fn spawn(&self, f: TaskFuture) -> SpawnedTask { | ||
Box::pin(async { | ||
wasm_bindgen_futures::spawn_local(f); | ||
Ok(()) | ||
}) | ||
} | ||
|
||
fn sleep(&self, duration: Duration) -> TaskFuture { | ||
Box::pin(async move { | ||
if let Ok(d) = duration.try_into() { | ||
gloo_timers::future::sleep(d).await; | ||
heaths marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
// This means the duration is negative, don't sleep at all. | ||
return; | ||
} | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better, but IMHO this should probably be tied to a specific feature which is the
gloo
runtime, unless you can assert safely thatgloo
is the runtime of choice for all wasm developers (I'm not familiar enough with wasm development to have an opinion here).This is similar to how we've tied the
tokio
runtime to thetokio
feature - because the tokio runtime is not a globally accepted async runtime, we hide its implementation behind a feature.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LarryOsterman The
gloo-timers
is provided by https://github.com/rustwasm, which is built on top ofwasm-bindgen
andjs-sys
, providing a more user friendly API. So if we really want to introduce a feature, then I'd prpose to name it "wasm-bindgen(as the
spawnis based on
wasm-bindgen-futures, and the
sleepis based on
gloo-timers, which in turn based on
wasm-bindgen`).Whilst, I believe even we use the target for this
create_async_runtime()
, it only means we provide a default runtime for the users. They are still able to callset_async_runtime()
to choose the runtime of their choice. Otherwise, if we do:For users who target to
wasm32-unknown-unknown
, without specifying thewasm-bindgen
feature, then they'll get aStdRuntime
by default, which will only panic in any case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can totally get behind a
wasm-bindgen
feature name.Thank you for your patience here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LarryOsterman Sure, I've pushed a new commit to introduce that feature name, made the wasm dependencies optional, and only bring them by this new feature.