Typed Event Handlers

[lukechu10]

Event handlers should not take a base web_sys::Event but rather the appropriate type for the type of event. See Yew docs from more information on what this should look like: https://yew.rs/concepts/html/events#event-types

[oovm]

I need this feature, is there any preliminary design, I can do this binding.

By the way, even with Typed Event, there is still no auto-completion on my IDE: Code completion regression in web-sys

[mc1098]

So I have been thinking about this and toying around in a local branch about a solution.

NewType Events vs Event Name Trait

One decision is whether to have a describing trait to replace the event name or whether each event have their own event type and implement a SycamoreEvent trait. I think the latter after seeing the #176 issue which will require Sycamore having its own event types so this can be a baby step there. Also with a unique event type the builder API can become less verbose:

This:

input().on("keyup", |e: web_sys::Event| {
    let keyup_event: web_sys::KeyboardEvent = e.unchecked_into();
    let key = keyup_event.key();
    // do something with key
}) 

can become this:

use sycamore::web::event::KeyupEvent;

input().on(|e: KeyupEvent| {
    let key = e.key();
    // do something with key
})

If KeyupEvent implements the SycamoreEvent trait then it will also define the const EVENT_NAME: &'static str which we can use internally in the builder to get the event name without the user specifying it. Obviously if the user doesn't set the type for the event then they will have to add the event in the generics:

use sycamore::web::events::ClickEvent;

button().on::<ClickEvent, _>(|_| {})

Note: The second generic is required because of the Fn(E) + 'a generic defintion - this might be changed in the near future when rust allows for mixing explicit generics and impl trait defintions.

Now whether this is better than just setting |_: ClickEvent| is up to user preference, but either way we have saved them from typing the event name in a string.

In terms of the view macro because the on: directive is always used we can set the generic event type for the user using the alias, click for ClickEvent etc, this means that users can use inline anonomous closures in the view without having to set the event type in the closure.

The SycamoreEvent trait could be defined as such:

pub trait SycamoreEvent {
    const EVENT_NAME: &'static str;
}

Oh so simple... but not for long 😈 once we get through allowing Custom Events we need to become more paranoid 😞.

Custom Events

Currently Sycamore allows any Ident to be used after the on: directive and then converts that to a string to register the event handler which takes a G::EventType. This does not support all custom events only those with one word names so an event like MDCSnackbar:closing will not be parsed by the view macro.

If we relied on the fact that each event type was just a Newtype around an existing event, such as web_sys::Event, then we could easily add this without much change to the DomNode code and allow transmute to dissolve away the NewType which is sound to do. The problem is that with #176, Sycamore is likely going to want to keep some local fields in new events and for custom events we can't guarantee whether the actual type might be a Newtype so transmuting custom events become dangerous at runtime. This means that new SycamoreEvents must be created from the GenericNode::EventType which might lead to something like this:

pub trait SycamoreEvent {
    type BaseEvent;
    const EVENT_NAME: &'static str;
    
    fn from_base_event(e: Self::BaseEvent) -> Self;
}

This ties the SycamoreEvent implementation to a specific base event type so for the web this is web_sys::Event and this allows custom events to be constructed safely from the raw event (or as safely as the user chooses).

For web_sys events this is pretty easy to implement:

use std::ops::Deref;
use sycamore::generic_node::SycamoreEvent;
use wasm_bindgen::JsCast;

struct ClickEvent(web_sys::PointerEvent);

// need deref so we can access the underlying pointer event
impl Deref for ClickEvent {
    type Target = web_sys::PointerEvent;
    
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl SycamoreEvent for ClickEvent {
    type BaseEvent = web_sys::Event;
    
    const EVENT_NAME: &'static str = "click";
    
    fn from_base_event(base_event: web_sys::Event) -> Self {
        Self(base_event.unchecked_into())
    }
}

So to see the above trait in action, the DomNode::event function might be implemented like:

fn event<'a, E, F>(&self, cx: Scope<'a>, mut handler: F)
where
    E: SycamoreEvent<BaseEvent = Self::EventType>,
    F: FnMut(E) + 'a,
{
    let boxed: Box<dyn FnMut(web_sys::Event)> =
        Box::new(move |e| handler(E::from_base_event(e)));
    // safety message as before
    let handler: Box<dyn FnMut(web_sys::Event) + 'static> =
        unsafe { std::mem::transmute(boxed) };
    let closure = create_ref(cx, Closure::wrap(handler));
    self.node
        .add_event_listener_with_callback(
            intern(E::EVENT_NAME),
            closure.as_ref().unchecked_ref(),
        )
        .unwrap_throw();   
}

current version of impl here for convenience: https://github.com/sycamore-rs/sycamore/blob/ea676489715c02c8e9cd57f41ed520b9691f124d/packages/sycamore-web/src/dom_node.rs#L326-L336

view Macro Support

The problem with typed events for the view macro is recognising the built in events and custom events well as the on: directive will now be parsing a path or ident associated with a type that implements SycamoreEvent. I can think of two ways to support this in the view macro without substantially changing the syntax (like adding a new custom directive or something like that which I'm not a fan of).

Parse Qualified Paths

One solution is that the on: directive should parse a Path instead of an Ident, it has to be a Path because (1) we probably don't want to litter the prelude namespace anymore; and (2) the input event conflicts with the input element. Now this might not be such an issue if we used the Rust type names like on:ClickEvent= but I'm not sure this will go down well.

So if Sycamore included an events module then it might look like this:

view! { cx,
    button(on:events::click=|_| {}) { "Click Me" }
}

More verbose... obviously users can decide to use sycamore::web::events::click then just use click as before but we can't just dump all of it into prelude.

This gives some auto-completion because it is just a type and the wrong type will cause compilation errors it is also easy to see how a custom event could be used:

struct MyCustomEvent(web_sys::CustomEvent);
#[allow(non_camel_case_types] // I think this is the right name :)
type custom = MyCustomEvent;

impl SycamoreEvent for MyCustomEvent {
    const EVENT_NAME: &'static str = "my:custom";
    //other impl
}

view! { cx,
    div(on:custom=|_| {})
    // can just use the Rust type name ofc 
    div(on:MyCustomEvent=|_| {})
}

I'm in two minds here.. the more verbose nature of paths is annoying and adds more typing but it far less magical and mirrors the state of how these types are used in the builder API.

List of known event names in Macro

This was how yew parsed event listeners, I'm not sure if they still do this tbh.

Another alternative to just accepting qualified paths is for the view macro to keep a list of known event names, similar to boolean attributes, and if the name is known then in the macro we append the qualifying path to the builtin definition otherwise we assume it is locally available and use the ident as is.

There is no auto-complete with this option because the view macro wouldn't know it was a builtin event until the name matched completely, of course it will still cause a compilation error if you typo so on:clik will be cause an error.

This does lead to a problem where the crate defining the events needs to stay in sync with the view macro and vice versa which from experience with yew is a pain. This would lend itself to needing a config file with all the events and using build scripts to create the list for view macro and another for creating the definitions for the events.

I'm not sure how much of a problem this really is but one thing with this approach is that users cannot override the default behaviour of builtin event names so even if they define their own ClickEvent with an alias of click this will still use Sycamores definition; however, they can use on:ClickEvent which will find their local definition so it just limits the list of names defined in the macro.

Conclusion

Sorry the comment is so long 😞, I hope it all makes sense! I do have some of this implemented in a local branch but there are quite a few decisions here so seemed better to just layout some of the ideas here and get some feedback and better yet if someone comes up with a better idea from this :)