Skip to content

Conversation

therealprof
Copy link
Contributor

The instant maintainer recommends switching to web-time instead. This also replaces #968 (from which I've shamelessly stolen the getrandom change) and fixes #891.

Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@schungx
Copy link
Collaborator

schungx commented Jul 7, 2025

This breaks code because you removed the wasm-bindgen and stdweb features.

Also, is it good to always include js for wasm? Some wasm builds may not have JavaScript available.

@therealprof
Copy link
Contributor Author

Would you prefer blind feature flags or removing them from CI?

Also, is it good to always include js for wasm? Some wasm builds may not have JavaScript available.

Honestly, I have no idea what the use case for the wasm32-unknown-unknown target even is nowadays... Free standing executors will most certainly use WASI and as far as I'm aware there's no way of using WASM in the browser without JS. The Cargo.toml could probably even be tighter to only include web-time for wasm32-unknown-unknown rather than all wasm32 targets...

Signed-off-by: Daniel Egger <daniel@eggers-club.de>
@schungx
Copy link
Collaborator

schungx commented Jul 8, 2025

Would you prefer blind feature flags or removing them from CI?

Well, removing them would be breaking, although we can keep an empty feature just for the sake of compatibility... I'm not hung up either way, just not break code.

Honestly, I have no idea what the use case for the wasm32-unknown-unknown target even is nowadays...

I remember some user before having his own execution environment that is not a browser and not wasi... That is why wasm32-unknown-unknown is full of unknown's... Otherwise it would probably be specified as browser or js...

@therealprof
Copy link
Contributor Author

Well, removing them would be breaking, although we can keep an empty feature just for the sake of compatibility... I'm not hung up either way, just not break code.

Right, with the latest commit everything should be working as before.

The supported list of targets in rustc is:

wasm32-unknown-emscripten
wasm32-unknown-unknown
wasm32-wali-linux-musl
wasm32-wasip1
wasm32-wasip1-threads
wasm32-wasip2
wasm32v1-none
wasm64-unknown-unknown

I do know wasm32-unknown-unknown has some uses to produce generic embeddable code, absolutely not sure what the use case for Rhai would be to have a generic interpreter outside of a browser or a WASI runtime. 🤷🏻

@schungx
Copy link
Collaborator

schungx commented Jul 8, 2025

I do know wasm32-unknown-unknown has some uses to produce generic embeddable code, absolutely not sure what the use case for Rhai would be to have a generic interpreter outside of a browser or a WASI runtime. 🤷🏻

Well, I made it work because someone was doing exactly that -- making a generic WASM. So as much as possible we shouldn't make it breaking, and specifying getrandom/js may break things. I'm not sure but it seems risky.

I suggest putting getrandom/js back with the wasm-bindgen and stdweb feature flags and not by default on all WASM targets.

Also, after you add back the flags, does it still compile with stdweb? We'd also need to check that... not telling whether someone will be using stdweb. I suspect that it won't work because web-time depends on wasm-bindgen to function:

Furthermore it depends on wasm-bindgen, which is required. This library will continue to depend on it until a viable alternative presents itself, in which case multiple ecosystems could be supported.

If we deprecate stdweb, then it is breaking. instant has stdweb bindings.

@therealprof
Copy link
Contributor Author

I already added the flags back. The current build failure is also on main and comes from unpinned ahash pulling in the incompatible getrandom 0.3.3:

# cargo tree --invert getrandom@0.3.3
getrandom v0.3.3
└── ahash v0.8.12
    └── rhai v1.22.2 (/Users/egger/OSS/rhai)

@therealprof
Copy link
Contributor Author

With ahash pinned to 0.8.4, this PR builds just fine:

# cargo build --target wasm32-unknown-unknown --features wasm-bindgen
   Compiling rhai v1.22.2 (/Users/egger/OSS/rhai)
   Compiling wasm-bindgen v0.2.100
   Compiling js-sys v0.3.77
   Compiling getrandom v0.2.16
   Compiling web-time v1.1.0
   Compiling ahash v0.8.4
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 11.20s

@schungx
Copy link
Collaborator

schungx commented Jul 9, 2025

With ahash pinned to 0.8.4, this PR builds just fine:

Yes, I know that. getrandom introduces a breaking change that is breaking a lot of crates.

My point is that web-time does not seem to support stdweb which is supported by Rhai. That would be breaking...

Maybe the solution will be to introduce yet another feature flag to switch to web-time...

@therealprof
Copy link
Contributor Author

"support" is a strong word for passing a feature flag to a dependency. Technically people could still use the stdweb crate which had it's last release 5 years ago but why anyone would do that is beyond me. wasm-bindgen has been the de-facto standard for doing WASM for quite some time.

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

Successfully merging this pull request may close these issues.

Rhai depends on unmaintained crate "instant"
2 participants