Issue a devtool warning if WorkerGlobalScope#addEventListener is called after intial execution

[jakearchibald]

For example, if listeners are added inside an oninstall listener, that's going to be lost at the next teardown. Is there any case where this is desirable?

[jyasskin]

We've definitely seen developers shoot themselves in the foot this way with Chrome event pages, so +1 to making this restriction.

You could imagine that the UA might skip starting a SW if it hasn't registered a listener for a particular event, and that the SW's author might try to optimize that startup process by registering listeners conditionally depending on how the user has configured them. Given SW's lack of synchronous storage APIs, there's no way to do that during the initial execution, so by banning this, I think we'd be giving up on that possible optimization. I think that's worth it, at least until the need becomes compelling.

[KenjiBaheux]

@jakearchibald Naive question: I could not understand why this is marked as an enhancement... Wouldn't we break folks who relied on the non throwing behavior or the optimization that jyasskin highlights?

Background: I'm going over the remaining opened issue to assess potential impact on our current implementation.

[annevk]

If we are defining something completely different from events, we should name these things somewhat differently imo.

[jakearchibald]

@KenjiBaheux

I could not understand why this is marked as an enhancement...

Maybe the wrong label. Removed it.

Wouldn't we break folks who relied on the non throwing behavior

We're only in Canary at the moment. I don't know if anyone is relying on the non-throwing behaviour, but we're changing lots of APIs day-to-day, I don't think it's a problem.

or the optimization that @jyasskin highlights?

Given how serviceworkers can be terminated at any time and spun up just in time to fire an event, I can't see how async listening would be a performance benefit.

@annevk

If we are defining something completely different from events, we should name these things somewhat differently imo

They're not completely different. You use subscribe to a thing that happens and get the opportunity to react, in some cases alter the default action. Due to the way serviceworker can terminate and start up at will, a listener attached after the initial execution is going to get lost.

[annevk]

I think they are different enough. What you are defining here requires all kinds of special casing for which there are no hooks in the basic eventing system.

[KenjiBaheux]

@jakearchibald here is how I personally see the enhancement label: it's something we could do in a follow-up without negatively affecting the core functionality. My simple test: regardless of having something that actually shipped, if X is gonna break expectations about the core functionality (minimal viable product; i.e. non enhancement bits) then it's not an enhancement. Ideally, we can ignore the enhancements and focus on resolving issues affecting the MVP.

[jakearchibald]

Works for me

[domenic]

I am still concerned (like @annevk) that this is being considered. If it is desired functionality, we should not use events, which is a change that needs to be made ASAP. If events are the desired model, we should work to align with them.

In particular this should be viewed from the point of view of special-casing, as he says. E.g. if addEventListener throws sometimes, then you surely aren't using EventTarget.prototype.addEventListener, which is bad.

[annevk]

See also #225 btw. If we can do something better here than event abuse that'd be great.

[annevk]

#256 seems to be about a better API.

[jakearchibald]

We decided to drop the throwing requirement in favour of warning via devtools

[annevk]

Note that if you during a fetch event you invoke addEventListener to add an additional listener for fetch, that'd be perfectly fine.

[KenjiBaheux]

Updated summary to reflect the latest decision as I understand it.
Impact on Blink's implementation tracked at crbug.com/401870 (*)

*: I hope you don't mind but I think cross-linking implementation bugs and spec bugs will helps us keep things in sync.

[jakearchibald]

Works for me. Good to keep track of browser progress.