dom icon indicating copy to clipboard operation
dom copied to clipboard

[Proposal] Add EventTarget.getEventListeners()

Open LeaVerou opened this issue 8 years ago • 89 comments

Originally posted at WICG: https://discourse.wicg.io/t/proposal-eventtarget-prototype-geteventlisteners/2015

Purpose: This would provide a way to get all event listeners added to an element via addEventListener.

Signature: element.getEventListeners([type])

Returns: Array of objects. Each object contains properties for type, callback, options for the arguments that registered the corresponding event listener. Events registered via inline event handlers are not included.

Use cases: This would enable removing events based on arbitrary criteria, instead of requiring a reference to the callback, which causes unnecessary couplings. Typically libraries deal with this by providing their own abstractions for adding events that track the listeners manually. However, this is fragile, as it means listeners not registered via the library cannot be retrieved or removed. Some libraries deal with this by hijacking addEventListener to keep track of listeners, but this is very intrusive for a library and it doesn't help with any listeners registered before the library was included. Browsers already keep track of event listeners, so it should be relatively easy to expose them, and is on par with the Extensible Web Manifesto principle of exposing browser "magic" via JS APIs.

LeaVerou avatar Feb 16 '17 21:02 LeaVerou

On balance I think this is probably worth it, but we should note the solid objections to this on a few grounds:

  • This opens up the ability for APIs (e.g. custom element APIs) to change their behavior based on the presence or absence of an event listener, which is very bad design. Adding event listeners should be side-effect free.
    • On the other hand, this opens up the ability for libraries to perform unobservable optimizations as side effects of listeners being present, if they are careful.
  • This opens up the ability to unregister listeners which you did not register, and thus interfere with other parts of your codebase or other code running on the same page in hard-to-debug ways. The current model has an "object capability" design wherein you can more easily reason about the system. (Note: this is not a security boundary, just a reason-about-code-better boundary, analogous to how you can't resolve or reject a promise unless you are the one that created it.)

I think both of these objections are solid, but as I said, on balance I think they are overcome. The fact that people are hacking around the current design so much, and that this abstraction appears in other platforms (e.g. jQuery's system, or Node.js's EventEmitter) implies to me it's probably better to expose.

It's probable that this subject has been discussed before, maybe pre-GitHub; I wonder if we could find those threads to see how the discussion went then and if anything has changed.

Events registered via inline event handlers are not included.

This is weird, but I see the issue; currently the actual listener function is not reified, and it would be unclear what removeEventListener("event", reifiedListener) does (does the content attribute or IDL attribute stick around?).

Browsers already keep track of event listeners, so it should be relatively easy to expose them, and is on par with the Extensible Web Manifesto principle of exposing browser "magic" via JS APIs.

I think this is a misinterpretation of the situation. Browsers don't have the magic ability to look at all event listeners; just like user code, they only have the ability to dispatch events. The actual event listener list is usually stored as a private variable in the EventTarget backing class, with only equivalents of addEventListener/dispatchEvent exposed to the rest of the codebase. You could always modify a browser to make that private variable public, but that's the same as what we're discussing here for the web. So there's no browser-only magic we're exposing here; this would be a purely new capability, not a piece of bedrock we're unearthing.

domenic avatar Feb 16 '17 21:02 domenic

All good points there, thanks for the thoughtful response @domenic!

On another note, another possible design would be to have the same signature as addEventListener with all three arguments (type, callback, options) which would act as filters for which listeners to return. I suspect if the only supported filter is type, the rest will be emulated by developers via .filter() on the returned array.

LeaVerou avatar Feb 16 '17 21:02 LeaVerou

Someone should probably research https://lists.w3.org/Archives/Public/www-dom/ as I recall this coming up quite a few times in the past and being dismissed each time. I don't recall much else unfortunately.

annevk avatar Feb 17 '17 08:02 annevk

This is what I could find: https://lists.w3.org/Archives/Public/www-dom/2012AprJun/0131.html https://lists.w3.org/Archives/Public/public-whatwg-archive/2012Jun/0157.html https://lists.w3.org/Archives/Public/www-dom/2014JulSep/0029.html

At first glance, it appears the pushback is entirely comprised of the theoretical purity arguments that @domenic mentioned would come up. No implementation difficulties or anything of the like.

Tab mentioned an alternative in the last thread: Allowing an option for namespacing and removing events via namespaces, with both strings and symbols allowed. This is a good idea but does not solve all use cases. For example, a major use case is cloning an element with its events. In that case you legitimately want access to listeners you did not bind.

LeaVerou avatar Feb 17 '17 15:02 LeaVerou

I also found https://lists.w3.org/Archives/Public/public-webapi/2008Apr/thread.html#msg66 that isn't very conclusive either way, mostly folks not convinced by the use cases given at the time. (Although allowing iteration over them is somewhat novel I suppose.)

(The identifier grouping idea is #208.)

annevk avatar Feb 17 '17 16:02 annevk

I don't think "theoretical purity" is a fair characterization of those arguments. They are practical arguments from real developers who want to be able to develop components without being afraid that any other piece of code on the page could interfere. The lack of ability to reason about program control flow is an actual issue that affects developers every day, not a theoretical one.

domenic avatar Feb 17 '17 16:02 domenic

For example, given the code for TacoButton here, you'd no longer be able to guarantee that all taco-buttons on the page properly forward enter/space keydowns to click events, or that they stop firing click events when disabled. Being unable to reason about your program in this way is a real burden, and it changes your story as a component author from "include my component and it'll work" to "include my component but make sure all your third-party scripts aren't doing some over-aggressive cleanup action or you'll see strange behavior".

I still lean toward exposing the functionality, and then you'll just have to accept that you can no longer write encapsulated components in that way, but I think it's a real tradeoff between two different types of developer convenience, not one between theoretical purity and developer convenience.

domenic avatar Feb 17 '17 16:02 domenic

It is an important security property that nobody without a reference to my listener function can remove my listeners.

If a use case is cloning a node with all listeners, perhaps this could be handled with a "copyListeners" API (taking event names as filters, eg) without the need to expose the listener references directly?

What about adding a new "addEventListener" option that opts the listener in to being unprotected? That way the default would remain the same - secure - but users who wanted to waive a bit of security for the convenience of jQuery-like event manipulation could do so?

ljharb avatar Feb 17 '17 16:02 ljharb

I like the idea of namespacing events; this way, I can remove all my handlers later on without needing to keep their references (which is very helpful when binding functions). As far as I understand, when using a Symbol as a namespace one could create a private namespace which could not be intercepted by third parties (which might be important for developers of third-party code).

Example:

(() => {
  const namespace = Symbol('eventNamespace');

  const myObj = {
    a: 'a',
    myFn() { console.log(this.a); },
  };

  function add() {
    document.addEventListener('click', myFn.bind(myObj), { namespace });
  }

  function remove() {
    document.removeEventListener('click', null, { namespace });
  }
})();

Combined with lhjarbs idea of a copyListeners API, this should probably cover most of the use-cases?

nilssolanki avatar Feb 17 '17 16:02 nilssolanki

It is an important security property that nobody without a reference to my listener function can remove my listeners.

To be clear, it is not a security property, because on the web our threat model is not meant to defend against malicious code running on the same page. It is purely a developer-reasoning-about-code property.

domenic avatar Feb 17 '17 17:02 domenic

Regardless of the characterization of these arguments, it's a false sense of security, since this is possible today by wrapping addEventListener. There are libraries that do this. However, I think we both agree that there's a multitude of reasons why this is better done natively than having a library hijack addEventListener, right?

LeaVerou avatar Feb 17 '17 17:02 LeaVerou

It's certainly not an intentional security property, but that does not mean it is not an emergent one.

I can defend against wrapping addEventListener by caching it in first-run code.

I definitely agree it should be done natively - i'm simply suggesting that it be opt in, not by default.

ljharb avatar Feb 17 '17 18:02 ljharb

I can defend against wrapping addEventListener by caching it in first-run code.

If you're writing a library, you don't control whether your library is first-run code, so tough luck. If you're not writing a library, then what's the point of defending anything? You control exactly what runs.

LeaVerou avatar Feb 17 '17 18:02 LeaVerou

I can extract an untainted addEventListener from an iframe, I can include in the documentation that it needs to be first-run, and often ad network code is ran - last - that I don't control. I don't think it's productive to try to make a claim that making code more robust is a fool's errand; whether it's part of the intentional security model of the web or not.

Is there a reason that an opt-in mechanism (at the addEventListener callsite, like how passive: true works) wouldn't be a good middle ground, where you could get the functionality you want with zero risk of breaking existing axioms, and zero risk of developers having difficulty reasoning about their code?

ljharb avatar Feb 17 '17 18:02 ljharb

Is there a reason that an opt-in mechanism (at the addEventListener callsite, like how passive: true works) wouldn't be a good middle ground, where you could get the functionality you want with zero risk of breaking existing axioms, and zero risk of developers having difficulty reasoning about their code?

One of the most basic HCI (or human psychology in general) principles is that when something is opt-in, few do it. Most of the time because they simply never considered it. Therefore, it renders the feature useless for libraries, and only helps in reducing coupling in one's own code.

Another basic HCI principle is sensible defaults. Most listeners have no reason to be hidden, so the default should be non-private.

Opt-out might work, like a stealth option for listeners.

LeaVerou avatar Feb 17 '17 18:02 LeaVerou

@LeaVerou

Regardless of the characterization of these arguments, it's a false sense of security, since this is possible today by wrapping addEventListener. There are libraries that do this. However, I think we both agree that there's a multitude of reasons why this is better done natively than having a library hijack addEventListener, right?

Just my two cents here, but I disagree. As has been pointed out, once you can enumerate event listeners, you can implement something like Node's removeAllListeners, which warns

Note that it is bad practice to remove listeners added elsewhere in the code, particularly when the EventEmitter instance was created by some other component or module (e.g. sockets or file streams).

In practice, I've found this means that, as a library author, if you vend event-emitting objects to your users whose events you also use to drive internal functionality, you need to separate the EventEmitter interface your users will depend on from the one your internal functionality depends on (otherwise, a removeAllListeners call could break you, and it's annoying to document around…). I would not like to see the DOM APIs follow suit, and I would prefer encapsulation over convenience in this case.

EDIT: Sorry, @LeaVerou re-reading what you wrote, I too agree hijacking addEventListener is bad practice.

markandrus avatar Feb 17 '17 18:02 markandrus

@markandrus The stealth option I mentioned above would solve this. Or namespaces, with a default namespace so that listeners bound without a namespace can still be retrieved. I assume UAs would also need this functionality, to protect their own internal listeners. Although I'm not sure why anyone would call removeAllListeners without wanting to, you know, remove all listeners.

LeaVerou avatar Feb 17 '17 18:02 LeaVerou

@LeaVerou a mechanism like stealth would be useful 👍 I am happy so long as there is at least an opt-out.

Although I'm not sure why anyone would call removeAllListeners without wanting to, you know, remove all listeners.

There are many different types of developers... I've heard bug reports from some who called this function expecting it to remove only "their" event listeners.

markandrus avatar Feb 17 '17 18:02 markandrus

Having overridden window.addEventListener to work around a browser bug, I would welcome this change as a way to make my code more obvious (get event listeners and remove where it makes sense) and more robust (don't have to have my code run first thing to have it work).

ibash avatar Feb 17 '17 23:02 ibash

If you want to copy your event listeners, you can redispatch your events on the old node. That doesn't require destroying the currently existing abstraction.

function redirect(event) {
  var ne = event.constructor(event.type, event);
  document.querySelector('input').dispatchEvent(ne);
}

https://jsfiddle.net/yrewbnes/

FremyCompany avatar Feb 17 '17 23:02 FremyCompany

@FremyCompany that fiddle doesn't seem to work in Chrome/Firefox/Safari. You'll also lose trustedness of the event that way and target/currentTarget and such.

annevk avatar Feb 18 '17 15:02 annevk

@annevk - it seems to just be missing a new (new event.constructor(event.type, event);).

phistuck avatar Feb 21 '17 06:02 phistuck

From a purely implementation point of view I don't think this would be a terrible hassle to implement. Blink internally uses the same EventListener machinery for some internal use cases, like IndexedDB index population and media controls and things. We'd just need some level of filtering or segregating for them.

I'm not sure about window. Are there any times you have cross origin iframes and different domains get to stick event listeners on the same window through the window proxy or anything like that?

As a recovering? wannabe? web developer I'm mildly anxious about the code that's out there assuming their event listeners, once registered, won't be messed with. On the other hand if you can get a reference to the EventTarget anyway you probably can already cause interesting headaches.

dominiccooney avatar Feb 21 '17 07:02 dominiccooney

IMHO the only reason to getEventListeners() is because you want to clean up your own listeners after leaving. But, you can put a little effort in order to track manually your listeners. Also, @domenic said

... and it changes your story as a component author from "include my component and it'll work" to "include my component but make sure all your third-party scripts aren't doing some over-aggressive cleanup action or you'll see strange behavior"

For me, this statement has a lot of sense... So, getEventListeners() doesn't add a significant value to DOM.

rianby64 avatar Feb 21 '17 07:02 rianby64

I've heard it many times. "If it's in the standard we're supposed to use it because it's the way to go, right?"

I think that's why everyone above is worried about it breaking abstractions.

Calling this method should not seen an obvious choice for a novice developer so that they put in some effort into managing their listeners instead of looping through the list and removing each one when they clean up.

How about making it a "static" method of the DOM node, so that it looks like Object.keys? I'm not sure if hurting discoverability of this feature is really helpful, but noone brought it up before.

Additionally, instead of taking this listing function as a solution for cloning nodes, it'd be cleaner to just have a method for cloning with events. Having that available removes the main use case for getting all events. The other major use case is inspection, but that is already available on Dev tools. Might need improvements.

OK. Final thought. If this goes in, please group the output by some categories that users could set, like "click.myown" to help people use it the right way.

naugtur avatar Feb 21 '17 07:02 naugtur

Another use case I came across yesterday: I was in a talk about an accessibility tool that automatically adds keyboard shortcuts to existing web apps. If they cannot read whether there are existing key* events on each command, they cannot avoid controls that already have keyboard shortcuts. (although this argues more for a declarative way to specify keyboard shortcuts than for this…)

LeaVerou avatar May 10 '17 23:05 LeaVerou

@LeaVerou - plus, having an event listener does not mean it supports keyboard shortcuts. accesskey exists for a declarative way.

phistuck avatar May 11 '17 07:05 phistuck

Small thread hijack: I'd like to draw the attention of people in this thread to the related, but distinct, proposal in #208. Although it doesn't cover all use cases of this thread, it covers some of them, so the audience may be similar. If you work on a library or app that would use the proposal in #208, let us know over in that thread, as implementers are currently trying to judge developer interest.

domenic avatar Jun 25 '17 17:06 domenic

Here's another use case I didn't see mentioned here: accessibility auditing tools currently have no API to inspect event listeners, so we can't point out inaccessible UI controls. It's a huge gap in what we can automate for WCAG compliance, and I'd LOVE to see an API for this.

For example, a DIV with a click handler and simple text in it would be a perfect candidate for an accessibility violation. We can look at inline onClick handlers (and we plan to), but not the ones bound in memory. In JavaScript frameworks, many event bindings start their life inline in templates but are removed from the rendered HTML, so we can't sniff those out unless we can evaluate before the page is rendered–not a likely scenario for many accessibility testers.

There are a few potential false positives we've identified, such as a wrapper DIV with a click handler for a larger hit area but legitimately accessible UI controls inside (which is fine), and elements bound through event delegation on a parent element. But having an event detection API would be an excellent starting point.

marcysutton avatar Dec 05 '17 00:12 marcysutton

@marcysutton for your use case, do you need access to the actual listener function, or simply to determine if a handler exists?

ljharb avatar Dec 05 '17 00:12 ljharb