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

Event delegation via EventListenerOptions #215

Open
ghost opened this issue Apr 11, 2016 · 25 comments
Open

Event delegation via EventListenerOptions #215

ghost opened this issue Apr 11, 2016 · 25 comments
Assignees
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: events

Comments

@ghost
Copy link

ghost commented Apr 11, 2016

Event delegation is a common feature of most framework's event handling libraries. It'd be rad if it was standard in DOM events.

obj.addEventListener('click', ..., { matches: ".foo" })

Things to figure out:

CC: @annevk
References: #207 #208

@annevk
Copy link
Member

annevk commented Apr 11, 2016

See https://gist.github.com/annevk/4475457 where I had the idea of adding an ignoreBubbles member that would basically cause the "bubbles" attribute to be ignored during dispatch (we would have to move where we check the bubbles attribute, but that seems fairly doable).

The combination of ignoreBubbles and matches would then enable event delegation as desired.

I was thinking matches would just filter on event.target, but I might be missing a subtlety.

(Thanks for raising this!)

@josh
Copy link

josh commented Apr 11, 2016

See https://gist.github.com/annevk/4475457 where I had the idea of adding an ignoreBubbles member that would basically cause the "bubbles" attribute to be ignored during dispatch (we would have to move where we check the bubbles attribute, but that seems fairly doable).

I dig it.

I was thinking matches would just filter on event.target, but I might be missing a subtlety.

Yeah, a rough implement usually looks something like:

document.addEventListener('click', function(event) {
  var delegateTarget = this
  var currentTarget = event.target.closest('.foo')
  if (!currentTarget) {
    return
  }

  // ...
})

@annevk
Copy link
Member

annevk commented Apr 12, 2016

Interesting. That snippet seems to set delegateTarget to event.currentTarget (the callback this value) and currentTarget to the object you're most likely interested in. Would it not make more sense to name them the other way around? We could even change the callback this value to match currentTarget since the filtering will happen before the listener is invoked. Not entirely sure what's most desirable.

Would love your input on the details and also from @wycats if he can find the time.

@domenic
Copy link
Member

domenic commented Apr 12, 2016

In general I think we should anticipate the callback this value becoming much less useful once arrow functions arrive. So exposing the relevant information through ev.{target,currentTarget,delegateTarget} or similar is important.

From what I remember using event delegation back in my jQuery and Backbone days it was always confusing which of target or currentTarget was correct for my use case.

@annevk
Copy link
Member

annevk commented Apr 12, 2016

Maybe if the API explicitly couples them it would be clearer:

obj.addEventListener(type, (e) => {
  e.target // the actual target
  e.currentTarget // obj
  e.delegateTarget // .baz
}, { delegate: ".baz", ignoreBubbles: true })

If delegate is not passed delegateTarget would return null or the same as e.target...

Another thing to consider is whether ignoreBubbles should default to true when delegate is passed. That might make sense since it's typically what you want to do. We'd still offer ignoreBubbles as a standalone primitive so you implement your own delegate if you're not happy with Selectors and want XPath or some such.

@josh
Copy link

josh commented Apr 12, 2016

Interesting. That snippet seems to set delegateTarget to event.currentTarget (the callback this value) and currentTarget to the object you're most likely interested in. Would it not make more sense to name them the other way around? We could even change the callback this value to match currentTarget since the filtering will happen before the listener is invoked. Not entirely sure what's most desirable.

Not saying I prefer it, but these are the semantics jQuery uses.

I'd say there logic is that they wanted this to be the element that matched the selector since thats the most commonly used target. And it followed if this was the matched element, then this === currentTarget.

Similarly, if you direct bind the handler, the matched target is your current target.

document.querySelector('.foo').addEventListener('click', function() {
  this // is a .foo
})

obj.addEventListener('click', function() {
  this // is a .foo
}, { matches: ".foo" })

I've pretty much never used delegateTarget in my own code. But its mostly there for completeness to known the actual element the event handler to bound too.

event.target - The DOM element that initiated the event.
event.currentTarget - The current DOM element within the event bubbling phase.
event.delegateTarget - The element where the currently-called jQuery event handler was attached.

https://api.jquery.com/category/events/event-object/

If we want to keep target and currentTarget the same, maybe adding a new third thing like you suggested, event.matchedTarget or event.delegatedTarget would be fine.

