[docs] add service-worker example #7461
Conversation
|
|
I'm not sure how I feel about this. Service workers stick around in the browser so if you run one app with a service worker and then run a different app the service worker from the previous one is still there so it's a very strong form of caching. It makes debugging so much more annoying because I'm constantly having to remove service workers from reproduction reports and sometimes forget it's needed and it confuses users who don't know it will be happening. Personally I'm not sure I would add a service worker at all, but if we do I'd at least argue for it to be an option with a default value of false. |
|
The service worker hanging around resulting in confusion is a good point - but don't you have that problem anyway as soon as you use any form of service-worker? Also, it's only for the demo app. What if we prefix the file with an underscore and add a comment to the file and the about page that you need to "opt in" to the service worker, and why we do that? |
|
Yes, you'd probably have that problem with any form of service worker. Making it unused would be fine. Perhaps it would be clearer to call it something like |
dummdidumm
changed the title
|
Removed the service worker from the demo, more details in the PR description |
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
|
|
||
| Because service workers need to be bundled (since browsers don't yet support `import` in this context), **SvelteKit's service workers only work in the production build, not in development**, since it depends on the client-side app's build manifest. To test it locally, use `vite preview` after running a build. | ||
|
|
||
| SvelteKit's service worker implementation is deliberately low-level. If you need a more full-flegded but also more opinionated solution, we recommend looking at solutions like [Vite PWA plugin](https://vite-pwa-org.netlify.app/frameworks/sveltekit.html), which uses [Workbox](https://web.dev/learn/pwa/workbox). For more general information on service workers, we recommend [the MDN web docs](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers). |
There was a problem hiding this comment.
we could add a heading here to make it linkable from above
| SvelteKit's service worker implementation is deliberately low-level. If you need a more full-flegded but also more opinionated solution, we recommend looking at solutions like [Vite PWA plugin](https://vite-pwa-org.netlify.app/frameworks/sveltekit.html), which uses [Workbox](https://web.dev/learn/pwa/workbox). For more general information on service workers, we recommend [the MDN web docs](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers). | |
| ## External solutions | |
| SvelteKit's service worker implementation is deliberately low-level. If you need a more full-flegded but also more opinionated solution, we recommend looking at solutions like [Vite PWA plugin](https://vite-pwa-org.netlify.app/frameworks/sveltekit.html), which uses [Workbox](https://web.dev/learn/pwa/workbox). For more general information on service workers, we recommend [the MDN web docs](https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API/Using_Service_Workers). |
There was a problem hiding this comment.
Given that the section isn't very big I'm not sure we need this. "On this page" would look rather empty, and the only entry being "External Solutions" feels kinda weird. So if we want to, we would need another heading somewhere.
There was a problem hiding this comment.
yeah, that's a fair point. other ideas for headings could be:
- creation
- registration (probably a sub-heading of the previous one)
- example
There was a problem hiding this comment.
I'll try to release the pwa plugin for kit next week. I need to merge a few PR and publish a New pwa plugin version before releasing pwa plugin integrations.
With vite-pwa/vite-plugin-pwa#390 we can use something similar to kit sw, just check the sw in the PR example.
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
| async function deleteOldCaches() { | ||
| const keyList = await caches.keys(); | ||
| const cachesToDelete = keyList.filter((key) => key !== CACHE_NAME); | ||
| await Promise.all(cachesToDelete.map((key) => caches.delete(key))); | ||
| } |
There was a problem hiding this comment.
Unless something has gone wrong, we should only ever need to delete a single old cache (and in any case, it should be a very fast operation) so i think we can use this theoretically-slower-but-in-practice-identical code which I think is a little easier to parse
| async function deleteOldCaches() { | |
| const keyList = await caches.keys(); | |
| const cachesToDelete = keyList.filter((key) => key !== CACHE_NAME); | |
| await Promise.all(cachesToDelete.map((key) => caches.delete(key))); | |
| } | |
| async function deleteOldCaches() { | |
| for (const key of await caches.keys()) { | |
| if (key !== CACHE_NAME) await caches.delete(key); | |
| } | |
| } |
| async function cacheFirst(request) { | ||
| const responseFromCache = await caches.match(request); | ||
| if (responseFromCache) { | ||
| return responseFromCache; | ||
| } | ||
| const response = await fetch(request); | ||
| addToCache(request, response.clone()); | ||
| return response; | ||
| } | ||
|
|
||
| event.respondWith(cacheFirst(event.request)); | ||
| }); | ||
| ``` | ||
|
|
||
| > Careful with caching too much for too long! Browsers have a limit on the amount they cache, and not everything should be cached, for example requests that contain dynamic data that change over time |
There was a problem hiding this comment.
Since people are likely to just copy whatever we put in the docs, I'm not sure we should use a cache-first strategy here — it's a footgun. Cache-first makes sense for build and files, but for pages we should probably have a network-first strategy instead (and I think we should only fall back to the cache for GET requests?)
There was a problem hiding this comment.
Nuxt uses this workbox preset/template: https://github.com/vite-pwa/vite-plugin-pwa/pull/390/files#diff-601038314cf235badafad57b9aca2c222c80d89c6a5153ee0708158a1efdb0d3R27
* network-first strategy * only cache 200 responses
Decided against a service worker in the demo app because as @benmccann points out that get's really confusing/annoying really quickly. This PR previously contained a service worker in the demo app, and I got confused for a moment when running another app in preview mode getting served the old demo app, so I threw it out.
Closes #2246
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. All changesets should bepatchuntil SvelteKit 1.0