Add native support for WASM targets

[esimkowitz]

Summary

This PR adds support for getting the configured system time zone via the js-sys crate for WASM targets. It also updates the dependencies to include inherent support for WASM targets by including the wasm-bindgen feature from the time crate. It also adds necessary attributes and dependencies for running all the existing tests against WASM targets.

I have a working implementation of this change here.

Reason for the change

I am building a cross-platform application that is a collection of useful widgets for developers. One of the features I currently support is converting a Unix timestamp to different locales. Previously, I was using the chrono-tz crate to get the IANA database, but I am working to adopt this crate instead as it patches https://github.com/advisories/GHSA-wcg3-cvx6-7396.

I was able to migrate my DateTime converter widget to use your crate easily, it only required patching the time crate to add the wasm-bindgen feature for WASM targets. I also wanted to add the system time zone feature, though, since that was not supported at all in chrono-tz, but it was failing for WASM targets since they don't have access to the Unix or Windows TZ syscalls.

Implementation details

The specific approach I took is based on https://stackoverflow.com/questions/61654015/how-can-i-get-the-browsers-local-timezone-using-rust-js-sys. It involves using the js-sys crate to invoke the Intl.DateTimeFormat object, which in its default form returns the time zone locale as configured in the browser/system runtime. Helpfully, the timeZone field is already formatted per the IANA standards, meaning it can easily be parsed by get_by_name, the same as the other targets.

Documentation

I've updated the system module with more verbose documentation and validated it using cargo doc. I've also added a new error Unsupported, which will be returned if get_timezone is called for an unsupported target platform (not WASM, Unix, or Windows).

Testing

I added support for testing against WASM browser targets using wasm-bindgen-test. This does not impact testing or building against other targets as the attribute is restricted to targets in the WASM family. Running these tests requires either the wasm-pack tool or the wasm-bindgen-cli be installed, otherwise the WASM binary cannot be run.

I've also updated the CI pipeline to add a new job that builds the crate against the wasm32-unknown-unknown target and runs all tests against Chrome, Firefox, and Safari.

All tests are passing:

➜  time-tz git:(evan/js-sys) ✗ cargo test --target wasm32-unknown-unknown --all-features
   Compiling time-tz v1.0.3 (/Users/esimk/source/time-tz)
    Finished test [unoptimized + debuginfo] target(s) in 3.19s
     Running unittests src/lib.rs (target/wasm32-unknown-unknown/debug/deps/time_tz-6ba78c3ee7233970.wasm)
Set timeout to 20 seconds...
running 12 tests                                  

test time_tz::posix_tz::parser::tests::basic ... ok
test time_tz::tests::handles_backward_changeover ... ok
test time_tz::tests::handles_broken_time ... ok
test time_tz::tests::handles_after_changeover ... ok
test time_tz::tests::handles_forward_changeover ... ok
test time_tz::tests::dst ... ok
test time_tz::tests::london_to_berlin ... ok
test time_tz::tests::offsets_and_name ... ok
test time_tz::tests::find ... ok
test time_tz::tests::names ... ok
test time_tz::system::tests::get_timezone ... ok
test time_tz::binary_search::tests::test_binary_search ... ok

test result: ok. 12 passed; 0 failed; 0 ignored

Non-Goals

This change has only been validated against the wasm32-unknown-unknown target, as the wasm32-wasi target is still experimental and likely to change. wasm32-unknown-emscripten is generally viewed as legacy so I also have not validated against this target.

I have not added support for running tests against Node as my primary goal here was to support browser compatibility, since I compile natively for Windows/Unix targets. The wasm-bindgen-test crate does not provide good support for targeting tests at both Node and browsers as each test module needs to call the wasm_bindgen_test_configure macro to set up browser compatibility. I thought about exposing the choice to target testing at either browser or Node as a feature flag, but this would interfere with the ability to run test with the --all-features option.

[Yuri6037]

Hi,

Thanks for the PR, honestly the sys part looks good, however the testing does not:

• First it uses a non-standard warm-bindgen-test cfg attribute which to me shouldn't be necessary. If I ever add more tests I shouldn't have to add another attribute, there should be a way to work with standard rust attributes in my opinion.
• Second, it exposes a public module in lib for wasm targets which has no reason to exist publicly (wasm_bindgen_test_wrapper), it should be private, if it can't be, then it's another sign of a bad design from wasm-bindgen.

I'd rather prefer a pre-processor which automatically modifies the code when compiling to wasm instead of manually adding these non-standard attributes to the code. Perhaps such a tool could be made as from what I've seen you don't call "cargo test" to run these tests anyway?

That said, I'm ok merging the system.rs part, as it looks straight forward and might be a nice addition. Would this be enough for your use case?

[esimkowitz]

Yeah, the situation for testing WASM targets is a bit messy, the existing test frameworks I found all required their own attributes to be added to the test methods. I can certainly look into swapping the attributes in a preprocessor but if you're fine just checking in the system.rs change for now, I can update the branch and contribute the preprocessor as a separate change.

[Yuri6037]

Thanks for the latest changes, I'll merge this probably tomorrow as the computer I use right now does not have the publish key.

[Yuri6037]

This is now available in v2.0.0. I had to bump major due to the change of error enum (new unsupported variant).