-
Notifications
You must be signed in to change notification settings - Fork 718
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
Feature gate web-sys #904
Feature gate web-sys #904
Conversation
Probably makes sense to rename the feature from |
@Jake-Shadle Thanks for filing the issue. However, I don't understand the problem. You wrote:
The Cargo.toml already has:
So, doesn't the fact that this is a target-specific dependency already do what is proposed here? |
It doesn't build them unless you build for wasm, but it does impact dependency resolution and is included as a part of the lock file. |
OK, but why does that matter? I would prefer a messy Cargo.lock over messy code/configuration logic, if all other considerations are equal. In another project I do have an issue where we're trying to generate "dependency reports" for security auditing purposes and we have to be careful that we are doing that kind of stuff on a per-target (per-configuration, really) basis instead of just using a tool that reads Cargo.lock and assumes every dependency mentioned there is actually used. I think some tools need to improve in this regard. Is your issue similar to this? |
Yes, our situation is basically the same with cargo-deny, where features are the cleanest way to keep your dependency graph simple. However, the target configuration will be accessible more readily from |
Great. I commented in the cargo-deny issue: EmbarkStudios/cargo-deny#63. I'm going to close this on the assumption that the improvements to cargo and other tools will make this unnecessary to do. |
The
web-sys
crate brings in a ton of extra dependencies unconditionally, this just feature gatesweb-sys
behind thewasm
feature so that users can opt in to those extra dependencies, if they are even targeting wasm at all.