Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WASM support #491

Merged
merged 1 commit into from
Jul 29, 2022
Merged

WASM support #491

merged 1 commit into from
Jul 29, 2022

Conversation

xy2i
Copy link
Contributor

@xy2i xy2i commented Jul 24, 2022

This PR allows using time with WebAssembly, resolving #490.

Concerns

Transitive dependecies

When on a wasm target, js-sys depends on wasm-bindgen, which pulls syn and quote among others:

time v0.3.11 (/home/xy/work/time)
├── js-sys v0.3.58
│   └── wasm-bindgen v0.2.81
│       ├── cfg-if v1.0.0
│       └── wasm-bindgen-macro v0.2.81 (proc-macro)
│           ├── quote v1.0.20
│           │   └── proc-macro2 v1.0.40
│           │       └── unicode-ident v1.0.2
│           └── wasm-bindgen-macro-support v0.2.81
│               ├── proc-macro2 v1.0.40 (*)
│               ├── quote v1.0.20 (*)
│               ├── syn v1.0.98
│               │   ├── proc-macro2 v1.0.40 (*)
│               │   ├── quote v1.0.20 (*)
│               │   └── unicode-ident v1.0.2
│               ├── wasm-bindgen-backend v0.2.81
│               │   ├── bumpalo v3.10.0
│               │   ├── lazy_static v1.4.0
│               │   ├── log v0.4.17
│               │   │   └── cfg-if v1.0.0
│               │   ├── proc-macro2 v1.0.40 (*)
│               │   ├── quote v1.0.20 (*)
│               │   ├── syn v1.0.98 (*)
│               │   └── wasm-bindgen-shared v0.2.81
│               └── wasm-bindgen-shared v0.2.81
├── libc v0.2.126
└── num_threads v0.1.6
[dev-dependencies]
...

Invalid JS dates

let dt = new Date(NaN);
passToRust(dt);

These dates are interpreted by js-sys as having timestamp 0, and will return a valid date (January 1, 1970).

@codecov
Copy link

codecov bot commented Jul 24, 2022

Codecov Report

Merging #491 (47027cc) into main (94d56ab) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #491   +/-   ##
=======================================
  Coverage   99.48%   99.48%           
=======================================
  Files          70       70           
  Lines        7219     7225    +6     
=======================================
+ Hits         7182     7188    +6     
  Misses         37       37           
Impacted Files Coverage Δ
src/sys/local_offset_at/mod.rs 100.00% <ø> (ø)
src/offset_date_time.rs 100.00% <100.00%> (ø)
src/parsing/parsable.rs 98.75% <0.00%> (-0.01%) ⬇️
src/quickcheck.rs 100.00% <0.00%> (ø)
src/parsing/iso8601.rs 100.00% <0.00%> (ø)
src/parsing/combinator/mod.rs 100.00% <0.00%> (ø)
src/parsing/combinator/rfc/rfc2822.rs 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

@xy2i xy2i force-pushed the wasmbind branch 2 times, most recently from c1a04b9 to 9750317 Compare July 24, 2022 20:54
@jhpratt jhpratt self-requested a review July 24, 2022 21:32
@tmpfs
Copy link
Contributor

tmpfs commented Jul 27, 2022

Thanks @xy2iii for putting this together!

I have updated to try this out as I have some existing wasm-bindgen test specs that call OffsetDateTime::now_utc() and I am getting this error:

