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
feat: Support wasm32 #119
feat: Support wasm32 #119
Conversation
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.
@@ -4,7 +4,8 @@ pub use self::memory::MemorySessionStore; | |||
pub(crate) use super::Session; | |||
use async_trait::async_trait; | |||
|
|||
#[async_trait] | |||
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] |
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.
It might be useful to mention this change (the Send
bound on whatever this is effecting now being optional for wasm32-*
targets) in the atrium-api
changelog (for the public traits that are altered).
@@ -29,7 +29,8 @@ where | |||
} | |||
|
|||
/// An abstract HTTP client. | |||
#[async_trait] | |||
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))] |
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.
Ditto, it would be useful to mention this in the atrium-xrpc
changelog (for the public traits that are changed).
isahc = ["dep:isahc"] | ||
reqwest-native = ["reqwest/native-tls"] | ||
reqwest-rustls = ["reqwest/rustls-tls"] |
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.
It would be useful to mention the removal of these feature flags (and addition of the new ones) in the atrium-xrpc-client
changelog.
surf = ["dep:surf"] | ||
|
||
[dev-dependencies] | ||
[target.'cfg(target_arch = "wasm32")'.dependencies] | ||
reqwest.workspace = true |
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.
This makes reqwest
non-optional on wasm32
, which is a little odd but I guess makes sense given that atrium-xrpc-client
is completely empty when no features are enabled. If a non-reqwest
backend gains wasm32
support then changing this to be default-enabled instead at that time is fine.
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.
utACK b460003
Thanks for reviewing! |
resolve #116
async_trait
depending on whethertarget_arch = "wasm32"
or not.wasm-pack --test
atrium-xrpc-client
's features