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

Cleaning up crate features #672

Open
KodrAus opened this issue May 6, 2023 · 4 comments
Open

Cleaning up crate features #672

KodrAus opened this issue May 6, 2023 · 4 comments

Comments

@KodrAus
Copy link
Member

KodrAus commented May 6, 2023

This is a meta issue to take a fresh look at the library’s dependency tree. In particular:

  • rng support: we have a fast-rng feature that adds rand as a dependency because getrandom is slow on Windows. Is that still necessary?
  • js: we pull in wasm-bindgen and getrandom unconditionally when building for the web. Could we use our crate features more cleverly to only pull in features we need?
  • macros: we have a macro-diagnostics feature that enables a proc-macro for better error reporting of compile-time UUID literals. Is this actually used? Can we emit more meaningful errors without needing a proc-macro on more recent compilers?

We can consider a major 2.x version of uuid that is semver-compatible with 1.x (no actual API changes and re-export 2.x in 1.x), but that removes or rejigs our crate features as necessary to keep our dependency tree sane.

@TylerBloom
Copy link

js: we pull in wasm-bindgen and getrandom unconditionally when building for the web. Could we use our crate features more cleverly to only pull in features we need?

From my understanding, you only need the js when you are compiling to WASM and need the rnd feature. (You might need it for atomics too, but the following still holds with a few additions). So, we can change the definition of the rnd feature to be: rnd = ["wasm-bindgen", "getrandom"]. Then, the dependencies imports can be gated by target:

rnd = ["wasm-bindgen", "getrandom"]

[dependencies]
getrandom = { version = "0.2" }

[target.'cfg(target_arch = "wasm32")'.dependencies]
getrandom = { version = "0.2", features = ["js"], optional = true }
wasm-bindgen = { version = "0.2", optional = true }

This also makes the js feature obsolete.

@KodrAus
Copy link
Member Author

KodrAus commented Jan 18, 2024

That's interesting 🤔 Would that rnd feature simply ignore the wasm-bindgen and getrandom features on targets that don't specify them as dependencies?

@TylerBloom
Copy link

That's interesting 🤔 Would that rnd feature simply ignore the wasm-bindgen and getrandom features on targets that don't specify them as dependencies?

Ya, that's correct. I don't know how this applies to user-selected features (i.e. features that you specify via cargo's --feature flag), but, for features that enable other features, cargo doesn't care. That or cargo is aware that wasm-bindgen is a valid feature sometimes, so if it gets specified and is not used that's ok.

@roderickvd
Copy link

For fast-rng with rand you would want to use SmallRng but we can do better: use fastrand instead. It's considerably lighter-weight than rand. I can put in a PR.

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

3 participants