Improving event bindings/listeners

[titoBouzout]

Ive worked in SolidJS related to this topic, and Im fresh on some things. So I want to share what I think could be improved here.

Im not too familiar with ripple syntax so ignore my "typos", but assuming the framework supports spreads and some stuff is done in runtime.

Support native object syntax

There is https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#listener

as follows

<button onClick={{ handleEvent(e){ console.log(e) }, capture:true }}/>

pure js demo: https://playground.solidjs.com/anonymous/91b42fc0-4699-497d-a3df-15f9cfaee447

Plus points:

• gets rid of all the onClickCapture and friends
• easy way to opt out of event delegation, if its an object then do not delegate
• future proof, it supports by default: once, capture, and any future event option

Default to no-case-change

onClick has the problem that you need to lowercase the C. And unless you ship an array with the list of native events, then any custom event that has some case will break.

Even then, someone could come with a custom onKeyDown, and lowering will break it.

So in my opinion the non-brainer solution is to just remove the on and do not lowercase or transform the event name in any way. This makes it automatically compatible with any kind of event.

The downside is that some component authors may prefer to call some of their event-alike things onThingClosed, even when these things are just callbacks and no events. Which makes you have a mixed bag of things like {onclick:()=>{}, onThingClosed:()=>{}}. Maybe if you can convince component authors to name this onThingClosed differently you can get away with it.

A alternative aproach could be to prefix event names, I am not sure of ripple syntax but I can imagine something like, or any other way. pota does this.

<button on:click={...}/>
<button @click={...}/>

--

Well thats my 2 cents, as a framework that learns from the mistakes of the others, this seems to be a good candidate.

[trueadm]

I actually love this idea. @leonidaz what do you think?

[leonidaz]

Yeah, I think we should definitely add it.

I'm thinking to have an object with a listener and options, instead of mixing everything in one.

const click = { handleEvent: (e) => {}, options: {capture: true} }

<div onClick={click} />

const events = {
  onClick: { handleEvent: (e) => {}, options: {capture: true} },
  onMouseOut: { handleEvent: (e) => {}, options: {once: true} },
}

<div {...events} />

[titoBouzout]

can't then use the native onload event in the same way

Can you please clarify that? Thanks!

[trueadm]

Native onload can be this <img onload="imageLoader.log(event)"> also goes for onerror too. These are native HTML elements that people use with Ripple today.

[titoBouzout]

Ah yeah, I do not think supporting inlined js is a good idea. Up to you. Dynamic seems to work just fine https://pota.quack.uy/playground#H4sIAJEQEWkAA1WQ0QrCMAxFfyXsxQ20fRcVxN/oS9nqnHZJTTJQxv7djjFkT7kk3HOTFF2fiBVG4IBN4D3cKHcwoMIEd6YedonU7xw6vA9Ya0cI15TKCkaHkG06MEI5a4DTZalZdX0LhMfATHwea0KhGEykdgLh+uyKhoLklPDpVMwzta4Au7GvGmZOJN9sMf/xwnuoJjlaO69r3oOvX2b4Wi8SVGw20EF6H6NJmKNW85p4WkTlMHMdLs8o851V8QNzGI2hIwEAAA==

[leonidaz]

<img onload="imageLoader.log(event)">, onerror

I agree that onload and onerror can be solved by just be converting by ripple to be regular event listener with addEventListener but I also don't like all lowercase handlers, they are hard to read.

on prop – and like Remix

lol, last night I typed up a reply suggesting we use on. This morning I spent a few hours looking into remix's implementation and thinking if it makes sense for us to adopt something similar - I mean typesafe events. I wanted to reserve on for that purpose, but now I'm thinking we can just start with regular events and if we decide to, adopt remix's strategy later.

import { press } from 'ripple/events';
import { countChange } from './some.component.module.ripple';

