wasm32 target without getrandom/js feature

[TheEdward162]

Hi, is it possible to remove the js feature flag passed to getrandom dependency on wasm32 targets by default?

oauth2-rs/Cargo.toml

Line 65 in e2e8a62

getrandom = { version = "0.2", features = ["js"] }

I'm trying to use oauth2 in a wasm32 environment that is not running under a javascript host and so I am getting import errors (for example unknown import: `__wbindgen_placeholder__::__wbindgen_describe` has not been defined) when running the module under wasmtime (since it depends on some bindgen imports that I'm not providing).

getrandom crate also recommends not setting the feature in library crates, to leave the choice for the final dependent.

[TheEdward162]

Hmm, or it could also be the chrono wasmbind feature. Possibly both.

[ramosbugs]

Hey, thanks for reporting this issue. How are random numbers supposed to work for non-JS WASM targets? Also HTTP clients and timestamps, for that matter.

Hmm, or it could also be the chrono wasmbind feature. Possibly both.

Yeah, I'd expect similar issues with both crates. See recent discussion in chronotope/chrono#1301 about disabling chrono APIs at compile time on targets where they currently panic at runtime.

It seems like getrandom would have a similar situation, and a compilation error is preferable to a runtime panic.

[TheEdward162]

My usecase is that I'm implementing the host functions for the operations I want to support (for example executing a HTTP request, oauth2 already supports integrating with custom HTTP clients), so I would expose a random and time implementations as well. This is just a personal project and the host is a full Rust host, basically I'm reimplementing parts of WASI.

getrandom has a custom feature which allows (root) crates to register their own implementation, so that's what I would use. If chrono had a similar option I suppose that would be a way forward too.

True, compile error is definitely preferable. Unfortunately now it is a runtime error for me because the import fails when trying to link the WASM module at runtime.

[ramosbugs]

Gotcha, thanks for that context.

We could address the getrandom issue by controlling the js feature using a feature flag within this crate, which I think should be enabled by default since most WASM users of this crate will likely be in a JS environment. However, it would provide an escape hatch for other uses.

Despite getrandom's recommendation not to add the js feature in library crates, I strongly believe that crates should be self-contained and not require their users to add separate dependencies with additional feature flags to ensure feature unification (which breaks as soon as the library updates to use a newer version of that crate, without any SemVer guarantees). In other words, whether this crate depends on a particular version of getrandom is not part of its public SemVer guarantees.

Exposing a feature flag would allow the behavior to respect SemVer guarantees. It sounds like the custom feature would still require you to depend on the same version of getrandom as this crate and deal with future SemVer issues, but at least the majority of WASM users could continue to depend on this crate without having to add the getrandom dependency separately.

The chrono issue is probably more significant since we currently depend on Utc::now directly in at least one place:

oauth2-rs/src/devicecode.rs

Line 93 in e2e8a62

time_fn: Arc::new(Utc::now),

Fixing that would require either changing the API to require a time_fn (without the sensible default of Utc::now), disabling the device token flow for non-JS WASM environments, or exposing a different API for non-JS WASM environments than for other targets. None of those options seem appealing in order to support a niche use case, I'm afraid. Guaranteeing support for these targets would also constrain future functionality in this crate, and would likely be impossible in the downstream openidconnect crate that depends a lot more on randomness and time.

I think your best bet may be to vendor the chrono and getrandom crates, make any changes you need to them to call your custom system interfaces, and then use [patch.crates-io] to substitute your patched versions everywhere they're used (see https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#testing-a-bugfix). That would allow you to continue using higher-level crates like this one as-is and only focus on making low-level system crates support your custom system APIs.

[TheEdward162]

Thanks for the discussion. I agree this is a niche usecase and does not warrant the maintenance burden.

I was trying to avoid vendoring the dependencies and thought I'd ask, but you are right that it seems like the most sensible solution. Other than that, since this is nothing serious I could also switch to WASI and stub out the host functions I don't use. The main reason why I haven't done that here is that I've done that on another project already 😅

I've also been trying to use WASM multivalue, which forced me into nightly and -Z build-std territory, so that's another hole I dug myself into.

Oh well. I'll close the issue, thanks for considering.