piccolo
piccolo copied to clipboard
Replace `rand` crate with `fastrand`
These changes allow piccolo
to compile for wasm32-unknown-unknown
target.
There's no reason to swap to the fastrand
crate just to get wasm32 support, it works just as easily with the rand
crate and the getrandom
dependency. In fact, no changes to piccolo
are required at all, except that a downstream project might need to add a false dependency on getrandom
with the js
feature to make it work. I use piccolo
in a browser right now and that's what I do.
I'm not sure what is the most correct thing for piccolo
to do though, if it's more correct to enable the getrandom
js
feature here for the wasm32-unknown-unknown target or let the final application do it.
add a false dependency on getrandom with the js feature to make it work
Nice! Sorry, I misread rand
docs. I pushed updated pull request with getrandom
dependency for wasm32
targets.
Yeah, that's more like what I was thinking, but now the question is... is this the correct thing to do, or should we just let the end user do it?
Do we enable the "js" feature unconditionally when the target arch is wasm32? If so, that just kind of raises the question of why doesn't getrandom
do this already?
My guess is that the answer is because the target is wasm32-unknown-unknown, javascript isn't really strictly implied.
So, if we accept that, we can EITHER have our own "js" feature which seems pretty heavyweight, or maybe just... leave the status quo and let the end user manually inject the "js" feature into getrandom
? Or do something else where we ask the user to seed the global random number generator?
I dunno, that's kind of why things were left like they are now actually, because I didn't know the correct answer.
From getrandom
doc:
This crate fully supports the wasm32-wasi and wasm32-unknown-emscripten targets. However, the wasm32-unknown-unknown target (i.e. the target used by wasm-pack) is not automatically supported since, from the target name alone, we cannot deduce which JavaScript interface is in use (or if JavaScript is available at all).
About js
feature:
This feature should only be enabled for binary, test, or benchmark crates. Library crates should generally not enable this feature, leaving such a decision to users of their library. Also, libraries should not introduce their own js features just to enable getrandom’s js feature. This feature has no effect on targets other than wasm32-unknown-unknown.
Just my 2¢:
If the only wasm32-unknown-unknown
platform Piccolo supports is Web, then I would argue enabling getrandom
s js
feature when on this target makes sense.
But if Piccolo intends to support other platforms, e.g. running on Wasmtime (requires an API to supply a random function by the user), it should not be enabled and left to the user instead. Though I don't see anything particularly wrong with re-exposing a js
crate feature in Piccolo either.
I definitely would want to run piccolo on wasmtime. I have a wasm based plugin system, so running Lua scripts via piccolo would be great.
(Wasmtime does support wasm32-wasi
, so my comment only pertains to running wasm32-unknown-unknown
on Wasmtime, which might be a requirements for some use-cases)
I'm planning on addressing this in the near future when I add explicit support for no_std. There should definitely be a version that just accepts a random seed from the user.
I think at that point just having a "getrandom" feature would be enough, and that can provide a math lib load function that doesn't require a random seed.
We could also provide maybe both "getrandom" and "getrandom-js" features so the user doesn't have to manually inject the "js" feature, but this is quite a lot of ceremony for what amounts to a single function call.