Skip to content

Conversation

@certuna
Copy link
Contributor

@certuna certuna commented Mar 31, 2021

This PR gives the Navidrome web UI a 'proper' own service worker, not just the current boilerplate service-worker.js that gets auto-generated during the build process by CRA, which is outdated anyway since it loads Workbox v3 libraries instead of the current v6.1.2. Plus, that service worker caches stuff for offline usage, which breaks things.

Workbox, by the way, is pretty cool stuff: basically it's a set of Google libraries to make service workers easy. See: https://developers.google.com/web/tools/workbox

Changes in this PR:

  • updated serviceWorker.js to load this new service worker
  • named the new service worker navidrome-service-worker.js so it doesn't get overwritten by the CRA generated one
  • added a new file offline.html containing an error message

So what does the service worker do?

Please test this, for example:

  • by changing between two different builds of the server and see if the fresh service-worker gets loaded
  • log out, close tab, log in on various browsers / PWA platforms
  • go offline, with both desktop and mobile browsers, check if offline.html gets displayed

@@ -0,0 +1,48 @@
// documentation: https://developers.google.com/web/tools/workbox/modules/workbox-sw
importScripts("https://storage.googleapis.com/workbox-cdn/releases/6.1.2/workbox-sw.js");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this imported at build time or at runtime? I don't want my navidrome instance to phone home to Google.

Copy link
Contributor Author

@certuna certuna Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Runtime. Believe it or not, but the current service worker also does that :)

We can pull in these libraries with npm too during the build: https://developers.google.com/web/tools/workbox/modules

@deluan deluan mentioned this pull request Apr 2, 2021
@deluan
Copy link
Member

deluan commented Apr 2, 2021

LGTM. Surely can be improved (ex: not loading the workbox libraries directly from google, making the offline page translatable), but works better than the current one. Thanks @certuna! Merging now.

@deluan deluan merged commit 9871919 into navidrome:master Apr 2, 2021
deluan added a commit that referenced this pull request Apr 13, 2021
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

service worker: "fetch" event handler needs to return a valid response when the device is offline response served by service worker has redirections

3 participants