---- wasm::wasm_tests::timestamp_encode output ----
    error output:
        panicked at 'invalid timestamp: Timestamp cannot fit in range: ComponentRange { name: "timestamp", minimum: -377705116800, maximum: 253402300799, value: 1658905765625, conditional_range: false }', git/forks/time/src/offset_date_time.rs:1345:14
        
        Stack:
        
        getImports/imports.wbg.__wbg_new_693216e109162396/<@http://localhost:8000/wasm-bindgen-test:669:21
        logError@http://localhost:8000/wasm-bindgen-test:153:18
        getImports/imports.wbg.__wbg_new_693216e109162396@http://localhost:8000/wasm-bindgen-test:668:66
        console_error_panic_hook::Error::new::h29497be23c947c4b@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3380]:0xe7448
        console_error_panic_hook::hook_impl::h34cf3f538be69b0f@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[492]:0x81c09
        console_error_panic_hook::hook::h599da1e54eb74ccd@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3746]:0xec492
        core::ops::function::Fn::call::h0b36bdea046ee49e@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3154]:0xe3c4a
        std::panicking::rust_panic_with_hook::h8888bbc201384d7b@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[979]:0xa61a2
        std::panicking::begin_panic_handler::{{closure}}::hea311ea2bd92a293@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1471]:0xbc1f1
        std::sys_common::backtrace::__rust_end_short_backtrace::h7df6c1a4b8949645@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[4489]:0xf3256
        rust_begin_unwind@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3554]:0xe9b63
        core::panicking::panic_fmt::h18b15be283411c65@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3598]:0xea544
        core::result::unwrap_failed::he84b58f6b44a7df4@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1658]:0xc23d1
        core::result::Result<T,E>::expect::h7563227c5d0f44e1@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[649]:0x8f9d7
        <time::offset_date_time::OffsetDateTime as core::convert::From<js_sys::Date>>::from::h78e0b6cea56de831@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[684]:0x926bc
        <T as core::convert::Into<U>>::into::h44202f12c0221598@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3570]:0xe9ed9
        time::offset_date_time::OffsetDateTime::now_utc::h960c44f6a4df3b12@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[4515]:0xf3445
        <sos_core::timestamp::time::Timestamp as core::default::Default>::default::h8ab4333f63098707@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[2019]:0xcc3b8
        wasm::wasm_tests::timestamp_encode::h52013ebf10bfc804@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[420]:0x79cbf
        core::ops::function::FnOnce::call_once::he0face7d8b62efeb@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[3916]:0xee677
        wasm_bindgen_test::__rt::Context::execute_sync::{{closure}}::h4c7fcafdaad35531@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1188]:0xb0b39
        <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hee390a86c5e17a9f@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1166]:0xafad0
        <wasm_bindgen_test::__rt::TestFuture<F> as core::future::future::Future>::poll::{{closure}}::{{closure}}::hda65de71c9118008@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1529]:0xbe22f
        wasm_bindgen::convert::closures::invoke0_mut::hfe01305b6e31d441@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1325]:0xb69a4
        __wbg_adapter_55@http://localhost:8000/wasm-bindgen-test:281:10
        cb0@http://localhost:8000/wasm-bindgen-test:472:28
        window.__wbg_test_invoke@http://localhost:8000/:25:38
        getImports/imports.wbg.__wbg_wbgtestinvoke_e5f1d09e8f92b525/<@http://localhost:8000/wasm-bindgen-test:477:30
        handleError@http://localhost:8000/wasm-bindgen-test:178:18
        getImports/imports.wbg.__wbg_wbgtestinvoke_e5f1d09e8f92b525@http://localhost:8000/wasm-bindgen-test:465:76
        wasm_bindgen_test::__rt::__wbg_test_invoke::h3a70a8c030d95b30@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[700]:0x939db
        <wasm_bindgen_test::__rt::TestFuture<F> as core::future::future::Future>::poll::{{closure}}::h092421bd644f70f5@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1340]:0xb730b
        scoped_tls::ScopedKey<T>::set::h5e124b6f69c5191d@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[870]:0x9f978
        <wasm_bindgen_test::__rt::TestFuture<F> as core::future::future::Future>::poll::haa1f7cc1564d8869@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[155]:0x4a449
        <wasm_bindgen_test::__rt::ExecuteTests as core::future::future::Future>::poll::h954d320e87a6d5cd@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[88]:0x2d431
        wasm_bindgen_test::__rt::Context::run::{{closure}}::hd013b90d575f5fdc@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[393]:0x769ca
        <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::ha866d401f870be81@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[951]:0xa4775
        wasm_bindgen_futures::future_to_promise::{{closure}}::{{closure}}::hfdd7d2d185931261@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[180]:0x511cb
        <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::he8dc13ea8b4f67e9@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1175]:0xb018f
        wasm_bindgen_futures::task::singlethread::Task::run::hc8da1ef974529b4f@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[354]:0x719e1
        wasm_bindgen_futures::queue::QueueState::run_all::hacbc2774ccac6687@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[277]:0x65a37
        wasm_bindgen_futures::queue::Queue::new::{{closure}}::h550121ad77617f5e@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[2099]:0xce3af
        <dyn core::ops::function::FnMut<(A,)>+Output = R as wasm_bindgen::closure::WasmClosure>::describe::invoke::h2c9c439e168be40f@http://localhost:8000/wasm-bindgen-test_bg.wasm:wasm-function[1061]:0xaa948
        __wbg_adapter_24@http://localhost:8000/wasm-bindgen-test:173:10
        real@http://localhost:8000/wasm-bindgen-test:136:20

Looking into this some more now.

@tmpfs
Copy link
Contributor

tmpfs commented Jul 27, 2022

I think the issue is that Date.getTime() returns milliseconds and from_unix_timestamp() expects seconds.

I updated to this:

let timestamp = (js_date.get_time() / 1000.0) as i64;

And it should work as expected now.

@xy2i xy2i force-pushed the wasmbind branch 3 times, most recently from 47f85d0 to 26cc74c Compare July 27, 2022 07:43
@xy2i
Copy link
Contributor Author

xy2i commented Jul 27, 2022

Hey @tmpfs, thank you for looking into it! Updated and co-authored you.

The from impl works with every js date with this bug fixed.

@tmpfs
Copy link
Contributor

tmpfs commented Jul 27, 2022

That is fantastic @xy2iii, thanks 🙏 A respectful nudge @jhpratt for a review please, would really like to see this land 🙌

@jhpratt
Copy link
Member

jhpratt commented Jul 27, 2022

I haven't forgotten about this. Just haven't had the time.

src/offset_date_time.rs Outdated Show resolved Hide resolved
src/offset_date_time.rs Outdated Show resolved Hide resolved
src/sys/local_offset_at/wasm_js.rs Show resolved Hide resolved
Co-authored-by: tmpfs <muji@tmpfs.org>
@jhpratt jhpratt merged commit ab17a66 into time-rs:main Jul 29, 2022
@tmpfs
Copy link
Contributor

tmpfs commented Jul 29, 2022

Massive thanks to both @xy2iii and @jhpratt for landing this so fast 🙌

My project's cargo audit now has a clean bill of health again as I was able to completely remove chrono from the dependency tree, much appreciated!

@jhpratt
Copy link
Member

jhpratt commented Jul 29, 2022

Not released yet, but it will be soon! There's at least one other thing I want to have land. If I can't get it reasonably quick, then I'll release as-is.

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.

None yet

3 participants