Offline support with service worker by xtuc · Pull Request #978 · babel/website

xtuc · 2016-11-15T21:53:56Z

First naive implementation to support offline mode.

Refers to issue #647

Some points:

Search is broken (search will work only for already searched items because Algolia doesn't support offline mode)
Handle trailing slashes in URLs (cache first strategy won't get invalid URL)
Remove worker development logs
Add offline indicator ?
If the ressource is not in the cache, show an offline page ?

Note that it is necessary to know the files to be cached in advance. That's why I need Jekyll templating in service-worker.js.

@thejameskyle @marcobiedermann What do you think ?

boopathi · 2016-11-19T21:24:30Z

Why not use sw-toolbox ? ... which provides a declarative way of handling routes

boopathi · 2016-11-21T17:14:54Z

+var swOptions = {
+  debug: true
+}
+


It'll be good to use a versioned cache name.

const VERSION = 1; toolbox.cache.name = "Babel-Cache-" + VERSION;

Also,

toolbox.cache.maxEntries = 20 // or 15

Yes, I will add cache versioning.

What do you think about a max age ? Like 7 days ?

I don't know if limiting entries is a good idea. I pre-cache already all pages.

TL for sw-toolbox and sw-precache here. Great to see you using it :)

If you're going to set maxEntries to 20, it means that after the 21st entry is cached, the least-recently used entry would be automatically deleted. Unless you're storing a number of really large assets, I'd consider whether this is really that valuable. I don't think the Babel site would benefit from setting this all that much.

I do think there's use in imposing a maxAgeSeconds age. @boopathi can probably comment to the length of time, but a week doesn't sound like a bad ballpark.

boopathi · 2016-11-21T17:38:00Z

+
+toolbox.precache(preCachedRessources);
+
+toolbox.router.get('/(.*)', toolbox.cacheFirst, swOptions);


I see requests to these domains as well,

toolbox.router.get("/*", toolbox.cacheFirst, { origin: "cdnjs.cloudflare.com" }); toolbox.router.get("/*", toolbox.cacheFirst, { origin: "cdn.jsdelivr.net" }); toolbox.router.get("/*", toolbox.cacheFirst, { origin: "unpkg.com" }); // for repl

Ok, I will add these origins.

I'm currently searching a way to respond to Algolia queries. It seems to not support offline mode. For pre-caching I already use a list of all pages, it should be easy to search in it.

Algolia queries are POST queries - toolbox.router.post(...)

Depending on how you want to handle post requests, you could do toolbox.router.post('/endpoint', toolbox.networkOnly). This means you're not just returning a cached response but are instead stating that requests made to endpoint need to be network only.

We could do networkOnly, but @xtuc wanted that the user be able to search even when offline - as mostly it is just static data updated via commits. So, it could be networkFirst with a separate cache name and maxEntries.

boopathi · 2016-11-21T17:43:22Z

+  '{{ "/css/main.css" | prepend: site.baseurl }}?t={{ site.time | date_to_xmlschema }}"',
+  {% for page in site.pages %}
+  '{{ page.url }}',
+  {% endfor %}


I'm not sure if the following is valid. I see this to be valid at-least for page transitions from and to REPL.

The Service Worker shouldn't change from pageA to pageB if both the pages are under the scope of the same service worker. This is because, service worker updates based on the byte diff, and during a page change, a new service worker will install (if the resources are different -> i.e new service-worker.js).

Since the strategy is cacheFirst I removed the file main.css which has a time in it.

xtuc · 2016-11-21T21:43:59Z

The offline indicator looks like this :

Might not be the most beautiful way to do.

xtuc · 2016-11-23T21:07:23Z

Or the grey version:

xtuc · 2016-11-23T21:19:38Z

Label version:

existentialism · 2016-11-23T21:33:37Z

Nice work!

I feel like the notification should stand out enough to draw attention, but not be distracting. Maybe black (fits color scheme and contrasts with the yellow header)?

boopathi · 2016-11-23T21:33:40Z

I tried a variant of grey version. I like the label version. My opinion is that removing/changing yellow might give the feeling of offline.

addyosmani · 2016-11-23T21:59:15Z

This is all really awesome to see come together. Fwiw, we're seeing a few sites choose the grayed out pattern for indicating offline (coupled with a top or bottom bar with the 'offline' labelling):

Some alternatives we've suggested elsewhere include showing a toast when switching online state or just having something prominent in the UI that can be toggled depending on when the user has a network connection again.

addyosmani · 2016-11-23T22:09:01Z

+    window.addEventListener('offline',  function() {
+      offlineIndicator.removeClass("hidden");
+    });
+


Mostly leaving here as a note in case helpful later :)

