Workbox by calebeby · Pull Request #162 · cloudfour/pwastats

calebeby · 2018-07-11T22:14:35Z

Replaces service worker plugins with workbox

Closes #161

gerardo-rodriguez

Woohoo! Nice job @calebeby! 👏

I've got some feedback but this is a great start! 😃

gerardo-rodriguez · 2018-07-12T22:07:58Z

+// Cache CSS and JS files
+workbox.routing.registerRoute(
+  /\.(?:js|css)$/,
+  workbox.strategies.staleWhileRevalidate({


For static assets, we should use a cacheFirst() strategy instead. 😉

Can you explain why? My reasoning for using staleWhileRevalidate is that then it would update the file in the background (for example if the css file is updated)

@calebeby You've convinced me. I did some more digging and it looks like using staleWhileRevalidate is a common way to handle CSS & JS that isn't precached. Thanks for double-checking! 😉

gerardo-rodriguez · 2018-07-12T22:12:01Z

- * Precache resources
- */
-staticCache.addToCacheList({
-  unrevisionedFiles: ['/', '/404/', '/error/']


It looks like the service worker was precaching resources before. Let's make sure the new service worker does the same. :)

Here are some docs about precaching files: https://developers.google.com/web/tools/workbox/guides/precache-files/

You can see the difference in action by going offline via the DevTools and trying to hit the 404 page:

gerardo-rodriguez · 2018-07-12T22:15:26Z

+    plugins: [
+      new workbox.expiration.Plugin({
+        maxEntries: 60,
+        maxAgeSeconds: 30 * 24 * 60 * 60 // 30 Days


Curious how you landed at a max entry amount of 60 and a max age of 30 days? It feels like a guessing game without something to lean on but was wondering if you had referenced a best practice you found. :)

Wondering if @grigs has any thoughts on this for the images cache? :)

I used the example from https://developers.google.com/web/tools/workbox/guides/common-recipes. I think that 30 days is reasonable, but maybe we could remove the maxEntries?

Sounds good. Keeping what you have works for me. 😉

gerardo-rodriguez · 2018-07-12T22:18:25Z

+        maxEntries: 30
+      }),
+      new workbox.cacheableResponse.Plugin({
+        statuses: [0, 200]


I'd rather we try not to cache opaque responses (the 0 status). Is there a way to opt-in to CORS mode for the font requests?

Not sure if this helps but just in case: https://developers.google.com/web/tools/workbox/guides/handle-third-party-requests

Oh, nice! I just checked and the fonts are already opting into CORS mode via the crossorigin attribute. 😉

<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Source+Sans+Pro:400,600,700" crossorigin>

You should be fine by just removing the 0 status from the array. 👍

gerardo-rodriguez · 2018-07-12T22:23:51Z

    <script>
      if ('serviceWorker' in navigator) {
-        navigator.serviceWorker.register('{{ site.url }}/sw.js');
+        navigator.serviceWorker.register('/sw.js');


Can we also add a catch() just in case the service worker fails to register? 😬

Something like:

navigator.serviceWorker .register() .catch(error => { console.error('Service worker registration failed:', error); });

gerardo-rodriguez · 2018-07-12T22:27:25Z

@@ -6,7 +6,7 @@
    {% include svg-icons.html %}
    <script>
      if ('serviceWorker' in navigator) {


Since we are making changes, can me add a friendly console warning message if service workers are not supported?

Something like:

if ('serviceWorker' in navigator) { // ... } else { console.warn('Service workers are not supported by this browser. :('); }

gerardo-rodriguez · 2018-07-12T23:07:09Z


-// Make this always match package.json version
-const version = '0.1.3';
-const cacheName = `defaultCache_${version}`;


Oh, one more thing I noticed! I ran the site locally, and the defaultCache_* cache is sticking around. 🙈

We'll want to purge the out-of-date caches as well. Workbox does not handle this for us. Instead, it is our responsibility to remove out-of-date caches.

In another project, I set up a two-level version cache naming system. Here are some snippets in case it helps (the module.exports may or may not apply), feel free to steal anything that makes sense:

// sw/caches.js /** * This sets up a two-level version cache naming system. * Edit the GLOBAL_VERSION to purge all caches. * Edit the individual cache version to purge the specific cache. */ const GLOBAL_VERSION = 1; const CACHES = { PRECACHE: `pwastats-precache-v${GLOBAL_VERSION + 0}`, FONTS: `pwastats-fonts-v${GLOBAL_VERSION + 1}`, STATIC_ASSETS: `pwastats-static-assets-v${GLOBAL_VERSION + 0}`, IMAGES: `pwastats-images-v${GLOBAL_VERSION + 0}` }; /** * This allows the CACHES to be available both in the * workbox.config.js as well as within the service worker. */ try { module.exports = CACHES; } catch (e) { // Do nothing // For when CACHES is used within service worker scope. }

// sw/sw-delete-caches.js importScripts('/sw/caches.js'); const validCacheList = Object.values(CACHES); /** * Checks the cache name in question against the valid list of caches * @param {String} cacheName The name of the cache in question * @returns {Boolean} Is the `cacheName` valid? */ const cacheIsValid = cacheName => validCacheList.some(validCacheName => cacheName.includes(validCacheName) ); /** * Delete out-of-date caches */ self.addEventListener('activate', event => { event.waitUntil( caches.keys().then(cacheNameList => Promise.all( cacheNameList.map(cacheName => { if (!cacheIsValid(cacheName)) { console.info(`SW :: Deleting cache "${cacheName}"`); return caches.delete(cacheName); } }) )) ); });

// sw.js importScripts("https://storage.googleapis.com/workbox-cdn/releases/3.2.0/workbox-sw.js"); importScripts("/sw/sw-delete-caches.js");

gerardo-rodriguez

Thanks for making all of those udpates @calebeby! 😄

A few more inline comments. Also, can you look into skipWaiting()? I think we want to set it up so that the service worker can update and control the web page immediately.

https://developers.google.com/web/tools/workbox/modules/workbox-sw#skip_waiting_and_clients_claim

gerardo-rodriguez · 2018-07-16T23:14:59Z

-router.registerRoutes({
-  routes: [assetRoute, cdnAssetRoute, navRoute]
-});
+workbox.precaching.precache(['/', '/404/', '/error/']);


@calebeby Hmm...I'm getting a Workbox warning. 🤔

The following precache entries might not be revisioned:

"/"
- "/404/"
- "/error/"

You can learn more about why this might be a problem here: https://developers.google.com/web/tools/workbox/modules/workbox-precaching

Maybe you can look into using the Workbox CLI (maybe as an npm task?) to automate this for us?

Should I do that in this PR or another?

@calebeby I think it should be a part of this PR.

That was very overly complicated, but now it works ✔️

As I understand it, this is the behavior we want for html files (which I implemented):

prefer network fall back to cache for that page (whether it is precached or user visited it before) fall back to displaying precached 404.html page

Workbox really doesn't want to do this (especially the 404 fallback part), so I had to implement a custom handler ☹️

Perhaps late but I found this. I'll share it still in case it helps: https://stackoverflow.com/questions/50762234/configure-workbox-to-use-cached-response-when-network-response-is-a-404

As I understand it, this is the behavior we want for html files (which I implemented):

This makes sense to me.

That does not work with precaching (workbox has a whole separate cache for precaching that works differently, and the paths will not match)

Also, if you have a network connection, and the server responds with a 404, you can just respond with the 404 page that came from the server, instead of doing caching things with that

gerardo-rodriguez · 2018-07-16T23:25:25Z

-  handler: new runtimeCaching.NetworkOnly()
-});
+const isCacheValid = cacheName =>
+  Object.values(CACHES).includes(cacheName) || cacheName.startsWith('workbox-');


@calebeby Sorry, I misled you. 🙈 Workbox does handle any caches it creates (the ones that start with workbox-). We have to handle any caches we create on our own (e.g. css-js, google-fonts, images).

It should be fine w/o the || cacheName.startsWith('workbox-') here. 😬

I wanted to have a whitelist of caches that would be allowed, the rest would be deleted. I allowed the specific ones that we had named, and any starting with workbox-. Any other cache was deleted

If I didn't whitelist caches starting with workbox-, all the workbox caches would be deleted

Got it, cool. Thanks for explaining! 👍

gerardo-rodriguez · 2018-07-16T23:27:28Z

-router.setDefaultHandler({
-  handler: new runtimeCaching.NetworkOnly()
-});
+const isCacheValid = cacheName =>


Thanks for renaming this method name. 👍😉

gerardo-rodriguez · 2018-07-16T23:38:08Z

    <script>
      if ('serviceWorker' in navigator) {
-        navigator.serviceWorker.register('/sw.js');
+        navigator.serviceWorker.register('/sw.js').catch(e => console.error("Problem installing service worker:", e));


Nit: Single quote for consistency? :)

gerardo-rodriguez · 2018-07-16T23:44:09Z

@calebeby One more thing! I was seeing this in the DevTools console when offline. I'm assuming this is because we are using staleWhileRevalidate, therefore, I don't think it's an issue. But wanted to call it out in case you saw any different. 😉

[workbox] Network request for '/main.css' threw an error. TypeError: Failed to fetch

calebeby · 2018-07-17T17:07:24Z

Yep, I don't think it's an issue if it tries to request a file from the network while offline

calebeby · 2018-07-19T16:57:29Z

@gerardo-rodriguez I think I addressed all of your feedback, when you get a chance can you look at the changes?

calebeby added 2 commits July 11, 2018 15:12

Replace sw libs with workbox

18c6474

Remove unused npm scripts

2cf3853

calebeby requested a review from gerardo-rodriguez July 11, 2018 22:14

Change sw url in html template

c3ddf42

calebeby changed the title ~~Feature/workbox~~ Workbox Jul 11, 2018

gerardo-rodriguez requested changes Jul 12, 2018

View reviewed changes

gerardo-rodriguez reviewed Jul 12, 2018

View reviewed changes

calebeby added 5 commits July 12, 2018 16:09

Log errors for service worker installation problems

5e45aa7

Merge remote-tracking branch 'origin/master' into feature/workbox

ed0deec

Do not cache opaque responses

60fa5ee

Fallback to 404 page for html files

1faf350

Delete unrecognized caches

a9b1d82

gerardo-rodriguez requested changes Jul 16, 2018

View reviewed changes

calebeby added 2 commits July 17, 2018 10:06

Single quote, semicolon in blank.html file

5ec46cc

Automatically start workbox/replace old service workers

b6bf3b2

calebeby added 4 commits July 17, 2018 14:23

Add precaching to service worker, with 404 fallback

bd0b3a4

Merge remote-tracking branch 'origin/master' into feature/workbox

cc0fbbc

Empty commit to trigger netlify build again

7df3c6d

Fix: copy revisioned service worker to dist

46e535e

Conversation

calebeby commented Jul 11, 2018

Uh oh!

gerardo-rodriguez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerardo-rodriguez Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerardo-rodriguez Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerardo-rodriguez Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerardo-rodriguez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

calebeby Jul 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gerardo-rodriguez commented Jul 16, 2018

Uh oh!

calebeby commented Jul 17, 2018

Uh oh!

calebeby commented Jul 19, 2018

gerardo-rodriguez Jul 12, 2018 •

edited

Loading

gerardo-rodriguez Jul 12, 2018 •

edited

Loading

gerardo-rodriguez Jul 12, 2018 •

edited

Loading

calebeby Jul 17, 2018 •

edited

Loading