opened
jjpe
username_0
Building on PR #286, this PR adds initial support to chrono for the wasm32 architecture.
Specifically, it adds wasm32-compatible versions of the `Local::now()` and `Utc::now()` constructors.
 Add initial support for the `wasm32` arch
issue
created
oleid
username_1
Is there a way to return a `datetime<Utc>` to JS? I'm running into the following error:

    121 | #[wasm_bindgen]
        | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::IntoWasmAbi` is not implemented for `std::boxed::Box<[(chrono::DateTime<chrono::Utc>, usize)]>`

when trying to return a `Vec<(datetime<Utc>, usize)>`.
comment
created
jjpe
username_0
That functionality is not covered by this PR.
comment
created
CryZe
username_2
Does this PR still support both stable rust and the non-clock / non-wasm-bindgen usecase? Cause that would very much mean that I can't use chrono anymore then.
comment
created
jjpe
username_0
With this PR I can use chrono as I would have before, except compiling for WASM does not fail anymore, and the `Local::now()` and `Utc::now()` constructors actually work.
comment
created
CryZe
username_2
It does not. Can I use this with wasm32-unknown-unknown on stable Rust? And does this force using wasm-bindgen on me, which is not something I want, as I'm using it in the true "unknown" / "no-std" way. Not in the implicit "wasm32-unknown-web" way.
comment
created
jjpe
username_0
I'm not sure what you mean by this.
comment
created
CryZe
username_2
The wasm32-unknown-unknown target should not imply compiling to the web. wasm has more targets than that. Unfortunately at the moment Rust doesn't have a -web target, and lots of documentation seems to indicate that wasm-bindgen is how you use the -unknown target, but that's just one way of using it. This is similar to including std in a no-std target. The -unknown target explicitly does not target the web, so just like putting std in no-std breaks everyone's use case that isn't targeting std or in this case the web.

However chrono already has a solution for this, the `clock` feature. With the feature not enabled, chrono only compiles in the no-std like code it can support, which means not exposing anything that would query the underlying environment. So to still properly support the wasm32-unknown-unknown target, your PR should only depend on wasm-bindgen when the `clock` feature is enabled, just like with any of the other targets.
comment
created
CryZe
username_2
So wasm32-unknown-unknown or wasm32-unknown-[whatever] shouldn't matter much.

Then that also breaks wasm32-unknown-emscripten, which does not support wasm-bindgen afaik.
comment
created
jjpe
username_0
This is not currently implemented (at least consciously), but may well be doable with little effort.
comment
created
CryZe
username_2
That's not what wasm32-unknown-unknown not targeting the web implies. What it implies is that there's no environment you can reliably import, because for example the actual binary part of the project decides which imports and exports exist and how they are named. This is a more than valid way to use the wasm32-unknown-unknown target that does not necessarily imply a "non-web wasm interpreter". This runs just fine in a browser or node.js. It just doesn't use wasm-bindgen which is just one of many flavors of generate the bindings. Another being stdweb, which probably gets broken by this PR too. Considering wasm-bindgen is just one of many ways of going about defining the imports / exports at this point in time, it should absolutely be gated by a feature flag that shouldn't be forced upon users. Chrono works just fine with stdweb and binary defined imports / exports right now, and you are breaking both of these use cases. It certainly makes sense to have better support for wasm-bindgen, but breaking every other use case is not the way to go at this point in time.
comment
created
jjpe
username_0
Without any environment to import, there's no date/time source, especially on `non-std` envs.
I don't see how chrono has any chance of working without such a source. So no, it would *not* run on the web.

As for the millions of other possible configs, that's way outside the scope of this PR, which is essentially only concerned with making `chrono`, stable rust, and wasm play nicely together.
comment
created
CryZe
username_2
I don't see how chrono has any chance of working without such a source. So no, it would not run on the web.

That's what the clock feature is for. Chrono explicits wants to support use cases where there is no specific environment to get the time from. That doesn't mean you can't use chrono. You can still create time stamps from times sourced through other means in such a no-std like environment. Which in the wasm32-unknown-unknown use case would for example be through an explicit "get_time" like import created by the binary. The `clock` feature is an explicit goal of the chrono library. You can't just randomly break it with this PR.
comment
created
jjpe
username_0
There's a difference between something being supported, and something working essentially by accident i.e. without the maintainer(s) intending to do so. For example, the former means that it should keep working lest it become a broken feature, but the latter does not. Compare the 2nd case to an internal API: you can use it and it'll work, but the risk of breakage is yours to take.
If a feature is supported, typically the devs will have announced it somewhere.


As for the `clock` feature gate, there I tend to agree with you.
But I cannot test that as I don't have any no-`std` envs available, so I can't properly write the code either. Care to commit the changes?
comment
created
CryZe
username_2
Okay, so I tested your branch against a few use cases and it seems wasm32-unknown-emscripten still compiles, but I didn't verify yet if wasm-bindgen has any negative effects that would break emscripten's bindings. It shouldn't be hard to make that target work again though, no matter whether intentionally supported by chrono or not.

wasm32-unknown-unknown still compiles on stable without any trickery just fine too, except it adds a whole lot of symbols including one import that shouldn't be there, so I guess if you aren't willing to feature flag wasm-bindgen (which could even be on by default), then I'll probably contribute that.
comment
created
Pauan
username_3
@username_0 The `wasm32-unknown-unknown` target is used for many things other than wasm-bindgen: e.g. there is also [stdweb](https://crates.io/crates/stdweb), which is quite popular (and has many features that wasm-bindgen doesn't have).

So as @username_2 said, this needs to be behind a feature flag, to allow for alternate uses of the `wasm32-unknown-unknown` target.
comment
created
oleid
username_1
FWIW: It would seem, that stdweb is going to be based uppon wasm-bindgen (or friends) eventually : https://github.com/rustwasm/team/issues/226
comment
created
jjpe
username_0
@username_2 I encourage you to do so, as I'm currently unable to spend time coding and testing this.
comment
created
quodlibetor
username_4
@username_2 @username_0 what are you currently using to verify that chrono works for you? Despite the fact that I'm very eager to merge this, I'm definitely not going to without any tests. I would like to verify that every `wasm32-unknown-*` (which is just `-unknown` and `-emscripten` right now, AFAIK?) at least compiles without error, and I'd rather have some actual tests. If it'll make it easier to collaborate I'll create a branch that y'all can PR against.

If neither of you have time to actually write the tests for the entire wasm community then, if you point me at some things that you're using, that will help me get started.

Thanks for the work and for everyone caring that everything still works!
comment
created
jjpe
username_0
The PR is in fact a 2-parter:

1. Absorb the time crate. This is meant purely as a refactoring i.e. what worked before should work now and vice versa.

2. The initial wasm32 code. These are just 2 constructors that are conditionally compiled if wasm32 is detected as arch.


As for what I've used so far to test, that would be a 20 KSLOC proprietary library that must be able to target both native (with std) and wasm32.

While I have no issue adding test in and of itself, I'm not sure what can be done in the ways of adding an actual test for this: the new constructors would never execute on native hosts, and in fact I'm not sure it would even compile there. Got any ideas to solve that?
comment
created
quodlibetor
username_4
There are two categories of test I would like to see:

* compile-pass tests-- just compiling chrono with `--target=wasm32-unknown-unknown` and `--target=wasm32-unknown-emscripten`
* a javascript integration test/script:  that imports chrono or some chrono wrapper that ensures that we can actually create a datetime, and with which we just run `node ci/wasmtest.js` , and which works with with each target.

Does that seem like it would catch major breakage? 

I would also prefer to not cause the `no-clock` use-case to suddenly generate much larger compiled objects, and I don't want to break `wasm32-unknown-unknown` if somebody else implements it.
comment
created
David-OConnor
username_5
What's the status of this? It would be nice to have Chrono work on wasm. Alternatively, are there any DT crates that work on wasm?
comment
created
jasonwilliams
username_6
Does anyone know what's going on with this?
@username_0 are you able to rebase?
comment
created
quodlibetor
username_4
This feature has been released via 331. Thanks for everyone's work!
comment