http-rs / surf

Fast and friendly HTTP client framework for async Rust
https://docs.rs/surf
Apache License 2.0
1.45k stars 119 forks source link

Default to using wasm-client when targeting wasm #253

Open mmstick opened 3 years ago

mmstick commented 3 years ago

Say I make a crate which is a binding to a web API, and I choose to use Surf as the http client in this API.

Currently, you must manually define if you want to target wasm with a feature flag (wasm-client).

However, if I make changes to the feature flags in the crate, anyone who uses the crate as a dependency has to live with that decision. Cargo does not allow you to change feature flags defined in your dependencies dependencies.

This means that if you want your crate to be accessible in a desktop application with a native HTTP client, and simultaneously also usable in a WASM application, Surf isn't currently capable of simultaneously supporting both uses cases without publishing two different crates with different Surf variations.

Honestly, it'd be nice if we have a generic HTTP client trait that crates could use so that it wouldn't matter what HTTP client the caller is using with their API.

jbr commented 3 years ago

We agree, that would be ideal! However, currently cargo doesn't support all of the feature that would be needed to do that right, which would be to enable dependency features based on target architecture. We recommend adding a wasm flag to your library for the end binary crate to enable, in order to propagate that through the dependency tree

mmstick commented 3 years ago

Last I checked, Cargo doesn't support this. You can change the feature flags of a direct dependency, but you can't use feature flags to change feature flags of a dependencies dependencies. It only allows using feature flags to change the dependencies of a dependency.

mmstick commented 3 years ago

However, currently cargo doesn't support all of the feature that would be needed to do that right, which would be to enable dependency features based on target architecture.

This is actually supported. https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies

jbr commented 3 years ago

The issue we ran into when attempting to solve this previously was that we needed to enable features on dependencies that were out of our control, based on target

Fishrock123 commented 3 years ago

I think for now you can use a bin-level overload to deal with this?

mmstick commented 3 years ago

@Fishrock123 How so?

Fishrock123 commented 3 years ago

Oh, If you are making a crate, I suggest you expose your own feature flag for wasm, and proxy the feature setting to Surf.

It's not exactly ideal, but this is really a cargo problem.

mmstick commented 3 years ago

Yeah but Cargo doesn't support that

mmstick commented 3 years ago

My only option right now is to publish two versions of the same crate with different surf features

Fishrock123 commented 3 years ago

I mean, have your crate expose it's own wasm feature, which then configures Surf.

mmstick commented 3 years ago

Last I checked, Cargo doesn't support that.

Fishrock123 commented 3 years ago

Honestly, it'd be nice if we have a generic HTTP client trait that crates could use so that it wouldn't matter what HTTP client the caller is using with their API.

This already exists and is what Surf uses: https://github.com/http-rs/http-client

Fishrock123 commented 3 years ago

@mmstick This is literally what Surf does for http-client. I do not follow.

MarcAntoine-Arnaud commented 3 years ago

Hello !

I'm starting using surf, and I have the same issue. It's not ideal the create a feature for wasm target.

Everything can be made automatically, as in your code you can detect the target and generate code regarding that.

For example

[target.'cfg(target_arch = "wasm32")'.dependencies.web-sys]
version = "0.3.25"
optional = true
features = [
    "TextDecoder",
]

can be changed with:

[target.'cfg(target_arch = "wasm32")'.dependencies.web-sys]
version = "0.3.25"
features = [
    "TextDecoder",
]

and so here in client.rs the code can look like (not tested):

cfg_if! {
    if #[cfg(feature = "curl-client")] {
        use http_client::isahc::IsahcClient as DefaultClient;
    } else if #[cfg(target_arch = "wasm32")] {
        use http_client::wasm::WasmClient as DefaultClient;
    } else if #[cfg(feature = "h1-client")] {
        use http_client::h1::H1Client as DefaultClient;
    } else if #[cfg(feature = "hyper-client")] {
        use http_client::hyper::HyperClient as DefaultClient;
    }
}

So like that for a main application the inclusion can be made without feature selection ! So it's cross target by default

MarcAntoine-Arnaud commented 3 years ago

In fact the major difference supported by http-client is the support of the feature native_client. That feature select a provider for native or web environment.

Fishrock123 commented 3 years ago

We are trying to avoid pulling in so many dependencies cross-platform. The wasm dependencies are some of the heaviest.

MarcAntoine-Arnaud commented 3 years ago

I have created a PR to provide the native-client feature. https://github.com/http-rs/surf/pull/265

But I recommend you to take a look on gfx. They are able to define multi-backends, using multi-crates provide something more easy to use.

Fishrock123 commented 3 years ago

Sure but this is unlikely to re-become the default - the complaint was it wasn't the default and that a flag had to be used, was it not?

Again doing this by default causes cargo to pull in a lot of stuff because those target statements don't quite work correctly in Cargo.toml. I am sure there was a cargo issue for this but I can't seem to find the link offhand.