We'd still offer ignoreBubbles as a standalone primitive so you implement your own delegate if you're not happy with Selectors and want XPath or some such.

👍 My "delegated xpath events" lib is going to be so popular someday!

@annevk
Copy link
Member

annevk commented Apr 13, 2016

@jaubourg @paulirish any input on whether we should attempt to copy the jQuery conventions and adjust what currentTarget means or whether we should introduce a new property (and keep currentTarget for the object to which the listener was attached)?

@annevk
Copy link
Member

annevk commented Apr 15, 2016

@scottgonzalez any thoughts?

@chemerisuk
Copy link

Agree with @domenic . Having a special logic for this in case of event delegation is confusing. Same for redefining event.currentTarget - much clearer to add a new field like event.delegateTarget. DOM frameworks can check existence of that property to use perf optimization, otherwise call Element#closest with the appropriate selector.

@gibson042
Copy link

I'd like to argue for formalizing jQuery semantics as follows:

  • event.target: "the object to which event is dispatched" (no change).
  • event.currentTarget: "the object on event's propagation path for which callbacks are currently being invoked" (generalizes the current definition¹ to a model with delegating). Also used as the this context value for callbacks (no change from 2.6 of inner invoke in 3.8. Dispatching events).
  • event.delegateTarget: "the object whose event listener’s callback is currently being invoked, equal to event.currentTarget when the event listener is not a delegate" (new, text copied from current event.currentTarget).

Doing so would preserve two important properties of the current model: equivalence of currentTarget with callback this context, and currentTarget tracking movement along the event path. I know from experience that both of those are often assumed in event handlers, and can't see how breaking them by essentially swapping the names of jQuery's precedent could lead to anything but confusion.

You can get a feel for things at https://jsfiddle.net/yrk2pwky/, which also highlights the interesting edge case of a single event matched by the same delegate event listener at multiple points along its propagation path (for which I'm not married to jQuery behavior).

¹ "the object whose event listener’s callback is currently being invoked" in the current (non-delegating) model

@scottgonzalez
Copy link

@annevk Thanks for the ping. I had started preparing a response, but decided to pull in someone else with more experience in the actual implementation of jQuery's event system. You can take @gibson042's response in place of what I was going to write :-)

@dmethvin
Copy link

I agree with @gibson042 as far as the targets go. It would introduce more confusion to redefine everything.

The rough implementation that @josh mentions above is much simpler than what is happening in jQuery though. That can only execute the handler once in the subtree and that particular code as-is could execute on an element above the delegation point.

The jQuery implementation bubbles the event from the target element up to but not including the delegate point element and executes the handler for any matches it finds; there can be multiple matches along the path. For each match, the handler executes and has the opportunity to call event.stopPropagation() to prevent further bubbling up to (or beyond) the delegation point. jQuery only handles bubbling for historical reasons so it doesn't need to worry about the symmetry of capturing for delegation.

I don't know whether jQuery's more general implementation is required to handle a lot of use cases or not.

@annevk
Copy link
Member

annevk commented Apr 24, 2016

Thank you for the input, this is much appreciated. Let's see if I have understood it all with an example. Tree: <a id=root> <b> <a id=2> <c> <a id=1/> ...

Now I register a delegate listener on #root using root.addEventListener("x", callback, { delegate: "a" }). Then I dispatch an x event on #1. This means that callback is first invoked for #1, with target and currentTarget being #1, and delegateTarget being #root. Then callback is invoked for #2, with target being #1, currentTarget being #2, and delegateTarget being #root. Then it stops, since we only look at descendants and there are no more a descendants of #root.

This is a little more involved than using selectors to filter the target. Would be interesting to hear what @dominiccooney and @smaug---- et all think of that.

It also gives us two choices as to how they integrate with the existing model:

  1. While going through the event path during the bubbling phase, also look at ancestor listeners that have "delegate" set and see if the specified selector matches the node in the event path.
  2. While invoking a listener that has "delegate" set, during the bubbling phase, go through its corresponding "currentTarget"'s descendants on the event path in reverse order.

Neither seems really attractive to me.

@gibson042
Copy link

Yes, and I am strongly in favor of option 1 (process all delegate callbacks before advancing currentTarget along the propagation path). Also (to make it explicit) I see no reason why this should be limited to the bubbling phase.

@dmethvin
Copy link

