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

utility crate: DOMContentLoaded future #10

Open
fitzgen opened this issue Feb 26, 2019 · 8 comments
Open

utility crate: DOMContentLoaded future #10

fitzgen opened this issue Feb 26, 2019 · 8 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Feb 26, 2019

Yosh and I started one at the 2019 Rust All Hands.

Give you a future for when the DOMContentLoaded event has fired.

@Pauan
Copy link
Contributor

Pauan commented Mar 13, 2019

This is another API which maps very well onto futures-signals: there can be an is_dom_ready() function which returns a Signal<Item = bool> which indicates whether the DOM is ready or not.

As a side note, when implementing this, you need to check document.readyState, because the code might be running after the DOMContentLoaded event has fired.

@yoshuawuyts
Copy link
Collaborator

I published https://github.com/yoshuawuyts/document-ready with the code we had from the all-hands. It still needs to be tested and probably polished a bit more, but happy to move it in here if people think it's useful!

@fitzgen
Copy link
Member Author

fitzgen commented Mar 15, 2019

Summary

Add layered APIs for document readiness.

Motivation

Code often doesn't want to start executing until the Web page's DOM has been fully constructed. It might want to grab a handle to a node that might not exist until the DOM is fully loaded. Additionally, one might dynamically add an <iframe> to a Web page and want to wait on the nested <iframe>'s DOM readiness.

Detailed Explanation

Similar to the timers crate, this proposes a layered API where the first layer is the callback-y interface with Rust callbacks instead of wasm_bindgen::Closures or js_sys::Functions, and the second layer is based on Futures on top of the callback-y layer.

  • pub fn is_ready(document: &web_sys::Document) -> bool { ... }

    Just checks whether document.readyState !== "loading", which is equivalent to "the DOMContentLoaded event has already fired".

  • pub fn on_ready<F>(document: &web_ses::Document, callback: F)
    where
        F: 'static + FnOnce
    { ... }

    Checks if the given document is ready, and if so invokes callback after a setTimeout(0). If not, adds callback as a listener to the documents "DOMContentLoaded" event.

  • pub struct DomReady { ... }
    impl Future for DomReady {
        type Item = ();
        type Error = ();
        fn poll(&mut self) -> Poll<Self::Item, Self::Error> { ... }
    }
    impl DomReady {
        pub fn top_level() -> Self { ... }
        pub fn with_document(document: &web_sys::Document) -> Self { ... }
    }

    A future that wraps the on_ready function with a futures::oneshot channel. Has a constructor for the default top-level document and a with_document constructor waiting on a particular document's readiness (for example iframe documents).

Drawbacks, Rationale, and Alternatives

We could potentially not take a document argument to simplify the API. However, besides allowing for working with <iframe>s' documents, taking a document argument allows us to more easily test this code and its different cases of ready already vs not ready yet.

