Make building with time support on wasm optional
Always use get_random on wasm targets but make the inclusion of the instant crate optional. This partially fixes https://github.com/rhaiscript/rhai/issues/891 by opting out of using the unmaintained instant crate.
I think there is a no_time feature...
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 Cargo.toml to explicitly omit a dependency when a feature is set.
Do you have reservations on pulling the crate in?
Our CI fails with a deprecation warning, but I guess we could also add instant as an exception. That is just a slightly imperfect solution because we are then no longer warned about adding the dep elsewhere
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 wasm-bindgen or std-web feature always produced a build error. I've moved the get_random dependency to non-optional target dependency because from what I could tell, it is not actually wasm-bindgen or std-web specific.
T = Target wasm{32/64}-unknown-unknown, W = wasm-bindgen | std-web feature used
Before:
| W | T | Result |
|---|---|---|
| ❌ | ❌ | Compiles fine |
| ✅ | ❌ | Compile error invalid target |
| ❌ | ✅ | Compile error must choose W |
| ✅ | ✅ | Compiles fine |
After:
| W | T | no_time |
Result |
|---|---|---|---|
| ❌ | ❌ | _ |
Compiles fine |
| ✅ | ❌ | _ |
Compile error invalid target |
| ❌ | ✅ | ❌ | Compile error undeclared crate or module instant |
| ❌ | ✅ | ✅ | Compiles fine |
| ✅ | ✅ | _ |
Compiles fine |
The Error message in case the no_time feature is not used should probably be improved. This essentially splits one of the invalid configs into two, one valid and one invalid. This change should not invalidate any old configs but allow for a more fine-grained choice over which dependencies are pulled in
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 Instant is not included under this scenario, which is a raw WASM build.
Maybe we need to make instant non-optional after-all?
Yeah, but shouldn't that have also failed before then?
Yeah, but shouldn't that have also failed before then?
I don't think so...