Option 1 (for bubble and capture) would seem a better fit to the user's existing mental model about how events behave, option 2 is what jQuery was stuck with given the constraints of the current DOM API.

The main concern I'd have is the additional cost and complexity. For GitHub the amount of extra overhead from delegation via jQuery was too much so they created their own delegation code. However, it only works for delegation done from document which is fine for their use case but not this one.

@annevk
Copy link
Member

annevk commented Apr 25, 2016

Interesting, I suppose we should ask @dgraham for feedback too then. It seems the main performance benefit listed in that repository is no longer having to create synthetic event wrappers, which is not directly related to delegation. But yeah, for large trees having to check all ancestor nodes in an event's path while iterating through an event's path to find delegate listeners is either going to be slow or require complicated data structures.

@josh
Copy link

josh commented Apr 25, 2016

But yeah, for large trees having to check all ancestor nodes in an event's path while iterating through an event's path to find delegate listeners is either going to be slow or require complicated data structures.

The main performance issue is that iteratively testing selectors is linear with the number of installed delegated handlers. So yeah, you'd want to use a complicated data structure to make this more efficient. Luckily most of the hard work has already been done here by browser engines as they need theses structures for CSS rule set matching. Blink, Gecko and WebKit all use some sort of variation on a hash map or bloom filter for fast path testing. @github uses this selector set implementation for its batch selector testing.

However, it only works for delegation done from document which is fine for their use case but not this one.

There's nothing document specific about the approach. That library just omitted delegation root configuration as @github rarely uses it. To implement it, each delegation root would manage their own selector list rather than hard coding it to the document itself.

While going through the event path during the bubbling phase, also look at ancestor listeners that have "delegate" set and see if the specified selector matches the node in the event path.

I'd love to have this implementation. This would allow delegated handlers to use stopPropagation in a useful way that matches how directly bound event handlers would behave.

<a>
  <b>
    <c>
a.addEventListener('x', () => { log('a') })  
b.addEventListener('x', () => { log('b') })
a.addEventListener('x', (e) => { log('c'); e.stopPropagation() }, {delegate: 'c'})
  1. Would only log c
  2. Would log b, a, c

@fvsch
Copy link

fvsch commented May 5, 2016

I thinks this has been clarified already but some comments talk about using event.target.matches(delegateSelector), and what jQuery and probably others do is more like looking for event.target.closest(delegateSelector). Just wanted to stress that.

As a front-end dev if I have <button><span>Hey</span></button> and use "button" as my delegateSelector for click events, I do want clicks originating on the span to trigger my callback.

@dmethvin
Copy link

dmethvin commented May 5, 2016

what jQuery and probably others do is more like looking for event.target.closest(delegateSelector). Just wanted to stress that.

No, see my comment above. The user's own event handler often uses .closest() to find some enclosing element, but jQuery itself walks up the DOM tree and finds all matches.

@annevk annevk added the needs implementer interest Moving the issue forward requires implementers to express interest label Aug 23, 2016
@domenic domenic self-assigned this Jun 23, 2017
@domenic
Copy link
Member

domenic commented Jun 23, 2017

