Closed YerkoPalma closed 6 years ago
Mostly because if you get service workers wrong, they can really mess up your app. Also in local dev people reported they struggled with things. Figured disabling it would make most sense until we figure it out.
This is the list I've been meaning to look at:
Most of this comes from this video: https://www.youtube.com/watch?v=CPP9ew4Co0M
Having someone look at this would be amazing! If we can provide a smooth service worker experience that'd be :100:.
A few questions about your points:
- Figure out clients API
- Figure out the Push API
Not sure what those means.
Use async/await everywhere
That means you are planning to use es6 on this module? with Babel transforms included?
The rest of the points are mostly specific to web services, so we should consider
a) Just use a tool to generate service workers (like workbox) b) Generate a lib specific to service worker on Choo, considering simple SW strategies c) Instead of using a third party or own lib, make a smart service worker template.
note: If we go for a lib, the common way to add libs to service worker is through importScript which isn't working in bankai.
Not sure what those means.
Yep, me neither haha. It's been a while since I scribbled this down (on my phone).
That means you are planning to use es6 on this module? with Babel transforms included?
I don't think we need to - service worker support overlaps cleanly with async / await support. On all but UC browser - which has partial support for service worker, soooo we might want to consider that? This means we would only need to minify, which is quite nice.
note: If we go for a lib, the common way to add libs to service worker is through importScript which isn't working in bankai.
I think we should stick with require()
on this for now. I feel having a single, nicely bundled service worker might end up being quite neat. This keeps some complexity out of Bankai too - which for now seems like a reasonable tradeoff.
As for strategies: I was thinking we create a smart-ish template first. And if there's anything in there that's non-trivial, we turn it into modules. I don't have a good sense of what a resulting boilerplate would look like, which makes me hesitant of starting off with modules - does that make sense?
Not sure if this goes in the right direction, but here are a few modifications to the current service worker provided
/* global self */
var VERSION = String(Date.now())
var URLS = [
'/',
'/bundle.css',
'/bundle.js',
'assets/icon.png'
]
// Register worker
self.addEventListener('install', function (e) {
e.waitUntil(self.caches.open(VERSION).then(function (cache) {
return cache.addAll(URLS)
}))
})
// Remove outdated resources
self.addEventListener('activate', function (e) {
e.waitUntil(self.caches.keys().then(function (keyList) {
return Promise.all(keyList.map(function (key, i) {
if (keyList[i] !== VERSION) return self.caches.delete(keyList[i])
}))
}))
})
// Respond with cached resources
self.addEventListener('fetch', function (e) {
e.respondWith(self.caches.match(e.request).then(function (response) {
return response || self.fetch(e.request)
// if some resource didn't match, and you want to add it to cache use
// return response || self.fetch(e.request).then(function (response) {
// cache.put(e.request, response.clone())
// return response
// })
}))
})
// to emit backgroundSync events in choo, with choo-service-worker installed, emit the event
// 'sw:sync' passing a tag
// you can use it to add non-urgent resources to cache or whatever you want
// self.addEventListener('sync', function (event) {
// if (event.tag === 'timeline') {
// vent.waitUntil(
// caches.open(VERSION).then(function(cache) {
// return cache.add('/timeline.json')
// })
// )
// }
// })
Is almost the same, just add a few options in comments to provide examples and stuff. Some things that might improve this.
What do you think?
@YerkoPalma a few things, probably:
process.env.FILE_LIST
. We could do var URLS = process.env.FILE_LIST
or something similar. This will then get auto-updated on runs :sparkles:false
, it disables the entire worker. We can't trust things to always go right, so having a hard killswitch to roll back a deploy can work wonders. Even if it requires a custom backend, and is disabled by default.VERSION
should probably be set to process.env.npm_package_version
. Turns out we don't support this yet in Bankai, but we should (opening up an issue about this right now).Use the package version as the sw version, this allows, for instance, only delete cache on major version, minor and patch only updates cache.
Hmmm, I think it's probably a good idea to always delete the cache, but try and copy over already existing resources over first. This is a large part of why we made hashes a part of the resource's urls, so that the name itself already verifies whether or not something has changed.
// to emit backgroundSync events in choo, with choo-service-worker installed, emit the event 'sw:sync' passing a tag you can use it to add non-urgent resources to cache or whatever you want
I'm not entirely sure what you mean by "pass a tag". Could you explain a bit more about what you mean by this?
Suggest a regex-router like in fetch event for more complex situations. For example, allow through regex to cache everything in assets folder, or even, copy the behaviour of the bankai/http router.
Oh oh oh oh, so yeah the process.env.FILE_LIST
var should do this! :grin:
Add a simple notification suggestion as user feedback on cache updates, install, just to improve UX.
I like this a lot!
Add some event handler to add resource to cache on user interaction, this might need some changes on client.
I'm intrigued by this. Could you explain a bit more about how you'd see this?
I'm liking this a lot. Now that Bankai 9 is stable it's great to be able to talk about this, and realize we can actually make this a great experience! So glad you started digging into this! :D :D
So, I'll answer to most of your comment, but first I think I didn't put much emphasis in this offline guide from Jake Archibald, is pretty much the main resource of almost anything I have to say here haha. So, here we go:
Bankai exposes a few env vars, most importantly process.env.FILE_LIST. We could do var URLS = process.env.FILE_LIST or something similar. This will then get auto-updated on runs :sparkles:
Cool, considering there is different kinds of files in a web app, I think that the file list mentioned is the minimum resources to have your app running offline, but still people could load resources that are not in that list. I would do as you say and map that list to the initial cache files, and maybe comment out or point to On install - not as a dependency strategy from the guide I linked, to guide people that want to load resources without changing that environment var of the file lists. This would look like this
cache.addAll(
// other resources
)
return cache.addAll(
// core FILE_LIST
)
We should have a commented-out url check to disable the worker. Something that if it returns false, it disables the entire worker. We can't trust things to always go right, so having a hard killswitch to roll back a deploy can work wonders. Even if it requires a custom backend, and is disabled by default.
I guess that having service-worker activation only when NODE_ENV is set to true is enough. Anyway I'm not against this, but if we keep this commented, we should mention in comments that running tests in fresh installs will always fail until the line is uncommented or choo-service-worker
is uninstalled.
the "add to cache by default on fetch" code should probably be enabled by default - and it should probably not register resources if incorrectly fetched. Otherwise there's not too much use for service workers, right?
True to all of that. We are not registering resources when fetch fails, not sure why you said that haha.
VERSION should probably be set to process.env.npm_package_version. Turns out we don't support this yet in Bankai, but we should (opening up an issue about this right now).
:+1:
Use the package version as the sw version, this allows, for instance, only delete cache on major version, minor and patch only updates cache.
Hmmm, I think it's probably a good idea to always delete the cache, but try and copy over already existing resources over first. This is a large part of why we made hashes a part of the resource's urls, so that the name itself already verifies whether or not something has changed.
:+1:
// to emit backgroundSync events in choo, with choo-service-worker installed, emit the event 'sw:sync' passing a tag you can use it to add non-urgent resources to cache or whatever you want
I'm not entirely sure what you mean by "pass a tag". Could you explain a bit more about what you mean by this?
Well that's how the BackgroundSync API works, you just send tags, like some kind of signaling to the service worker, that's what we are doing in choo-service-worker.
Add some event handler to add resource to cache on user interaction, this might need some changes on client.
I'm intrigued by this. Could you explain a bit more about how you'd see this?
Well this is actually a bit similar to the background sync thing, because both actions are dispatched from the client to the service worker. The difference is that background sync is dispatched fro the client, but not the user, think about twitter in the browser, when you have a new tweet, the home page tells you there are n new tweets, without needing your interaction, this could be done with background sync dispatched in a time interval (not sure how they are actually doing it, just guessing haha). On the other hand, an user could save for offline (add to cache) voluntarily, for example, check Una Kravets blog, if you pick a random post, you'll see a link to save for offline, so this will go in cache only when the user wants. Ideal for blog posts or gallerys with several Gb of images when users only want a few of them.
So wrapping up, I think we should:
FILE_LIST
env var as the core files to be cached for offline.sync
event listener.The client should get some updates too:
Re this: https://github.com/choojs/bankai/issues/391
the VERSION
thing can also be done in create-choo-app using require('./package.json').version
@yoshuawuyts @goto-bus-stop I just sent a PR with some small changes related to this, still some work should be done, but I think it is a good start.
With the latest PR service workers are enabled in production and tests are passing. Closing :)
choo-service-worker
is commented and installed in a fresh choo app and I don't know why? if we uncomment that line tests would also start green and we could also close #22