w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
578 stars 50 forks source link

Proposal: Event.addListener should accept an interface. #519

Open newRoland opened 5 months ago

newRoland commented 5 months ago

addListener accepts a callback function as the first argument. The subsequent arguments are used for event-specific types. For example, webRequest.onAuthRequired.addListener takes a RequestFilterfor the second argument, and extraInfoSpec for the third.

There's no place to provide parameters that are relevant to all events. Allowing an interface for the first argument would provide the flexibility for future use cases and proposals. For example, I make use of this hypothetical interface in my #501 proposal.

browser.storage.local.onChanged.addListener(onChanged)

// flexible form allows for future use cases
browser.storage.local.onChanged.addListener({
    callback: onChanged
})
tophf commented 5 months ago

Maybe use handleEvent instead of callback to match DOM addEventListener?

newRoland commented 5 months ago

This must be null, an object with a handleEvent() method, or a JavaScript function.

I learned something new today, didn't know that was a thing.

dotproto commented 5 months ago

There's no place to provide parameters that are relevant to all events

I don't see why this specific solution would be the best way to address common parameters given the current design of WebExtensions APIs. It seems like we could accomplish something similar by having common parameters in an optional property bag. As you mentioned, a number of existing event listener already have the equivalent of an options parameter that contains a number of key-value pairs to control listener behavior. This feels to me like a better fit given the way event registration is currently handled in extensions APIs.

It's also worth noting that the web uses a different approach to what this issue proposes for common properties associated with all events; the web uses an options object on EventTarget.addEventListener() for things like providing an AbortController or signaling if the listener is passive.

Beyond that, I'm also very hesitant about changing how Event.addListener() behaves. Adding new behavior to all existing event listeners is a nontrivial amount of work and would increase the maintenance cost for browser engineers. In order to tackle something like that, I suspect that vendors will want to see substantial benefits to the new approach and possibly a deprecation plan for the the older approach. I am also concerned about the challenges related to supporting two different registration patterns on the same method names. To my knowledge browser engineers strongly prefer not to overload method signatures where possible.

All that said, I'm very interested in cleaning up how extension event listeners are registered and managed. I would like to explore what the platform would look and feel like if we used an event model that's more in line with the web platform. Specifically, things like using potentially using the EventTarget interface for extension namespaces. But that's all a bit tangential to this specific proposal.

newRoland commented 5 months ago

I'm interesting in having a designated area where you can supply parameters that might be relevant to all events. If the 2nd parameter makes more sense, that's also fine.

Beyond that, I'm also very hesitant about changing how Event.addListener() behaves.

My intent is to make it easier to change how the addListener behaves. If there's no appetite for that, I think this is a dead end proposal. I'll close it for now and mull it over. Thank you for the detailed reply.

dotproto commented 5 months ago

My intent is to make it easier to change how the addListener behaves. If there's no appetite for that, I think this is a dead end proposal.

At this point I don't think we know how much appetite there is. I have concerns, but I don't speak for any browsers. It might be worth discussing this idea in a meeting to get a sense of how this approach strikes the browser reps. Even if they raise concerns, that conversation may help inform what we explore next.

jpmedley commented 5 months ago

Specifically, things like using potentially using the EventTarget interface for extension namespaces. But that's all a bit tangential to this specific proposal.

Would this create a situation where documentation would constantly be saying this such as: like an interface except or like a namespace except. I'm being impressionistic because I'm not sure those exact words would be used. I'm trying to make the point that exceptions to existing patterns can be stumbling blocks to users even when they're adequately documented.

tophf commented 5 months ago

addEventListener accepts an interface for decades(?) and how many developers are confused by its documentation or implementation? I think it's probably 0 because this edge case is ignored by anyone who is not looking for it specifically. So it's possible to document/implement this functionality without creating unnecessary confusion.

jpmedley commented 5 months ago

I was about to elaborate, but realized I overlooked part of Simeon's statement. "But that's all a bit tangential to this specific proposal." I'll comment again if a specific proposal is made.