-
Notifications
You must be signed in to change notification settings - Fork 373
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
Move from instant -> web_time #2093
Conversation
cd1f0ed
to
3bbb94c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Let's also forbid instant
in scripts/clippy_wasm/clippy.toml
under disallowed-types
.
We could also add it to the [bans]
list in deny.toml
… but banning instant
from all our dependencies may be taking it a bit too far
Oh, and make sure you test it on the web! |
59c1f6d
to
96c035a
Compare
Turns out there does appear to be a regression... if you open a web-viewer in a hidden tab and allow enough time to pass (3 to 4 seconds?) then when changing to that tab we don't have any content for the spatial views: Seems like we end up in a state where maybe an unknown time is ends up selected or something goes weird with forwarding playing of time. There's a known issue listed in the repo, so I'm wondering if this is related.
So far only seems to repro when loading a hosted RRD in the web-viewer:
|
cafe140
to
96c035a
Compare
The above issue appears likely to have been caused by: #2103 With that merged, we should investigate this again |
The issue was fixed by #2106 so I think this PR just needs a rebase and a new test |
Works great in Firefox and Chromium on my Mac. |
What
instant
has at least one issue affecting us:SystemTime::elapsed()
is broken sebcrozet/instant#49It also hasn't been updated in 18 months.
web-time
: https://github.com/daxpedda/web-time appears to achieve the same goals, but is under active development and notably doesn't have the bug thatinstant
had.This replaces all usage of
instance
withweb_time
.Checklist
PR Build Summary: https://build.rerun.io/pr/2093