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

Why caching what we decided not to cache in the example js13kpwa? #2957

Open
1 task
Dominic-Mayers opened this issue Mar 12, 2020 · 7 comments
Open
1 task
Assignees
Labels
30 minute task This is a fairly quick issue to fix, estimated time 30 minutes. Content:WebAPI For content triage purposes: This is related to WebAPI content Est:Medium Task estimated as medium (up to a day's work?) P2 Fix next quarter

Comments

@Dominic-Mayers
Copy link

Request type

  • [] Please close this issue, I accidentally submitted it without adding any details
  • New documentation
  • [x ] Correction or update

Details

I am sharing the experience that I had while studying the page https://developer.mozilla.org/en-US/docs/Web/Progressive_web_apps/Offline_Service_workers. In the code, games.js is not in the list of the shell items to cache. I thought, hey I can add an entry server side and see that it will be updated. It did not work ! It took me a few minutes to realize that in the code below when an item is not found in the cache, we cache it. So, we could as well have included games.js in the list. I found this a bit confusing. The example should be more explicit about whether the goal is to cache every thing (including game.js) or not.

@chrisdavidmills chrisdavidmills added Content Content:WebAPI For content triage purposes: This is related to WebAPI content Est:Medium Task estimated as medium (up to a day's work?) P2 Fix next quarter labels Mar 12, 2020
@Dominic-Mayers
Copy link
Author

Dominic-Mayers commented Mar 13, 2020

Still sharing my experience, here is what I did as an exercise to test my understanding. I created a separate asset file for the service worker:

// data/sw_assets.js
var DoNotCacheFiles = [];

var FetchFirstFiles = [
	'/pwa-examples/js13kpwa/data/games.js'
];
 
var InitialCacheFiles = [
  '/pwa-examples/js13kpwa/',
  '/pwa-examples/js13kpwa/index.html',
  '/pwa-examples/js13kpwa/app.js',
  '/pwa-examples/js13kpwa/style.css',
  '/pwa-examples/js13kpwa/fonts/graduate.eot',
  '/pwa-examples/js13kpwa/fonts/graduate.ttf',
  '/pwa-examples/js13kpwa/fonts/graduate.woff',
  '/pwa-examples/js13kpwa/favicon.ico',
  '/pwa-examples/js13kpwa/img/js13kgames.png',
  '/pwa-examples/js13kpwa/img/bg.png',
  '/pwa-examples/js13kpwa/icons/icon-32.png',
  '/pwa-examples/js13kpwa/icons/icon-64.png',
  '/pwa-examples/js13kpwa/icons/icon-96.png',
  '/pwa-examples/js13kpwa/icons/icon-128.png',
  '/pwa-examples/js13kpwa/icons/icon-168.png',
  '/pwa-examples/js13kpwa/icons/icon-192.png',
  '/pwa-examples/js13kpwa/icons/icon-256.png',
  '/pwa-examples/js13kpwa/icons/icon-512.png'
];

Then I modified the service worker as follows:

// sw.js

self.importScripts('data/games.js');
self.importScripts('data/sw_assets.js');

// In normal mode newCacheName = cacheName. 
// Stay in transition mode long enough so that users that use often the app have installed the new cache.   
var cacheName    = 'js13kPWA-v8';
var newCacheName = 'js13kPWA-v8';

// Files to cache. 
var gamesImages = [];
for(var i=0; i<games.length; i++) {
  gamesImages.push('data/img/'+games[i].slug+'.jpg');
}


var contentToCache = InitialCacheFiles.concat(gamesImages);

// Installing Service Worker
self.addEventListener('install', (e) => {
  console.log('[SW] Install ' + newCacheName);
  e.waitUntil(
    caches.open(newCacheName).then((cache) => {
      console.log('[SW] Caching app shell and content.');
      return cache.addAll(contentToCache);
    })
  );
});


// Removing old caches. 
self.addEventListener('activate', (e) => {
  e.waitUntil(
    caches.keys().then((keyList) => {
          return Promise.all(keyList.map((key) => {        
        if(key !== cacheName && key !== newCacheName) {
          console.log('[SW] We delete cache ' + key);
          return caches.delete(key);
        }
      }));
    })
  );
});

// Fetching content using Service Worker
self.addEventListener('fetch', function(e) {
  e.respondWith(
    caches.match(e.request).then(function(r) {
      var p=  new URL(e.request.url).pathname; 
      if (r) {
        if (FetchFirstFiles.includes(p)) {
          return fetch(e.request)
          .then((response) => {
            console.log('[SW] Fetching, even if cached : ' + p);
            return response;})
          .catch( (e) => {  
            console.log('[SW]', e.message, 'Returning not fetchable, but cached : ' + p);
            return r; 
          }); 
        } 
        else {
          console.log('[SW] Returning cached  : ' + p);
          return r; 
        }
      }
      else if (DoNotCacheFiles.includes(p) ) {
        console.log('[SW] Fetching non cachable content : ' + p);
        return fetch(e.request); 
      }
      else {
        console.log('[SW] Fetching and caching content : ' + p);
        return fetch(e.request).then((response) => {
          return caches.open(cacheName).then((cache) => {
            console.log('[SW] Caching new resource: '+p);
            cache.put(e.request, response.clone());
            return response;
          });
        });
      }
    })
  );
});

@Elchi3 Elchi3 removed the Content label May 28, 2020
@chrisdavidmills chrisdavidmills added the 30 minute task This is a fairly quick issue to fix, estimated time 30 minutes. label Oct 27, 2020
@Rumyra Rumyra self-assigned this Nov 11, 2020
@Rumyra
Copy link
Contributor

Rumyra commented Nov 11, 2020

I have thought about this for a while and agree that the tutorial should be more explicit as to whether to cache games.js or rather why not.

As games.js contains the data, in reality that would be called from an API endpoint or database, caching the data would mean updating it periodically when there was network connectivity. For scope of the this tutorial it's best to keep games.js from the cache. It would be good to find out the intentions of the original author, but my understanding would be keeping it out was intentional.

It may be a task in the future to update the tutorial to include periodic background sync https://developer.mozilla.org/en-US/docs/Web/API/Web_Periodic_Background_Synchronization_API which allows for data to be updated in the background.

I can update the tutorial to be more explicit about why games.js isn't cached @chrisdavidmills

@chrisdavidmills
Copy link
Contributor

@Rumyra sounds good to me, and I think your explanation is sound.

@Rumyra
Copy link
Contributor

Rumyra commented Nov 12, 2020

Thanks @chrisdavidmills I've added:

You may notice we haven't cached game.js. This is the file that contains the data we use when displaying our games. In reality this data would most likely come from an API endpoint or database and caching the data would mean updating it periodically when there was network connectivity. We won't go into that here, but the Periodic Background Sync API is good further reading on this topic.

At the bottom of this section https://wiki.developer.mozilla.org/en-US/docs/Web/Progressive_web_apps/Offline_Service_workers#Lifecycle_of_a_Service_Worker

@chrisdavidmills
Copy link
Contributor

Looks great, thanks @Rumyra !

@Dominic-Mayers
Copy link
Author

Dominic-Mayers commented Nov 12, 2020

My point was that in the example provided, the code (see below) will systematically cache game.js at the first request even though it's not in the list to cache. It might be that this is an aspect that the example did not intend to cover and that a real code in practice would not systematically cache game.js. I am not sure the explanation covers this aspect.

self.addEventListener('fetch', (e) => {
  e.respondWith(
    caches.match(e.request).then((r) => {
          console.log('[Service Worker] Fetching resource: '+e.request.url);
      return r || fetch(e.request).then((response) => {
                return caches.open(cacheName).then((cache) => {
          console.log('[Service Worker] Caching new resource: '+e.request.url);
          cache.put(e.request, response.clone());
          return response;
        });
      });
    })
  );
});

@Rumyra
Copy link
Contributor

Rumyra commented Nov 13, 2020

I'm going to pick this up again @chrisdavidmills as I overlooked the initial cache. I might go through the tutorial myself and even out anything I find

@Rumyra Rumyra reopened this Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
30 minute task This is a fairly quick issue to fix, estimated time 30 minutes. Content:WebAPI For content triage purposes: This is related to WebAPI content Est:Medium Task estimated as medium (up to a day's work?) P2 Fix next quarter
Projects
None yet
Development

No branches or pull requests

4 participants