-
Notifications
You must be signed in to change notification settings - Fork 248
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
add wasm support #700
add wasm support #700
Conversation
subxt/src/client/online_client.rs
Outdated
#[cfg(any( | ||
all(feature = "jsonrpsee-ws", not(target_arch = "wasm32")), | ||
all(feature = "jsonrpsee-web", target_arch = "wasm32") | ||
))] |
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.
Should we have a check at the root lib.rs to make sure that somebody cannot enable both of these jsonrpsee features simultaneously?
What are the pros and cons for also having this target_arch
check as well as just checking the features? It'd be nice if we could just check for a feature and not worry about the rest, for simplicity
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.
Should we have a check at the root lib.rs to make sure that somebody cannot enable both of these jsonrpsee features simultaneously?
Sure
What are the pros and cons for also having this target_arch check as well as just checking the features? It'd be nice if we could just check for a feature and not worry about the rest, for simplicity
It's my own fault in jsonrpsee i.e, it won't compile without the target_arch
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.
Ok that makes sense! Is it worth filing an issue/PR in jsonrpsee to tweak that so we can simplify 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.
yeah, I can take a look if it's possible to remove those
To avoid leaking `jsonrpsee-web` feature into the workspace
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.
Looks great! Would be ncie if we could slightly simplify the feature flags but it's not a blocker to getting this merged!
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.
Awesome! Can't wait to build a toy web app with this 😎
Co-authored by @Xanewok