A high-level, convenient, and safe, statically typed event system about gloo HOT 24 OPEN

A potentially relevant fact: support for the once event listener option (Edge 16, Firefox 50, Chrome 55, Safari 10) is broader than WebAssembly (Edge 16, Firefox 52, Chrome 57, Safari 11).

from gloo.

@chris-morgan Do you mean using target.addEventListener(event_type, callback, { once: true })? I wouldn't have thought of that.

That's very interesting, I like it. web-sys already has support for it, so in that case we no longer need a JS snippet, and we don't need Rc either.

from gloo.

@yoshuawuyts Sure, and that's handled by #10. Are you suggesting that all uses of FnOnce event listeners should get specialized ad-hoc APIs like that, rather than a generic mechanism for all FnOnce events? I suppose that's a reasonable solution.

from gloo.

I suspect that for mid-level APIs it's acceptable if we fail to completely constrain all interfaces, because we get a chance to do that at the high-level APIs (if we need to expose them at all).

The issue is that this is the high-level API. The mid-level API is #30 (which isn't constrained at all).

However, there is one thing that convinced me that you're right: we can simply not create LoadEvent / DomContentLoadedEvent / etc. structs.

So that way there's no confusion: FnOnce uses ad-hoc APIs (like #10), and StaticEvent is only for FnMut. A very clean separation.

What makes this work so well is that FnOnce APIs will almost always have a Future implementation, so we don't need them to be accessible via the high-level event listener API.

from gloo.

@olanod yeah I think #33 covers this. FWIW, not everyone wants to use FRP, and we are trying to build things in a layered fashion so that if folks want a different high-level paradigm, they can reuse the lower-/mid-level foundation.

from gloo.

@dcormier Note that the original problem (described in that issue) is being fixed by the browsers, so it doesn't need to be fixed by gloo:

https://bugzilla.mozilla.org/show_bug.cgi?id=1541349

https://bugs.chromium.org/p/chromium/issues/detail?id=949056

from gloo.

I'm not really sure how to handle FnOnce. It's easy enough to just provide a once function, but... there are some events (like load, DOMContentLoaded, etc.) which only make sense with FnOnce. I'm not sure how to guarantee that statically.

from gloo.

there are some events (like load, DOMContentLoaded, etc.) which only make sense.

I'm not sure which load event you're referring to here (this?), but the DOMContentLoaded event could probably use a higher-level wrapper anyway where FnOnce can be guaranteed.

I wonder if at this layer it's perhaps okay to be quite flexible with what's possible, and only start locking things down a bit more at the higher layers?

from gloo.

