-
Notifications
You must be signed in to change notification settings - Fork 197
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
ban mio dependency so we don't accidentally break WASM users #2919
Conversation
A new generated diff is ready to view.
A new doc preview is ready to view. |
Putting this on hold since I couldn't figure it out in a short time. |
@@ -34,7 +37,7 @@ http = "0.2.9" | |||
http-body = "0.4.5" | |||
md-5 = "0.10.1" | |||
ring = "0.16" | |||
tokio = { version = "1.23.1", features = ["full"] } | |||
tokio = { version = "1.23.1" } |
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 think you need default-features = false
here. I'm not sure that the default-features are wasm-compat, and I wouldn't count on it.
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 cargo.toml isn't actually used in anything
@@ -47,6 +50,7 @@ aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api" | |||
tempfile = "3.6.0" | |||
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } | |||
aws-smithy-async = { path = "../../../rust-runtime/aws-smithy-async", features = ["test-util"] } | |||
tokio = { version = "1.23.1", features = ["rt", "macros"] } |
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.
Same here, needs default-features = false
@@ -19,5 +19,5 @@ aws-sdk-s3 = { path = "../../build/aws-sdk/sdk/s3", default-features = false } | |||
aws-smithy-async = { path = "../../build/aws-sdk/sdk/aws-smithy-async" } | |||
aws-credential-types = { path = "../../build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] } | |||
futures = "0.3.25" | |||
tokio = { version = "1.23.1", features = ["full", "test-util"] } | |||
tokio = { version = "1.23.1", features = ["macros", "test-util"] } |
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.
needs default-features = false
@@ -47,7 +47,7 @@ aws-smithy-async = { path = "../aws-smithy-async", features = ["rt-tokio"] } | |||
hyper-tls = { version = "0.5.0" } | |||
serde = { version = "1", features = ["derive"] } | |||
serde_json = "1" | |||
tokio = { version = "1.23.1", features = ["full", "test-util"] } | |||
tokio = { version = "1.23.1", features = ["macros", "test-util"] } |
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.
needs default-features = false
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 only features that are wasm-compat for wasm32-unknown-unknown are:
sync
macros
io-util
rt
time
https://docs.rs/tokio/latest/tokio/#wasm-support
The default features are seemingly not wasm-compatible.
When we bring in
mio
, it breaks WASM users. We don't currently have a need formio
so I've added a ban so that we don't accidentally bring it in.This change also
async-trait
we're using so that we don't bring in two versions ofsyn
.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.