A high-level, convenient, and safe, statically typed event system

[Pauan]

Summary

An API that provides guaranteed static typing for events, which is both more convenient and much safer.

Motivation

#30 lays the groundwork for a mid-level event listener system, but it only supports web_sys::Event, and it requires passing in string event types, which are error-prone: typos can happen, and there's no guarantee that the string matches the desired type:

EventListener::new(&foo, "click", move |e| {
    // Oops, "click" isn't a KeyboardEvent! Panic!
    let e: KeyboardEvent = e.dyn_into().unwrap_throw();

})

This proposal completely solves that, by guaranteeing that the static types are always correct.

It also does automatic type casts, which is more convenient for the user (compared to the manual type casts of EventListener).

Detailed Explanation

First, we must create a trait which supports conversion from web_sys::Event:

pub trait StaticEvent<'a> {
    const EVENT_TYPE: &'a str;

    fn unchecked_from_event(event: web_sys::Event) -> Self;
}

Now we create various structs, one struct per event type, and impl StaticEvent for them:

// https://www.w3.org/TR/uievents/#event-type-click
pub struct ClickEvent {
    event: web_sys::MouseEvent,
}

impl ClickEvent {
    // Various methods go here...
}

impl StaticEvent<'static> for ClickEvent {
    const EVENT_TYPE: &'static str = "click";

    #[inline]
    fn unchecked_from_event(event: web_sys::Event) -> Self {
        Self {
            event: event.unchecked_into(),
        }
    }
}

(And similarly for MouseUpEvent, MouseDownEvent, KeyDownEvent, etc.)

If possible, the above should be automatically generated based upon WebIDL.

If that's not possible, we should create a macro that makes it easy to generate impls for StaticEvent:

#[derive(StaticEvent("click"))]
pub struct ClickEvent {
    event: web_sys::MouseEvent,
}

Lastly, there should be an on function which allows for actually using StaticEvent:

pub fn on<'a, A, F>(node: &EventTarget, callback: F) -> EventListener<'a>
    where A: StaticEvent<'a>,
          F: FnMut(A) + 'static {
    EventListener::new(node, A::EVENT_TYPE, move |e| {
        callback(A::unchecked_from_event(e));
    })
}

And now it's possible for users to use the API:

on(&foo, move |e: ClickEvent| {
    // ...
})

on(&foo, move |e: MouseUpEvent| {
    // ...
})

Drawbacks, Rationale, and Alternatives

The primary drawback is the extra complexity, and the extra work of needing to create an extra struct per event type.

However, I don't see any better way to achieve the goals (full static typing, and automatic casts to the correct type).

Unresolved Questions

Perhaps the structs should accept a reference instead? In other words, it would look like this:

pub struct ClickEvent<'a> {
    event: &'a web_sys::MouseEvent,
}

impl<'a> ClickEvent<'a> {
    // Various methods go here...
}

impl<'a> StaticEvent<'a, 'static> for ClickEvent<'a> {
    const EVENT_TYPE: &'static str = "click";

    #[inline]
    fn unchecked_from_event(event: &'a web_sys::Event) -> Self {
        Self {
            event: event.unchecked_ref(),
        }
    }
}

Does that buy us anything?

---

There is a very important bug which we need to solve: rustwasm/wasm-bindgen#1348

I have given a lot of thought to this, and my (tentative) conclusion is that we can solve this bug by modifying a select few APIs.

In particular, if we change the input, keydown, keypress, and keyup events to do unpaired surrogate checking, that might be good enough to fix everything!

So, in that case, StaticEvent allows us to do that sort of checking:

#[derive(StaticEvent("input"))]
pub struct InputEvent {
    event: web_sys::InputEvent,
}

impl InputEvent {
    // Returns None if there are unpaired surrogates
    fn value(&self) -> Option<String> {
        // Unpaired surrogate checks...
    }
}

And now users can do this:

on(&foo, move |e: InputEvent| {
    if let Some(value) = e.value() {
        // We know there aren't any unpaired surrogates!
    }
})

So this API is not just about convenience or static type safety, it's also about fixing unpaired surrogates!

[Pauan]

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.

[yoshuawuyts]

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?

[Pauan]

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?

[yoshuawuyts]

@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.

[Pauan]

@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.

[Pauan]

@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

[yoshuawuyts]

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.

[Pauan]

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.

[chris-morgan]

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.

[yoshuawuyts]

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.

[chris-morgan]

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.

[Pauan]

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.

[fitzgen]

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.

[Pauan]

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);
}

[fitzgen]

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.

[fitzgen]

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.

[Pauan]

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.

[chris-morgan]

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).

[Pauan]

@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.

[olanod]

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.

[fitzgen]

@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.

[Pauan]

@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.

[dcormier]

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.

[Pauan]

@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