Yes, I mean that load event. I'm sure there's many more FnOnce events (I just can't remember them off the top of my head).

I wonder if at this layer it's perhaps okay to be quite flexible with what's possible, and only start locking things down a bit more at the higher layers?

As it stands, this is currently the highest level for event listening. So which "higher layer" are you referring to? A Virtual DOM library?

from gloo.

@Pauan I was thinking of for example a FnOnce that is guaranteed to:

1. fire exactly once
2. always fire after the DOM has loaded

In order to do so you need to check document.readyState + listen for DOMContentLoaded.

Other examples could include the CustomElement API, and perhaps even some of the networking abstractions.

from gloo.

@OddCoincidence asked whether we need the <'a> lifetime, and after thinking it over, I don't think it benefits us at all, so I think we should instead have EVENT_TYPE always be &'static str

from gloo.

Are you suggesting that all uses of FnOnce event listeners should get specialized ad-hoc APIs like that

@Pauan that's putting it a bit more strongly than what I was going for, but I suspect that for mid-level APIs it's acceptable if we fail to completely constrain all interfaces, because we get a chance to do that at the high-level APIs (if we need to expose them at all).

If my experience with authoring frameworks is anything to go by, I suspect it might very well be that most end-users might not ever need to interact with some of these APIs directly.

from gloo.

I’m not certain that you can trust that load and DOMContentLoaded will be fired at most once; there have definitely been situations historically where they could be fired multiple times, with no clear consensus on whether it was buggy software or not (and whether that buggy software may still be in use). I would recommend invoking removeEventListener for safety after an invocation.

Here’s an alternative design that you haven’t considered:

pub trait EventType {
    type Event: JsCast;
    static EVENT_TYPE: &'static str;
}

pub fn on<E, F>(node: &EventTarget, _: E, callback: F) -> EventListener
where
    E: EventType,
    F: FnMut(E::Event) + 'static,
{
    EventListener::new(node, E::EVENT_TYPE, move |e| {
        callback(e.unchecked_into());
    })
}

pub struct Click;

impl EventType for Click {
    type Event = MouseEvent;
    const EVENT_TYPE: &'static str = "click";
}

on(&foo, Click, |e| {
    …
})

This eschews a wrapper type (which I think is good), at the cost of a new type to represent the event type string (which I think is pretty neutral).

I haven’t considered whether type inference on the closure will cope with |e| rather than needing |e: MouseEvent|. Maybe, maybe not—I know there are cases similar to this where the answer seems unambiguous but Rust can’t work it out.

from gloo.

there have definitely been situations historically where they could be fired multiple times, with no clear consensus on whether it was buggy software or not.

Do you have a recent example of this? Web browsers have been around for a while, and WASM is a pretty recent addition. It's the first time I'm hearing about this; having more details of when/how this happens would be of great help.

from gloo.

Recently, I have no examples of DOMContentLoaded or load being fired multiple times. https://stackoverflow.com/questions/4355344/domcontentloaded-event-firing-twice-for-a-single-page-load is one example of it happening, but that’s 2010. I believe I encountered evidence of it happening sometimes in maybe 2014. Wouldn’t surprise me if browser extensions or third-party site JS fired a second load or DOMContentLoaded event because they didn’t know better, either. All up, it’s a dangerous thing to trust, so at the very least you should check, if using FnOnce, to make sure that everything doesn’t explode if it gets fired twice. If it just throws a “tried to call a freed function” exception, that’s probably OK. If it calls the wrong function in some sort of use-after-free scenario, that’s disastrous and using removeEventListener will be necessary.

from gloo.

This eschews a wrapper type (which I think is good), at the cost of a new type to represent the event type string (which I think is pretty neutral).

I don't think removing the wrapper type is good.

The purpose of the wrapper type is to provide high-level methods, rather than the low-level raw web_sys methods.

I showed how we need a high-level value method in order to fix the unpaired surrogate issue, but it goes beyond that: we can provide idiomatic methods to get the current mouse position, provide an enum for the mouse button (rather than a number), or provide an enum for the pressed key (rather than a string).

There's a lot of weird stuff on the web which can benefit from some Rustification.

Having said that, I'm glad you posted an alternate design, since it helps contrast my proposal.

If it just throws a “tried to call a freed function” exception, that’s probably OK.

Yes, that's exactly what would happen. No undefined behavior, no memory unsafety.

from gloo.

Regarding events that should only happen once:

• Creating a wasm_bindgen::closure::Closure from a FnOnce automatically builds in a "poison flag" type thing that makes it so that if the function is called more than once, it throws an exception. But even so we should still remove our single-use event listeners from the DOM to avoid leaking memory.

• We probably also want a once helper function for both the dynamic, mid-level events API and the static, high-level API. It is pretty common functionality to need at some point. Something roughly equivalent to this JS:

  function once(target, eventType, f) {
      target.addEventListener(eventType, function temp(event) {
          target.removeEventListener(eventType, temp);
          f(event);
      });
  }

  There is a little bit more nuance to defining this sort of utility in Rust, however. We probably want a OnceEventListener type instead of trying to re-use EventListener directly. There is fundamental shared ownership here: we want to return a cancel-able handle to the code that registers the one-time event listener, but the internal callback that actually gets registered as an event listener needs to be able to remove the event listener after it has been invoked as well.

  Because of this extra machinery, and because it uses new types, I think we can safely add these utilities in an incremental, semver-compatible manner. I.e. we don't need to block all progress on gloo_events until we figure this out.

  I'm imagining that the specific single-event crates/APIs we expose, like DOMContentReady, could internally build on these once helpers.

from gloo.

We probably also want a once helper function for both the dynamic, mid-level events API and the static, high-level API.

Agreed for the mid-level EventListener (it already defines a once function, it just needs to be tweaked a bit).

Not sure I agree for StaticEvent: what use cases are there for it?

There is a little bit more nuance to defining this sort of utility in Rust, however. We probably want a OnceEventListener type instead of trying to re-use EventListener directly.

One possibility is to create a local .js file which handles that, so we don't need a new type in Rust, and we don't need to use Rc either:

export function once(target, event_type, callback) {
    function listener(event) {
        target.removeEventListener(event_type, listener);
        callback(event);
    }

    target.addEventListener(event_type, listener);
}

#[wasm_bindgen(module = "/events.js")]
extern {
    fn once(target: &EventTarget, event_type: &str, callback: &Function);
}

from gloo.

One possibility is to create a local .js file which handles that, so we don't need a new type in Rust, and we don't need to use Rc either:

As written, this doesn't allow cancellation, but we could probably fix that.

from gloo.

Not sure I agree for StaticEvent: what use cases are there for it?

I guess if we tend to make API-specific wrappers for most single-use event listeners (like DOMContentLoaded) then it matters less.

from gloo.

As written, this doesn't allow cancellation, but we could probably fix that.

Ah, yeah, I had thought it would, but you're right it won't. Good catch!

So it would have to be changed to something like this:

export function on(target, event_type, callback) {
    target.addEventListener(event_type, callback);

    return function () {
        target.removeEventListener(event_type, callback);
    };
}

export function once(target, event_type, callback) {
    var cancel = on(target, event_type, function (event) {
        cancel();
        callback(event);
    });

    return cancel;
}

...at that point, it may be easier to just use Rc in Rust.

from gloo.

This proposal looks nice and convenient, but if aiming for a high level event API what about going a step further and make the on function work with streams! developers love this reactive way of dealing with event data and standard JS might get there some day. my_element.on("click").filter(|e| {...}).for_each(|foo| {...}) is far sexier isn't it? And it would be aligned with what people want native JS to offer some day.
It's possible that the Observable proposal gets to see the light some day, eventually opening the doors to a standard that extends the HTMLElement API for elements to handle events using observables, but that path might take years 😱 we could have this nice things with rust now! 😄
Does this kind of abstraction make sense and fits in the goals of Gloo?
Edit: didn't see this one before #33 I guess it's pretty much what I suggest.

from gloo.

@olanod but if aiming for a high level event API what about going a step further and make the on function work with streams!

I actually do plan to make a proposal about an on function (or similar) that returns a Stream of events. It's a good idea, thanks for mentioning it.

But that can be added later, in a backwards compatible way, layered on top of this API.

from gloo.

rustwasm/wasm-bindgen#1348, mentioned in the original post for this issue, has now been closed. It would be great to see this proposal move forward.

from gloo.