The navigator.{online/offline} events may not accurately indicate that you can or can't access the network. There are well documented gotchas where you might need additional means to check that you're really online. One is checking connection loss by making failed XHR requests (retry a request a few times, if it doesn't go through, you're definitely offline).

@addyosmani flipkart is awesome !

ikhattab · 2016-11-23T22:15:19Z

using glyphicon glyphicon-off and background color of #f55555 and we can fade color to green and fade out the offline div when the user is online

marcobiedermann · 2016-11-23T22:20:37Z

@xtuc Awesome, great work you did there. I quite like that there would be a little indicator on the top which shows if the user is offline.
Looking forward getting this into production 😄 🎉

addyosmani · 2016-11-23T22:29:48Z

+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdnjs.cloudflare.com" });
+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdn.jsdelivr.net" });
+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "unpkg.com" }); // for repl
+toolbox.router.post('/*', toolbox.cacheFirst, { origin: "algolia.net" }); // Cache Algolia search response


I mentioned this in your earlier commits, but is the goal here to always return a cached post response from Algolia or to only have this work when the network is available?

I wanted to keep search even once offline. The goal here is to cache some responses from Algolia because it does not support offline searching. I'm not sure about this since the user will be able to find what he already searched online.

@addyosmani I guess not. it should be networkFirst.

@xtuc cacheFirst, networkFirst, fastest - all these ultimately hit the cache in some way.

mbj36 · 2016-11-23T22:49:42Z

'offline fallback' for the last point would do the same. We have to make a offline page for it. We have to add service worker 'fetch' event in index.js .

Any idea what should offline page looks like ?

jeffposnick · 2016-11-23T23:11:11Z

Apologies for the drive-by feedback. I work on sw-toolbox/sw-precache, and I've spent some time thinking about best practices for using service workers with templated sites, so I wanted to throw in my two cents.

I'll leave some inline feedback on this PR, and I've started a series of posts laying out some best practices for this scenario, with part 1 at https://jeffy.info/2016/11/02/offline-first-for-your-templated-site-part-1.html

jeffposnick · 2016-11-23T23:13:00Z

+
+importScripts('/scripts/sw-toolbox.js');
+
+const VERSION = 1;


How will you make sure that VERSION is incremented whenever any existing content anywhere on the site is updated? If VERSION doesn't get incremented, then the previously cached content for a given URL will never get refreshed, since there's a cacheFirst policy being used.

Yes sure. We could use the site.date from Jekyll. But this way the entier cache will be invalided even if the content hasn't changed.

jeffposnick · 2016-11-23T23:18:26Z

+toolbox.precache(preCachedRessources);
+
+toolbox.router.get('/*', toolbox.cacheFirst);
+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdnjs.cloudflare.com" });


You'd probably want to configure a different cache name here the runtime CDN caches, so that they're reused indefinitely and don't become irrelevant whenever you change the default toolbox.cache.name value above.

If you're using cacheFirst strategy for them, please make sure that all of the URLs that they refer to are versioned. E.g. https://unpkg.com/react@15.3.1/dist/react.min.js includes 15.3.1 in the URL, so it's fine to use cacheFirst. But if you request URLs like https://unpkg.com/react/dist/react.min.js, once it's cached for the first time, the initial version will be reused indefinitely (at least until the cache name changes).

Yes good point. This is part of good practice I guess.

I checked, resources from CDN (Cloudflare and jsdelivr) have a version in the URL.

jeffposnick · 2016-11-23T23:19:44Z

+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdnjs.cloudflare.com" });
+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdn.jsdelivr.net" });
+toolbox.router.get('/*', toolbox.cacheFirst, { origin: "unpkg.com" }); // for repl
+toolbox.router.post('/*', toolbox.cacheFirst, { origin: "algolia.net" }); // Cache Algolia search response


I don't know exactly what algolia.net is used for specifically, but if it's search-related, then cacheFirst doesn't sound like a good strategy to use. networkFirst would normally be more appropriate, since you'd want fresh search results, but could fall back to stale results if the network is unavailable.

algolia.net is for the search functionality

