-
Notifications
You must be signed in to change notification settings - Fork 320
typespec_client_core: New web_runtime to support wasm32-unknown-unknown
#2838
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
Conversation
|
Thank you for your contribution @magodo! We will review the pull request and get back to you soon. |
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.
Pull Request Overview
This PR introduces WASM support for typespec_client_core by adding a new web_runtime module that provides a WASM-compatible async runtime implementation. The main goal is to fix the sleep functionality that was previously panicking on wasm32-unknown-unknown targets.
- Adds a new
WasmBindgenRuntimeimplementation usinggloo-timersfor sleep andwasm-bindgen-futuresfor task spawning - Updates the
RetryPolicytrait to conditionally remove theSendbound for WASM targets - Adds comprehensive test coverage for WASM functionality using
wasm-bindgen-test
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Cargo.toml |
Adds workspace dependencies for gloo-timers and wasm-bindgen-futures |
sdk/typespec/typespec_client_core/Cargo.toml |
Adds optional dependencies and WASM-specific features configuration |
sdk/typespec/typespec_client_core/src/async_runtime/mod.rs |
Updates runtime creation logic to prioritize WASM runtime when appropriate |
sdk/typespec/typespec_client_core/src/async_runtime/web_runtime.rs |
Implements new WASM-compatible async runtime using browser APIs |
sdk/typespec/typespec_client_core/src/async_runtime/tests.rs |
Adds WASM-specific tests and conditional compilation for existing tests |
sdk/typespec/typespec_client_core/src/async_runtime/standard_runtime.rs |
Updates sleep method signature to use TaskFuture type alias |
sdk/typespec/typespec_client_core/src/http/policies/retry/mod.rs |
Conditionally removes Send bound from async_trait for WASM compatibility |
eng/dict/crates.txt |
Adds gloo to the dictionary for spell checking |
sdk/typespec/typespec_client_core/src/async_runtime/web_runtime.rs
Outdated
Show resolved
Hide resolved
sdk/typespec/typespec_client_core/src/async_runtime/web_runtime.rs
Outdated
Show resolved
Hide resolved
…` when target wasm The `typespec_client_core::sleep::sleep` is using the async_runtime's sleep implementation, which is complicated to support `wasm32`, and will panic now when targeting to `wasm32`. Instead of make the `standard_runtime` to support wasm32 sleep (I wonder if it's feasible), we can instead just introduce another `sleep` implementation to simply use the `gloo_timers::future::sleep`. Also, this PR adds a `cfg_attr` for conditionally remove the `Send` trait from the `async_trait` for the `RetryPolicy`, as the other parts of the code do. Additionally, the `getrandom` crate will enable `wasm_js` features when targetting to `wasm`, which is necessary.
…e.rs Co-authored-by: Copilot <[email protected]>
…e.rs Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
If we only test wasm_bindgen, some json tests were failing. Also updates CHANGELOG.
heaths
left a comment
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.
Approved with my changes in there.
|
The macOS issue is the #2887 issue and not really a blocker. |
|
/check-enforcer override |
|
Thanks @magodo for all the work on this! |
The
typespec_client_core::sleep::sleepis using the async_runtime's sleep implementation, which is complicated to supportwasm32, and will panic now when targeting towasm32. Instead of make thestandard_runtimeto support wasm32 sleep (I wonder if it's feasible), we can instead just introduce anothersleepimplementation to simply use thegloo_timers::future::sleep.Also, this PR adds a
cfg_attrfor conditionally remove theSendtrait from theasync_traitfor theRetryPolicy, as the other parts of the code do.Fix #2754
Supersedes: #2770
Test
Some note about the wasm32 test:
futures::executor::block_on(handle), as it introduces a deadlock in browser (since theblock_onblocks the current thread)Instant::now()when usingnodeas runtime