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

Disable wasmbind feature by default #1164

Closed
devongovett opened this issue Jul 2, 2023 · 2 comments
Closed

Disable wasmbind feature by default #1164

devongovett opened this issue Jul 2, 2023 · 2 comments
Labels
API-incompatible Tracking changes that need incompatible API revisions

Comments

@devongovett
Copy link

When compiling a dependency that transitively includes chrono for a wasm32 target, chrono implicitly pulls in wasm_bindgen. However, this is not desirable if you are not using wasm bindgen, but compiling for a plain wasm target. wasm_bindgen imposes requirements on the output wasm file, e.g. imports it needs from the JS environment, which might not always be available. Assuming that all wasm32-unknown-unknown targets are using wasm_bindgen is not a safe assumption.

If chrono was a direct dependency, I could turn off the default features, but since it is a transitive dependency this is not possible. Therefore I think it should be turned off by default. Turning on features in transitive dependencies is possible via feature unification. You'd make chrono a direct dependency and add the wasmbind feature, and it will also be enabled for other dependencies. However the opposite is not possible.

@pitdicker
Copy link
Collaborator

The problem is that this would be a breaking change for users that rely on the wasmbind feature. Maybe something to consider for 0.5?

@pitdicker pitdicker added the API-incompatible Tracking changes that need incompatible API revisions label Jul 5, 2023
erickt added a commit to erickt/chrono that referenced this issue Feb 29, 2024
This removes `wasmbind` from the default feature set, which stops chrono
from implicitly depending upon wasm-bindgen and js-sys. This is helpful
for a few reasons:

* It reduces the default dependency set by default for non-wasm
  projects, which shrinks the download size.

* Projects like Fuchsia have a policy where 3rd party crates need to be
  audited. While we don't use wasm-bindgen, we can't opt out of it by
  setting `default-features = false` because of [feature unification]
  ends up enabling chrono's default feature. See this [cargo issue]
  for more details. `wasm-bindgen` is large and complicated, so it's
  pretty expensive for us to update.

Fixes chronotope#1164

[feature unification]: https://doc.rust-lang.org/cargo/reference/features.html#feature-unification
[cargo issue]: rust-lang/cargo#4463
djc pushed a commit that referenced this issue Feb 29, 2024
This removes `wasmbind` from the default feature set, which stops chrono
from implicitly depending upon wasm-bindgen and js-sys. This is helpful
for a few reasons:

* It reduces the default dependency set by default for non-wasm
  projects, which shrinks the download size.

* Projects like Fuchsia have a policy where 3rd party crates need to be
  audited. While we don't use wasm-bindgen, we can't opt out of it by
  setting `default-features = false` because of [feature unification]
  ends up enabling chrono's default feature. See this [cargo issue]
  for more details. `wasm-bindgen` is large and complicated, so it's
  pretty expensive for us to update.

Fixes #1164

[feature unification]: https://doc.rust-lang.org/cargo/reference/features.html#feature-unification
[cargo issue]: rust-lang/cargo#4463
@pitdicker
Copy link
Collaborator

#1472 fixes this for 0.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API-incompatible Tracking changes that need incompatible API revisions
Projects
None yet
Development

No branches or pull requests

2 participants