Yes, I'm now thinking that idea wasn't that good. (#978 (comment))

jeffposnick · 2016-11-23T23:21:44Z

+
+toolbox.precache(preCachedRessources);
+
+toolbox.router.get('/*', toolbox.cacheFirst);


This is super broad. Are you sure you're okay with every URL under your origin being served cacheFirst? And aside from the caching strategy, are you sure that any time there's any request made for a resource on your site, you want that added to the cache? If there are large images or other large media, then that's not necessarily a good approach.

I wanted an offline documentation. This caching strategy might indeed not be the best approach.

jeffposnick · 2016-11-23T23:23:01Z

+
+const VERSION = 1;
+
+toolbox.cache.name = "Babel-Cache-" + VERSION;


I don't see anything here that will delete old caches once this cache name changes. That means that you'll end up with many copies of your full site in the SW site, each time you change the name.

Normally, you'd want to clean up unneeded caches inside an activate handler.

👍 to Jeff's comment. You'll want to iterate through your cache entries and cache.delete doing something along these lines:

// Where urlsToCacheKeys is a map of your items to cache self.addEventListener('activate', function(event) { let setOfExpectedUrls = new Set(urlsToCacheKeys.values()); event.waitUntil( caches.open(cacheName).then(function(cache) { return cache.keys().then(function(existingRequests) { return Promise.all( existingRequests.map(function(existingRequest) { if (!setOfExpectedUrls.has(existingRequest.url)) { return cache.delete(existingRequest); } }) ); }); }).then(function() { return self.clients.claim(); }) ); });

Rather than cleaning manually the cache, I think we need to limit the cache using cache.maxEntries or cache.maxAgeSeconds.
The old cached website(s) will be cleaned up given these configurations.

jeffposnick · 2016-11-23T23:30:18Z

+
+var preCachedRessources = [
+  {% for page in site.pages %}
+  '{{ page.url }}',


If you're precaching entire HTML documents, then you're going to end up populating user's caches with a lot of duplicated data. I.e. if your HTML shares common headers/footers (because they're all using some underlying layouts) then the structure of those headers and footers will be take up space on your user's disk when they don't actually add any extra value.

The ideal implementation would just cache the layout elements once, then cache the underlying content once, and then perform the templating logic in the service worker to combine them at runtime.

I've got a very experimental example of this implemented at https://github.com/jeffposnick/jeffposnick.github.io/tree/work/src, but I don't know that it's stable enough to recommend using for a high-traffic site.

boopathi · 2016-11-26T11:14:14Z

-toolbox.router.get('/*', toolbox.cacheFirst);
-toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdnjs.cloudflare.com" });
-toolbox.router.get('/*', toolbox.cacheFirst, { origin: "cdn.jsdelivr.net" });
+toolbox.router.get('/*', toolbox.cacheFirst, contentCacheOptions);


networkFirst or fastest - cacheFirst is never going to hit the network after first time.

xtuc · 2016-11-26T12:47:10Z

An other try :

mathiasbynens · 2016-11-29T13:13:01Z

+    var offlineIndicator = $("#offline-indicator");
+
+    window.addEventListener('online',  function() {
+      offlineIndicator.addClass("hidden");


nit: quotes are inconsistent (single quotes are used elsewhere)

Yes, thanks. I updated both files with single quotes.

mathiasbynens · 2016-11-29T13:13:20Z

+var VERSION = '{{ site.time }}';
+
+var contentCacheOptions = {
+  name: "Babel-Cache-" + VERSION,


same in this file

xtuc · 2016-12-01T17:29:12Z

@boopathi @marcobiedermann @mathiasbynens @jeffposnick what are you thought about this ?

jeffposnick · 2016-12-01T18:28:21Z

The current implementation, with toolbox.fastest for (most of) the requests means that you'll have a much better upgrade story, and you'll avoid the issues I've previously pointed out where content would be cached once and potentially reused indefinitely. I don't see any specific shooting-yourself-in-the-foot issues.

What I'd recommend is that you try doing some sanity checks in a local deployment by keeping an eye on Chrome DevTool's Cache Storage display in the Applications panel, and the Network panel.

The Cache Storage display will give you a list of URLs that are being cached, and you can verify that you're not accidentally caching anything that you don't intend to. (Your /* route is very broad, and it's possible that there are requests that the site is making which you realize don't need to be cached.)

xtuc · 2016-12-03T15:30:49Z

Thanks a lot @jeffposnick for your explanations.

LGTM, @hzoo @boopathi can we merge it ?

stale · 2017-06-12T19:56:43Z

This issue has been automatically marked as stale because it has not had recent activity 😴. It will be closed if no further activity occurs. Thank you for your contributions 👌!

babel-bot · 2017-11-04T09:57:24Z

Deploy preview ready!

Built with commit 18d2580

https://deploy-preview-978--babel.netlify.com

0xnmn · 2017-11-04T13:54:34Z

@jeffposnick @xtuc Hello I am new here and trying to contribute.

I was thinking of if we can have different named caches for different kind of assets. Like for images we can have a separate cache, so that we can manage it differently and even it may improve the performance while matching requests from the cache.

Also I think it would be safer to go with networkFirst for the website assets, with cache maxAge.

And do we need maxEntries? Can't we let it be maximum as they will get expired by age and so the the caching of assets won't be limited.

xtuc

We can remove the scripts/sw-toolbox.js in the sources now.

xtuc · 2017-11-13T10:32:51Z

+  , cdnCacheOptions = {
+  name: 'cdn',
+  maxAgeSeconds: 60 * 60 * 6
+};


Could you please make multiple var declarations instead here?

feat: support offline with service worker

85a014b

xtuc changed the title ~~Offiline support with service worker~~ Offline support with service worker Nov 19, 2016

refactor(offline): use sw-toolbox

1bd6f2d

boopathi reviewed Nov 21, 2016

View reviewed changes

xtuc added 4 commits November 21, 2016 21:31

fix(sw): removed pre-cached CSS

ef6d78b

feat(sw): add external cache origins

2d6207d

feat(sw): add cache version

2936fad

feat(sw): offline indicator

53458ba

Merge remote-tracking branch 'upstream/master'

49ca43f

addyosmani reviewed Nov 23, 2016

View reviewed changes

jeffposnick reviewed Nov 23, 2016

View reviewed changes

boopathi reviewed Nov 26, 2016

View reviewed changes

fix(sw): update cache strategies

c79524c

xtuc added 3 commits November 26, 2016 13:47

feat(sw): improved offline indicator

1e75370

fix(sw): remove site precaching

ae14615

Merge remote-tracking branch 'upstream/master'

5ce2a31

xtuc added the enhancement label Nov 28, 2016

mathiasbynens requested changes Nov 29, 2016

View reviewed changes

lint: use single quotes

ac3f4dc

Update service-worker.js

25e916d

xtuc requested review from existentialism and hzoo December 8, 2016 11:51

xtuc requested a review from raspo December 17, 2016 16:58

stale Bot added the stale label Jun 12, 2017

stale Bot closed this Jun 20, 2017

hzoo reopened this Jun 20, 2017

stale Bot removed the stale label Jun 20, 2017

Resolved conflicts

c8f206f

0xnmn added 3 commits November 5, 2017 18:08

Imported sw-toolbox from unpkg

d32e376

Added Offline.md page

687842e

Made Cache Pattern Network First

7c90808

xtuc commented Nov 13, 2017

View reviewed changes

0xnmn added 2 commits November 13, 2017 17:17

Made multiple var declarations

6d557eb

Removed sw-toolbox.js

18d2580


		toolbox.precache(preCachedRessources);

		toolbox.router.get('/(.*)', toolbox.cacheFirst, swOptions);


		toolbox.precache(preCachedRessources);

		toolbox.router.get('/*', toolbox.cacheFirst);


		const VERSION = 1;

		toolbox.cache.name = "Babel-Cache-" + VERSION;

Uh oh!

Conversation

xtuc commented Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boopathi commented Nov 19, 2016

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!

xtuc Nov 21, 2016 • 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!

xtuc commented Nov 21, 2016

Uh oh!

xtuc commented Nov 23, 2016

Uh oh!

xtuc commented Nov 23, 2016

Uh oh!

existentialism commented Nov 23, 2016

Uh oh!

boopathi commented Nov 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addyosmani commented Nov 23, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikhattab commented Nov 23, 2016

Uh oh!

marcobiedermann commented Nov 23, 2016

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!

mbj36 commented Nov 23, 2016

Uh oh!

jeffposnick commented Nov 23, 2016

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!

xtuc commented Nov 15, 2016 •

edited

Loading

xtuc Nov 21, 2016 •

edited

Loading

boopathi commented Nov 23, 2016 •

edited

Loading

xtuc commented Nov 26, 2016 •

edited

Loading

babel-bot commented Nov 4, 2017 •

edited

Loading