It's on my goals for this quarter (so, next week) to prototype a spec and tests for this. Unlike the other EventTarget stuff I've been taking on, I'm not sure I understand all the subtleties here fully, so help would be appreciated. Let me try to summarize what I think is the current best plan:

  • A new matches option (or delegate option? delegateSelector? filter? #bikeshedtime) that takes a selector
  • A new ignoreBubbles boolean option that defaults to true when matches is provided
  • event.currentTarget and the this value get redefined/changed when delegation is involved, to be "the object on event's propagation path for which callbacks are currently being invoked", which is apparently a generalization of the current definition.
  • A new event.delegateTarget property which is defined the way event.currentTarget is currently defined
  • The exact filtering process is as explained in Event delegation via EventListenerOptions #215 (comment) option (1); this may be slow, and will require prototyping in browsers to see if it's doable. Notably, this version is not as simple as just checking if the target matches the selector.

Does this seem reasonable?

@FremyCompany
Copy link

I certainly don't want to prevent you from prototyping this, but I don't understand what is the added value of the proposed event bubbling sequence. I would love if someone could explain to me in which cases is the proposed sequence is useful!

I am asking because EventTarget is a generic interface implemented by things for which css selectors do not mean anything, and if the proposed event bubbling is not something that can be proven to be an advantage, I have a tough time grasping why document.addEventListener('click', e => { ... }, { matches: "a[href]" }) is perceived as better than document.addEventListener('click', e => { if(e.target.matches("a[href]") { ... } });

@fvsch
Copy link

fvsch commented Jun 24, 2017

@FremyCompany It’s better because the proposed matching is not on event.target but on event.target and its ancestors up to the element the listener is attached to.

It’s a great question though, because it shows that this option should not be called matches:sel if it doesn’t do something close to a if(!event.target.matches(sel))return; at the start of your callback.

Tossing a coin in the bikeshed machine:

A new matches option (or delegate option? delegateSelector? filter? #bikeshedtime) that takes a selector

I vote "anything but matches or closest".
delegate or delegateSelector are okay-ish, but if you want event.delegateTarget to be the element the event listener is attached to then it’s a contradiction:

document.body // event.delegateTarget
  .addEventListener('x', event => {  }, {
    delegateSelector: '.foo' // event.currentTarget
  })

Possible solutions when keeping the "delegation" root:

  • delegation: '.foo'
  • delegated: '.foo' ("the elements for which we're delegating this event listener will match this selector")
  • delegateFor: '.foo' or asDelegateFor: '.foo' ("the element we're attaching an event listener to is a delegate for descendants matching this selector")

With the "for" root, you can also do:

  • forEach: '.foo' ("call the callback for each element matching this selector between the event.target and the event.delegateTarget")

My faves might be delegated and filter, but I have no strong opinion besides "beware of misnomers".

  • event.currentTarget and the this value get redefined/changed when delegation is involved, to be "the object on event's propagation path for which callbacks are currently being invoked", which is apparently a generalization of the current definition.
  • A new event.delegateTarget property which is defined the way event.currentTarget is currently defined

This seems fine. It follows the precedent set by jQuery but also adds the ability to retrieve the element the event listener is attached to (delegateTarget).

One could argue that the jQuery way was "bad" it would be cleaner to not redefine event.currentTarget and instead add a new property for the actual target (event.delegatedTarget maybe). But personnally, I’m okay with not having to relearn "use event.currentTarget in 99% of cases". :P

@FremyCompany
Copy link

Ok... I think I understand the proposal better now. Do I?

<section>  <span a>  <div>  <span b>  <a href="#">  <span c></span>  </a>  </span>     ...
div.addEventListener(
    'click', 
     handler, 
     {
         for: t => t.matches("span"), 
         ...otherOptions
     }
)

would be similar in spirit to

div.addEventListener(
    'click', 
    addDelegatedHandlers(
        handler, 
        t => t.matches("span"), 
        { ...otherOptions }
    ), 
    true
);

function addDelegatedHandlers(handler, condition, otherOptions) {
    return event => {
        let futureTargets = event.getFutureTargets() // [ span[b], a[href], span[c] ]
        let futureTargetsToHook = futureTargets.filter(condition); // [ span[b], span[c] ]
        for(let futureTarget of futureTargetsToHook) {
             futureTarget.addEventListener(event.type, handler, otherOptions);
             setImmediate(t => futureTarget.removeEventListener(event.type, handler, otherOptions));
        }
    };
}

Is that right?

@stuartpb
Copy link

Looping in this thread on WICG about event listeners applied by selector.

@Jamesernator
Copy link

Jamesernator commented Aug 21, 2017

Would this allow for multiple firings of events that can normally wouldn't fire again for the parent? (mouseenter/mouseleave are the only ones I think of that'd be affected by this)

For example would this fire an event for every delegated child that is mouseenter-ed?

container.addEventListener('mouseenter', event => {
    // do stuff
}, { matches: '.hoverable' })

Or would it still behave like current and only fire an event when entering the element the event listener is attached to (assuming it crosses a child that matches)? e.g. Roughly similar to the current code:

container.addEventListener('mouseenter', event => {
    // This is only fired on entering my container so turns out to be
    // pretty useless for detecting hover over child nodes
    if (event.target.matches('.hoverable')) {
        // do stuff
    }
})

SECOND EDIT: As it turns out I was getting doubly confused with both pointer-events and getting it confused with mouseover/mouseenter (and mouseleave/mouseout), so I think my question still applies to those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: events
Development

No branches or pull requests