-
Notifications
You must be signed in to change notification settings - Fork 196
Make building with time support on wasm optional #968
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
base: main
Are you sure you want to change the base?
Conversation
I think there is a |
This still pulls in the crate dependency |
It pulls it in but does not use it. I am not sure there is a way in Do you have reservations on pulling the crate in? |
Our CI fails with a deprecation warning, but I guess we could also add |
I just hesitate to do major surgery here as may break some build combinations. |
This shouldn't really break anything because previously, compiling without either the
After:
The Error message in case the |
I vaguely remember that it is possible to do a raw wasm build without wasm-bindgen or std-web by setting some compiler flags. Therefore the no-W+T combo you tested should have worked. Check in one of the previous issues, I remember it there. Also targets such as wasm-wasi would need to build fine. Here is some docs on it: https://rhai.rs/book/start/builds/wasm.html#admonition-raw-wasm32-unknown-unknown It seems that it your solution should also work, but just make sure it checks out. |
OK, it seems that the following new checks is failing CI: #[cfg(target_family = "wasm")]
#[cfg(not(any(feature = "no_time", feature = "wasm-bindgen", feature = "stdweb")))]
compile_error!("WASM targets have to use either the `wasm-bindgen`, `stdweb` or `no_time` feature"); I think it can be removed because it should be possible to build for WASI? |
Ah, I see that makes sense. Should I just revert my latest commit then? |
@TrueDoctor OK, it seems the following CI has failed: cargo build --target wasm32-unknown-unknown --no-default-features Looks like Maybe we need to make |
Yeah, but shouldn't that have also failed before then? |
I don't think so... |
Always use get_random on
wasm
targets but make the inclusion of the instant crate optional. This partially fixes #891 by opting out of using the unmaintainedinstant
crate.