Skip to content
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

Default to using wasm-client when targeting wasm #253

Open
mmstick opened this issue Oct 15, 2020 · 18 comments
Open

Default to using wasm-client when targeting wasm #253

mmstick opened this issue Oct 15, 2020 · 18 comments

Comments

@mmstick
Copy link

mmstick commented Oct 15, 2020

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
Copy link
Member

jbr commented Oct 15, 2020

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
Copy link
Author

mmstick commented Oct 15, 2020

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
Copy link
Author

mmstick commented Oct 15, 2020

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
Copy link
Member

jbr commented Oct 15, 2020

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
Copy link
Member

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

@mmstick
Copy link
Author

mmstick commented Oct 15, 2020

@Fishrock123 How so?

@Fishrock123
Copy link
Member

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
Copy link
Author

mmstick commented Oct 15, 2020

Yeah but Cargo doesn't support that

@mmstick
Copy link
Author

mmstick commented Oct 15, 2020

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

@Fishrock123
Copy link
Member

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

@mmstick
Copy link
Author

mmstick commented Oct 15, 2020

Last I checked, Cargo doesn't support that.

@Fishrock123
Copy link
Member

Fishrock123 commented Oct 15, 2020

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
Copy link
Member

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

@MarcAntoine-Arnaud
Copy link

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
Copy link

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
Copy link
Member

Fishrock123 commented Nov 16, 2020

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

@MarcAntoine-Arnaud
Copy link

I have created a PR to provide the native-client feature.
#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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants