How do we handle listeners in Workbox

[gauntface]

we had a discussion in a sync regarding moving all event listeners over to default exports only
We agreed to add it to the styleguide
the reason is to ensure that the simplicity of workbox-sw is still encapsulated but doesn't block use cases of using the modules outside of event listeners
workbox-routing (the bug you link to) highlighted a change that resulted in a descrepency - i.e. routing had the magic method, precaching didn't
I'm happy to add helpers anywhere you would like

#721

What do people think is best?

cc @philipwalton @addyosmani @jeffposnick

[jeffposnick]

I've been starting to migrate the template used by generateSW() over to v3, which has forced me to approach the API as more of a consumer.

It feels pretty good to have, e.g., workbox.router always set up the fetch event listener for me by virtue of that being in the default export, and then just being able to call the methods on that Router instance to add routes, etc. I think we can keep that model as long as it doesn't mess up anything for the developers who end up bundling their own instances of workbox-sw + supporting modules.

I'm open to re-adding optional helper methods to the Router class and whatever other classes are relevant to explicitly register the event listeners. This would benefit developers who are using those classes directly (i.e. not the default exports via workbox-sw) but aren't comfortable adding in their own event listeners. That's a pretty niche group, so we probably wouldn't steer folks towards that path, and I don't feel strongly about it.

[philipwalton]

There are a few issues going on here, so let me try to summarize how I see things to make sure we're all on the same page:

The concern of mine that originally brought up this discussion was not about event listeners, it was that a method was removed without any public issue tracking it. But @jeffposnick filed #914, so that's no longer a concern.

On the subject of how to handle adding event listeners, I think that's actually two separate (and orthogonal) issues.

1. Should workbox classes add event listeners in their constructors that call methods in response, or should we require users to explicitly add those listeners themselves (or call connivence methods, e.g. addFetchHandler())?

2. Should workbox modules add event listeners at import time?

Re: (1) I think we should handle it on a case-by-case basis since not all event listeners are conceptually the same and not all classes offer the same types of functionality. To generically say that classes should never add event listeners seems a bit heavy-handed to me.

In general though, I think we should err on the side of giving developers more control rather than giving them more magic.

Re: (2) I'm almost never in favor of modules having any side effect just from being imported, especially not the default export of the package (since you get that no matter what).

Having side-effects at import time has two big downsides form my perspective:

• Users that only want one part of a package get the side-effects no matter what.
• The fact that a module has side-effects, prevents it from being tree-shaken (as I described in Self-executing code breaks tree shaking #909).

For example, if a user only wants workbox only for workbox-google-analytics they'll get the default fetch event listener added from workbox.routing even though they're not using it. And if we add a default export to workbox.strategies, they'll get that too.

[gauntface]

I think we are happy with this and see if developers request helpers.