We could potentially only expose the future type, and not the is_ready or on_ready functions to simplify the API. However, the implementation of the future type has to have these things under the hood anyways, and in the interest of making low-level, building blocks functionality reusable (maybe someone isn't using futures?) we should expose these functions. It isn't much of a maintenance burden.

We could avoid the setTimeout(0) behavior that ensures that the callback is always invoked on a new tick of the event loop. This gives us slightly better latencies when the document is already ready. However, the motivation for doing it is to cut down on the space of possible behaviors (by making it so that there is no special case of "already loaded" that calls the callback on the same tick of the event loop) and therefore make bugs less likely.

Unresolved Questions

  • Should we really do the setTimeout(0) thing?
  • Please bike shed the type and function names.

@Pauan
Copy link
Contributor

Pauan commented Mar 16, 2019

Just checks whether document.readyState === "complete".

Actually, that corresponds to the "load" event, not the "DOMContentLoaded" event.

So it needs to be document.readyState !== "loading"

Should we really do the setTimeout(0) thing?

I'm torn on that. There's good arguments for both.

There's also the possibility of using the microtask event loop rather than the macrotask event loop (though that introduces another subtle inconsistency...)

If somebody wants synchronous behavior, they can always manually check is_ready(), so we probably should use setTimeout(0) for consistency.

Please bike shed the type and function names.

I think pub fn on_ready(document: &web_sys::Document) -> impl Future<Output = ()> is actually the best API (with the DomReady type not exposed).

Also, looking at your types, they seem to use Futures 0.1. I think that's a mistake: Futures are nearly stabilized, and so I think we should use Futures 0.3 instead. That also gives us access to async/await, which is huge.

@fitzgen
Copy link
Member Author

fitzgen commented Mar 18, 2019

Also, looking at your types, they seem to use Futures 0.1. I think that's a mistake: Futures are nearly stabilized, and so I think we should use Futures 0.3 instead. That also gives us access to async/await, which is huge.

I think we should use 0.1 until 0.3 is published/stable. I'd like to avoid nightly completely. With wasm-bindgen-futures, we intend to switch from 0.1 to 0.3 as soon as it is published/stable, so I think it makes sense to do the same here.

Actually, that corresponds to the "load" event, not the "DOMContentLoaded" event.

So it needs to be document.readyState !== "loading"

Good catch!

@fitzgen
Copy link
Member Author

fitzgen commented Mar 18, 2019

I think pub fn on_ready(document: &web_sys::Document) -> impl Future<Output = ()> is actually the best API (with the DomReady type not exposed).

How would you expose the callback-based readiness API?

@Pauan
Copy link
Contributor

Pauan commented Mar 18, 2019

I think we should use 0.1 until 0.3 is published/stable. I'd like to avoid nightly completely. With wasm-bindgen-futures, we intend to switch from 0.1 to 0.3 as soon as it is published/stable, so I think it makes sense to do the same here.

That's fair, though it wouldn't surprise me if Future becomes stable before Gloo starts to take off, so I'm hesitant to build a lot of things on top of Futures 0.1, simply because of the extra work involved...

How would you expose the callback-based readiness API?

As I proposed in this issue, I would put it into a raw submodule. It would have this type:

pub fn on_ready<F>(document: &Document, f: F) -> Listener<'static, F> where F: FnOnce()

Listener is a struct which is used to implement RAII-based event deregistration. I think we should discuss that in a different issue.

@Pauan
Copy link
Contributor

Pauan commented Mar 18, 2019

RAII-event deregistration issue posted here: #30

ranile added a commit that referenced this issue Feb 15, 2022
* Update to Rust 2021 and use JsError from gloo-utils

* use new toolchain
ranile added a commit that referenced this issue Feb 16, 2022
* Initial commit

* provide a better interface for errors, rename `RequestMethod` to `Method`

* remove method for array buffer and blob in favor of as_raw

* prepare for release

* add CI, update readme

* hide JsError in the docs

* fix CI?

* Install wasm-pack in CI

* misc

* websocket API

Fixes: ranile/reqwasm#1

* add tests for websocket

* update documentation, prepare for release

* fix mistake in documentation

* Rewrite WebSockets code (#4)

* redo websockets

* docs + tests

* remove gloo-console

* fix CI

* Add getters for the underlying WebSocket fields

* better API

* better API part 2 electric boogaloo

* deserialize Blob to Vec<u8> (#9)

* Update to Rust 2021 and use JsError from gloo-utils (#10)

* Update to Rust 2021 and use JsError from gloo-utils

* use new toolchain

* Add response.binary method to obtain response as Vec<u8>

Fixes: ranile/reqwasm#7

* Remove `Clone` impl from WebSocket.

When the WebSocket is used with frameworks, passed down as props, it might be `drop`ed automatically, which closes the WebSocket connection. Initially `Clone` was added so sender and receiver can be in different `spawn_local`s but it turns out that `StreamExt::split` solves that problem very well.

See #13 for more information about the issue

* Rustfmt + ignore editor config files

* Fix onclose handling (#14)

* feat: feature gate json, websocket and http; enable them by default (#16)

* feat: feature gate json support

* feat: feature gate weboscket api

* ci: check websocket and json features seperately in CI, check no default features

* feat: feature gate the http API

* refactor: use futures-core and futures-sink instead of depending on whole of futures

* ci: test http feature seperately in CI

* fix: only compile error conversion funcs if either APIs are enabled

* fix: add futures to dev-deps for tests, fix doc test

* 0.3.0

* Fix outdated/missing docs

* 0.3.1

* Change edition from 2021 to 2018 (#18)

* Change edition from 2021 to 2018

* Fix missing import due to edition 2021 prelude

* hopefully this will fix the issue (#19)

* There's no message

* Replace `async-broadcast` with `futures-channel::mpsc` (#21)

We no longer need a multi-producer-multi-consumer channel. There's only one consumer as of ranile/reqwasm@445e9a5

* Release 0.4.0

* Fix message ordering not being preserved (#29)

The websocket specification guarantees that messages are received in the
same order they are sent. The implementation in this library can violate
this guarantee because messages are parsed in a spawn_local block before
being sent over the channel. When multiple messages are received before
the next executor tick the scheduling order of the futures is
unspecified.
We fix this by performing all operations synchronously. The only part
where async is needed is the conversion of Blob to ArrayBuffer which we
obsolete by setting the websocket to always receive binary data as
ArrayBuffer.

* 0.4.1

* move files for gloo merge

* remove licence files

* gloo-specific patches

* fix CI

* re-export net from gloo

Co-authored-by: Michael Hueschen <mhuesch@users.noreply.github.com>
Co-authored-by: Stepan Henek <stepan+github@henek.name>
Co-authored-by: Yusuf Bera Ertan <y.bera003.06@protonmail.com>
Co-authored-by: Luke Chu <37006668+lukechu10@users.noreply.github.com>
Co-authored-by: Valentin <vakevk+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants