Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

Proposal: Pass "event" to strategies #196

Open
samertm opened this issue Oct 5, 2016 · 3 comments
Open

Proposal: Pass "event" to strategies #196

samertm opened this issue Oct 5, 2016 · 3 comments

Comments

@samertm
Copy link
Contributor

samertm commented Oct 5, 2016

Currently, handlers/strategies have access to the following information: request, values, and options

I'm extending sw-toolbox by writing my own strategies, and there are some things I can't do because I don't have access to the event itself. E.g., I want to be able to call waitUntil and use the clientId in my strategy.

Express passes "request" and "response" objects to every route handler, which makes that framework very flexible. I propose we do something similar with sw-toolbox handlers:

  1. Pass "event" as the first argument to handlers.
  2. Handlers are responsible for calling "event.respondWith" synchronously if they want to respond to an event.

The modifications needed are minimal and arguably reduce the "magic" of the framework. This is what cacheOnly would look like:

function cacheOnly(event, values, options) {
  var request = event.request;
  helpers.debug('Strategy: cache only [' + request.url + ']', options);
  event.respondWith(helpers.openCache(options).then(function(cache) {
    return cache.match(request);
  }));
}

And networkOnly could look like this:

function networkOnly(event, values, options) {
  helpers.debug('Strategy: network only [' + event.request.url + ']', options);
  // We don't call event.respondWith, so the browser simply makes a request.
  // (Equivalent to: `event.respondWith(fetch(event.request))`). 
}

Thoughts?

@jeffposnick
Copy link
Contributor

This is generally a good idea, and is necessary for some specific use cases. (Though I'm not sold on handlers taking responsibility for calling event.respondWith().)

Exposing the FetchEvent is part of my proposed interface for runtime handlers in the "future" sw-toolbox-esque library, detailed at GoogleChrome/workbox#44 (comment)

sw-toolbox has a solid user base and isn't deprecated by any means, but I feel like some warning about making larger changes to the current sw-toolbox codebase is in order. We're actively thinking about these use cases and removing "magic" as part of that next-generation set of service worker libraries.

CC: @addyosmani @gauntface for their thoughts.

@samertm
Copy link
Contributor Author

samertm commented Oct 5, 2016

(Though I'm not sold on handlers taking responsibility for calling event.respondWith().)

My thought there was that it brings sw-toolbox closer to the express framework:

app.get('/', function(req, res) {
  res.send("some response");
});

The proposal still works without it, but I think it's a familiar pattern for web devs.

@samertm
Copy link
Contributor Author

samertm commented Oct 5, 2016

Re: making a large change like this, we could bump the major version number and add a note in the documentation about the change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants