-
Notifications
You must be signed in to change notification settings - Fork 6
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
Wasm support for client using grpc-web #23
Conversation
TODO - allow user to give client, this way we can support both grpc-web and grpc and remove a dependency |
@manu0466 My implementation only uses grpc_web, ideally the user should be able to use grpc_web or grpc based on their situation. This can be accomplished by allowing them to inject a generic grpc client into the cosmos_client. Currently my rust knowledge is not strong enough to implement it correctly. |
Hi @shravanshetty1 thanks for your work 🙏. To do that i suggest you to create 2 private function with the same name like Here i provide you an actual example that should work
After adding this 2 fuctions you can simply call After this modifications you must make sure that the right dependencies are available at compile time. To do so you should tell cargo to use |
Made wasm compatibility into a feature,
default
|
@shravanshetty1 Well done!! Only one thing I think that the feature |
I initially tried to conditionally compile tonic "transport" feature based on target but couldnt find any work around. But after doing research again I was able to find this rust-lang/cargo#7914. This will force us to add build arguement while compiling same as feature. So i dont see any reason to implement this either since we are just changing the build argument. |
Seams that the problem was fixed with the new Feature Resolver 2.
|
Doesnt seem to work for me, here is some info:
rustc
wasm-pack
wasm-pack command used
|
Logs
|
I was having the same errors. You should add the resolver = "2" to the root Cargo.toml as suggested from the compiler:
|
@manu0466 Requested changes have been made |
@manu0466 after manual testing seems to work fine for both grpc and grpc-web |
@@ -18,4 +19,4 @@ debug = false | |||
debug-assertions = false | |||
|
|||
[patch.crates-io] | |||
cosmos-sdk-proto = { git = "https://github.com/forbole/cosmos-rust", branch = "main"} | |||
cosmos-sdk-proto = { git = "https://github.com/shravanshetty1/cosmos-rust", branch = "wasm2"} |
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.
Can we use the upstream version instead of the your fork?
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.
The upstream version is not wasm compatible
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.
Working on getting it merged -
cosmos/cosmos-rust#97
cosmos/cosmos-rust#90
cosmos/cosmos-rust#88
prost = { version = "0.7.0"} | ||
prost-types = { version = "0.7" } | ||
reqwest = { version = "0.11.0", features = ["blocking", "json"]} | ||
reqwest = { version = "=0.10.10", features = ["json"] } |
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.
Why exactly the 0.10.10 instead of the latest 0.11.4? Can we use the latest one?
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.
0.11.0 onwards was not wasm compatible, cannot remember the exact issue - I can find out if you need to know
Co-authored-by: Manuel <manuel.turetta94@gmail.com>
Co-authored-by: Manuel <manuel.turetta94@gmail.com>
No description provided.