// Wanted to avoid using event names as strings so that's why this object structure
<div on={[
  { click: {handler: (e)=> { console.log(e.target) }, options: {capture: true} },
  { PointerRawUpdate: {handler, native: true, options: {once: true, signal: someSignal}  },
  press((e, signal) => { fetch('/api', {signal}) }, {once: true}), // <-- remix-like by ripple
  countChange((e) => {}, {once: true}), // <-- remix-like your own
]} />

// one event use object
<div on={{ click: {handler: (e) => { console.log(e.target) }, options: {capture: true} }} />

// or just the handler
<div on={{ click: (e) => { console.log(e.target) }} />

Maybe, if you don't want to use all lower case for native events, you can just use native: true and we'll lowercase it, like above for PointerRawUpdate.

For the remix-like implementation, I was thinking that in addition to what they're doing, also implement scoped (to avoid name collisions) and global event names.

As far as spreading props

• we can just identify on in the props and attach the events.
• maybe also allow people to define random prop names for events as long as they attach a key to the event object, so we can identify it as an event. And then these can also be spread or added to on later, thus not limiting events only to on, e.g.

import { createEvent, EventKey } from 'ripple/events';

using a func wrapper
const myProp = createEvent({ click: {handler: (e) => { console.log(e.target) }, options: {capture: true} }});
 // or shorthand
const myProp = #event.click({ handler: (e) => { console.log(e.target) }, options: {capture: true} });

// or with a Symbol key
const myProp =  {click: { handler: (e) => { console.log(e.target) }, options: {capture: true}, [EventKey]: true } };
  // or shorthand
const myProp =  {click: { handler: (e) => { console.log(e.target) }, options: {capture: true}, #event: true } };

<Button {myProp} on={{ mouseout: {handler => () {}, options: {once: true}} }}>{'Click Me'}</Button>

component Button({ children, on, ...rest }) {
  <button {on} {...rest}><children /></button>
}

[trueadm]

I don't think we want to use the event name directly, but rather just use on no? Whilst click looks nice, things like dblclick and pointercancel look strange IMO.

<button on={[{ type: 'click', handler } ]} />

[leonidaz]

Forgot to ask, should we then get rid of the Capture suffix on event props, e.g. onClickCapture?

[titoBouzout]

Thats part of the idea

[trueadm]

That's also what I alluded to in #512 (reply in thread)

[leonidaz]

Let's summarize.

• We agree that removing Capture and introducing a way to define it and other options through an object notation is the right direction.
• Custom event names get currently lower-cased which may break event handling.
• We don't want to use all lower-cased event names with the on prefix, like onpointerrawupdate

We have a few proposals on the syntax but let's prioritize DX, as using onEvent syntax is very intuitive.

• I think we can solve the custom event naming while keeping the current react-like camel-cased syntax intact with an object notation.

// keep the current function syntax for browser-defined dom events
<button onClick={(e) => {console.log(e.target}} />

// with object notation browser-defined dom events:
<button onClick={{ handleEvent: (e) => {console.log(e.target}, capture: true, passive: true, signal, once: true }} />

// for custom events use object notation with `custom: true`
<button onwhateverMyheartdesires={{ handleEvent: (e) => {console.log(e.target}, custom: true, capture: true  }} />

• For custom events, use the object notation with custom: true if you don't want the name to be lower-cased.
  • on will still be removed as per usual.
  • to determine if it's an event, since capitalization after on is not required for custom events, we'd check that it starts with on, the prop value is an object and contains handleEvent and a truthy custom property.
  • if both above are true, no lower-casing will occur.
• Why handleEvent vs something like handler?
  • It matches the native property name for addEventListener when an object is used vs a function.
  • Strong semantics when trying to determine if it's an event for custom events, as they don't have to follow the camel-case notation.
• on prop - we can introduce it now or later once we're ready to do a remix-like type-safe implementation that requires passing in the node / element.
  • I think its ergonomics are not as good as onEvent, but if there is a lot value in it, then people will use it.
  • So I think we should wait until we're ready to do the remix-like approach.
  • But even then, I think we should keep the onEvent syntax - we can just say that whatever is inside on will override any of the onEvent handlers if they happen to be regular event handlers vs the remix-style.
• More importantly, after really digging into and understanding what remix 3 is doing, the on on components is not a prop, it doesn't get passed in to the component. It attaches an event handler to the component's root element or creates one with style="display: contents" if there is no root, e.g. <></>
  • so, on is really a beast of its own and we can tackle it separately. I now have a good understanding what's involved with on, so it's not that difficult to implement, but before we release it, we should do what remix 3 did by creating cross-platform events such as press, pressUp, longPress, etc.
  • Overall, I think the "component"-level declarative event handlers is a great DX and we should adopt it.

[titoBouzout]

add to the summary that the object form allows opting out of event delegation, and you can also say that this form is future-proof, as new events or options would be automatically available without needing to change anything.

[leonidaz]

I was just digging into our code, and one small side-effect is that we no longer can tell at compile-time if an event should be delegated for delegatable browser-defined events. This is because we can't lower case the event name for case-sensitive custom events, e.g. if someone wants to use Click vs click. But regardless, with objects being the value of the event property / attribute, we still can't tell at compile-time if someone uses the proposed custom: true property in the listener options. (custom: true - just means that the event name will not be lower-cased, we could name it something else, like 'keepCase: true', or ...)

But, checking if an event can be delegated at runtime should not affect performance or very little.

But, I still think we should delegate whenever possible and it should be the default behavior.

So, I suggest we flip it around and make every event, including custom ones delegatable, unless:

• it's an event that doesn't bubble, e.g. focus, blur, etc.
• capture is set to true
• a newly introduced delegated option is set to false

The other thing that comes with an object notation is reactivity on various options. I haven't seen it yet in the code but maybe we're already handling reactivity when a listener is a tracked function. But, regardless, the question is do we want to handle it?

• I think we should, to make things consistent. Which means that we have to remove and recreate event listeners.

[trueadm]

I've change my mind, I think we should largely keep what we have today as is, expect we'd remove the Capture suffix and enforce you can pass in an object { fn, capture, once, signal } and in that case the event handler is always inline and never delegated.

[titoBouzout]

What about an optional property custom? That when set, the jsx attribute name case is respected.

That way you can write onmYCustoMeVent={{ custom:true, handleEvent: console.log, capture: true, passive: true, signal, once: true }} and custom events will work. This can be figured out at compile time.

[leonidaz]

In any case, i think we should keep the custom flag to keep the casing on custom events. The only thing is that we'll need to only rely on event attributes having to start with on, without checking the capitalization of the next letter.

I think this would be a reasonable requirement. At runtime, though, if we see that the attribute value is not a function or an object with handleEvent, we can consider it a non-event attribute and process it accordingly.

[trueadm]

It just dawned on me that we can't statically know what this is onClick={thing} and that alone unravels a big problem with how the reactivity system works. Today we pass in the current active block to event handlers that need it, and re-write their function arguments to include __block. That now becomes impossible with this approach.

For that reason, I'm all for dropping this proposal.

Otherwise, we have to tell people that they can no longer create state inside event handlers – and that won't go down too well.

[leonidaz]

we can still attach __block at runtime when calling _$_.event, which would attach it to the element btn.__click = [handler, __block] like we do statically.

_$_.event is where this onClick={thing} use case would go, assuming that thing came outside of the component.

[trueadm]

It's more the fact we re-write the function arguments itself. We append __click to the end of the function arguments, skipping event if it doesn't exist (we add it in)

[leonidaz]

implemented in #561

@titoBouzout thank you for the feedback!

we ended up keeping onCapitalLetter requirement, but added customName that can be anything, including special characters like :, etc.

Capture suffix was also retained so that people can use and spread function handlers without having to resort to an object.

The object properties ended up being: handleEvent, capture, once, passive, signal, delegated, customName.

[titoBouzout]

Thats cool, glad to be of help 👍🏼