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

sw-toolbox Integration #74

Merged
merged 4 commits into from
Feb 8, 2016
Merged

sw-toolbox Integration #74

merged 4 commits into from
Feb 8, 2016

Conversation

jeffposnick
Copy link
Contributor

@wibblymat @gauntface @petele @addyosmani

Here's my first cut of what an integration between sw-precache and sw-toolbox could look like. As part of your sw-precache config in your build process, you can now include a runtimeCaching array that includes one or more entries. Each entry will result in a sw-toolbox-style handler in the generated service worker file. For example, including this in the config:

runtimeCaching: [{
  urlPattern: /runtime-caching/,
  handler: 'cacheFirst',
  options: {
    cache: {
      maxEntries: 1,
      name: 'runtime-cache'
    }
  }
}]

Results in this being added to the end of the generated service worker file:

toolbox.router.get(/runtime-caching/, toolbox.cacheFirst, {"cache":{"maxEntries":1,"name":"runtime-cache"}});

There are some existing rough patches that make this not suitable for merging yet, but I wanted to get some concrete feedback at this stage.

A few random bits:

  • The entire sw-toolbox.js contents are included inline in the generated service worker file. By inlining the contents as opposed to pulling them in with importScripts(), I'm hoping to avoid some of the messier bits that we ran into with <platinum-sw>, where it took some effort to get the dependent files served from the correct URL. The downside is that the generated service worker file is now ~25kb (~8kb gzip-ed).
  • I don't have updates to the docs yet, pending any initial feedback about the approach and new configuration option.
  • We currently need to pull in the master branch of sw-toolbox, since the changes to support RegExp routing isn't in a tagged release yet.

@gauntface
Copy link

My first thought is that if sw-precache could read in, manipulate and spit
out a sw file, then the sw-toolbox code would be just as easy to write as
the gulp config.

If that were the case it would make sense for developers to learn about
sw-toolbox and add it go the original sw file up front rather than learn
how to use sw-precache and then learn about sw-toolbox if they need to.

But this has been my view from the first time I played with sw-precache.

On Wed, 6 Jan 2016, 19:49 Jeffrey Posnick [email protected] wrote:

@wibblymat https://github.com/wibblymat @gauntface
https://github.com/gauntface @petele https://github.com/petele
@addyosmani https://github.com/addyosmani

Here's my first cut of what an integration between sw-precache and
sw-toolbox could look like. As part of your sw-precache config in your
build process, you can now include a runtimeCaching array that includes
one or more entries. Each entry will result in a sw-toolbox-style handler
in the generated service worker file. For example, including this in the
config:

runtimeCaching: [{
urlPattern: /runtime-caching/,
handler: 'cacheFirst',
options: {
cache: {
maxEntries: 1,
name: 'runtime-cache'
}
}
}]

Results in this being added to the end of the generated service worker
file:

toolbox.router.get(/runtime-caching/, toolbox.cacheFirst, {"cache":{"maxEntries":1,"name":"runtime-cache"}});

There are some existing rough patches that make this not suitable for
merging yet, but I wanted to get some concrete feedback at this stage.

A few random bits:

The entire sw-toolbox.js
https://github.com/GoogleChrome/sw-toolbox/blob/master/sw-toolbox.js
contents are included inline in the generated service worker file. By
inlining the contents as opposed to pulling them in with
importScripts(), I'm hoping to avoid some of the messier bits
https://github.com/PolymerElements/platinum-sw#considerations that
we ran into with , where it took some effort to get the
dependent files served from the correct URL. The downside is that the
generated service worker file is now ~25kb (~8kb gzip-ed).

I don't have updates to the docs yet, pending any initial feedback
about the approach and new configuration option.

We currently need to pull in the master branch of sw-toolbox, since
the changes
GoogleChromeLabs/sw-toolbox@f04c671
to support RegExp routing isn't in a tagged release yet.


