Skip to content

Conversation

@nickray
Copy link

@nickray nickray commented Mar 9, 2023

Some Wasm platforms (e.g. Cosmwasm) do not support bindgen.
On the other hand, a lot of libraries with Wasm support activate the "js" feature.

This PR allows the final crate to set getrandom to UNSUPPORTED, avoiding this issue.
Specifically, it can run RUSTFLAGS='--cfg js_unsupported' cargo build etc.

@newpavlov
Copy link
Member

It's a blatant misuse of the js feature by the libraries unless they have hard dependency on wasm-bindgen. It's clearly stated in our documentation:

This feature should only be enabled for binary, test, or benchmark crates. Library crates should generally not enable this feature, leaving such a decision to users of their library. Also, libraries should not introduce their own js features just to enable getrandom’s js feature.

So I suggest sending fix PRs against the misbehaving libraries. I don't think that introduction of such configuration flag is a good idea.

@josephlr
Copy link
Member

josephlr commented Mar 9, 2023

I understand that this is frustrating, but there is a reason that the js feature is off by default. It is to avoid the issues you are running into now.

Some crates incorrectly enable the js feature, thinking "hey this is how we support wasm", without reading our documentation about the crate features. The solution here is to get these crates to stop unconditionally enabling the js feature or exposing weird "feature forwarding" features (for example).

Some libraries intentionally enable the js feature, saying "we only want to support Web/Node.js when using wasm32-unknown-unknown". While this is not the recommended approach of this crate, it is a choice other libraries can make. However, this choice usually only makes sense if the library fundamentally needs cryptographic randomness.

Finally, if it turns out getting other libraries fixed is too hard, it is possible to have a (custom or supported) Cosmwasm-only target. Something like wasm32-cosmwasm-unknown. That would make it easier for this crate to distinguish when its being compiled for the Web/Node.js and when its being compiled for Cosmwasm.

However, none of these are changes to getrandom, so I'm going to close this. Feel free to open an issue, if there's another strategy you want to discuss to fix this issue.

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

Successfully merging this pull request may close these issues.

3 participants