You can view, comment on, or merge this pull request online at:

#74
Commit Summary

  • Initial cut of sw-toolbox integration

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#74.

@addyosmani
Copy link
Contributor

  • Personally, I'd be fine with the inlined sw-toolbox library given the low gzip side
  • You currently inline the sw-toolbox library after checking runtimeCaching is defined. If the configuration objects specified here are invalid, would it make sense to plumb back exceptions from sw-toolbox to the command-line given sw-precache would be used as part of a build step, or would it be sufficient for the developer to make their own conclusions after seeing toolbox errors in the browser?
  • With the runtimeCaching array supported, would you ever need to go back to separately configuring or interacting with sw-toolbox or would this strictly be done through the new array?

@jeffposnick
Copy link
Contributor Author

@gauntface
I'm not ruling out a mode where sw-precache generates precaching snippets suitable for insertion into an existing service worker template that you provide. But I'm of the opinion that that's not going to be friendliest use case for new developers, and I didn't want to focus on that right now. I think the change I'm making here, to inline the sw-toolbox code into the generated service worker, does pave the way for migrating code from the sw-precache template to the sw-toolbox project, and that's something that we discussed which you were in favor of.

@addyosmani:
I can catch a class of bad configuration errors at build time and spit that out as build errors. There is a class of bad configuration that wouldn't be obviously bad at build time, and for those sorts of things, the runtime errors would have to suffice. I'll update the PR.

I think the majority of common sw-toolbox usage scenarios would be covered by these config options. Although it's a bit awkward/unseemly to include runtime logic in a build config, it is possible to pass in arbitrary request handlers as a function in addition to using the built-in handlers. Using toolbox.precache() directly isn't possible, but I think that would be an anti-pattern anyway, since sw-precache should handle precaching. There might be some valid scenarios that I'm not thinking of, but if those come up, we can adjust the support options as needed.

@jeffposnick
Copy link
Contributor Author

Hey All—I've updated this PR based on feedback, but also added in a new Getting Started guide that I would like to ship along with the sw-toolbox integration. I hope folks don't mind that getting slipped in to what's already a large PR, but I hope it better explains how to use sw-precache and the new sw-toolbox integration.

I'm flagging @jpmedley especially to review the new Getting Started content, which you can also preview at https://github.com/GoogleChrome/sw-precache/blob/sw-toolbox-integration/GettingStarted.md

@gauntface
Copy link

Looks good to me - getting started guide is super useful and concise.

Out of interest, in the navigateFallback sw-toolbox default or is it something in sw-precache?

@jeffposnick
Copy link
Contributor Author

Out of interest, in the navigateFallback sw-toolbox default or is it something in sw-precache?

Are you asking whether navigateFallback comes from sw-toolbox or sw-precache? If so, the answer is sw-precache: https://github.com/GoogleChrome/sw-precache#navigatefallback-string

@jeffposnick
Copy link
Contributor Author

I'll merge this early next week unless there are any objections.

@wibblymat
Copy link

I didn't already LGTM this because I haven't had the chance to thoroughly review the docs part, but I can give you an LGTM on the code part.

A helpful analogy when thinking about your App Shell is to the code and
resources that would be published to an App Store for a native iOS or Android
application.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"A helpful analogy is to think of your App Shell as the code and resources that would be published to an app store for a native iOS or Android application."

Suggested rewrite because I had a little trouble parsing the text as written.

@jpmedley
Copy link
Contributor

jpmedley commented Feb 8, 2016

@jeffposnick

Pete asked me to look it over. Can you address my comments?

@jeffposnick
Copy link
Contributor Author

I went over and addressed @jpmedley's comments in person.

jeffposnick added a commit that referenced this pull request Feb 8, 2016
@jeffposnick jeffposnick merged commit 0f4cc3d into master Feb 8, 2016
@jeffposnick jeffposnick deleted the sw-toolbox-integration branch February 8, 2016 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants