Unregister non-matching serviceworkers (#15834)

* Unregister non-matching serviceworkers

With the addition of the /assets url, users who visited a previous
version of the site now may have two active service workers, one with
the old scope `/` and one with scope `/assets`. This check for
serviceworkers that do not match the current script path and unregisters
them.

Also included is a small refactor to publicpath.js which was simplified
because AssetUrlPrefix is always present now. Also it makes use of the
new joinPaths helper too.

Fixes: #15823

Author: silverwind

models/user.go

@@ -789,6 +789,7 @@ var (
 		"debug",
 		"error",
 		"explore",
+		"favicon.ico",
 		"ghost",
 		"help",
 		"install",
@@ -807,10 +808,10 @@ var (
 		"repo",
 		"robots.txt",
 		"search",
+		"serviceworker.js",
 		"stars",
 		"template",
 		"user",
-		"favicon.ico",
 	}
 
 	reservedUserPatterns = []string{"*.keys", "*.gpg"}

web_src/js/features/serviceworker.js

@@ -1,18 +1,26 @@
-const {UseServiceWorker, AppSubUrl, AppVer} = window.config;
+import {joinPaths} from '../utils.js';
+
+const {UseServiceWorker, AppSubUrl, AssetUrlPrefix, AppVer} = window.config;
 const cachePrefix = 'static-cache-v'; // actual version is set in the service worker script
+const workerAssetPath = joinPaths(AssetUrlPrefix, 'serviceworker.js');
 
-async function unregister() {
-  const registrations = await navigator.serviceWorker.getRegistrations();
-  await Promise.all(registrations.map((registration) => {
-    return registration.active && registration.unregister();
-  }));
+async function unregisterAll() {
+  for (const registration of await navigator.serviceWorker.getRegistrations()) {
+    if (registration.active) await registration.unregister();
+  }
+}
+
+async function unregisterOtherWorkers() {
+  for (const registration of await navigator.serviceWorker.getRegistrations()) {
+    const scriptURL = registration.active?.scriptURL || '';
+    if (!scriptURL.endsWith(workerAssetPath)) await registration.unregister();
+  }
 }
 
 async function invalidateCache() {
-  const cacheKeys = await caches.keys();
-  await Promise.all(cacheKeys.map((key) => {
-    return key.startsWith(cachePrefix) && caches.delete(key);
-  }));
+  for (const key of await caches.keys()) {
+    if (key.startsWith(cachePrefix)) caches.delete(key);
+  }
 }
 
 async function checkCacheValidity() {
@@ -30,24 +38,20 @@ export default async function initServiceWorker() {
   if (!('serviceWorker' in navigator)) return;
 
   if (UseServiceWorker) {
+    // unregister all service workers where scriptURL does not match the current one
+    await unregisterOtherWorkers();
     try {
       // normally we'd serve the service worker as a static asset from AssetUrlPrefix but
       // the spec strictly requires it to be same-origin so it has to be AppSubUrl to work
-      await Promise.all([
-        checkCacheValidity(),
-        navigator.serviceWorker.register(`${AppSubUrl}/assets/serviceworker.js`),
-      ]);
+      await checkCacheValidity();
+      await navigator.serviceWorker.register(joinPaths(AppSubUrl, workerAssetPath));
     } catch (err) {
       console.error(err);
-      await Promise.all([
-        invalidateCache(),
-        unregister(),
-      ]);
+      await invalidateCache();
+      await unregisterAll();
     }
   } else {
-    await Promise.all([
-      invalidateCache(),
-      unregister(),
-    ]);
+    await invalidateCache();
+    await unregisterAll();
   }
 }

web_src/js/publicpath.js

@@ -1,10 +1,6 @@
 // This sets up the URL prefix used in webpack's chunk loading.
 // This file must be imported before any lazy-loading is being attempted.
+import {joinPaths} from './utils.js';
 const {AssetUrlPrefix} = window.config;
 
-if (AssetUrlPrefix) {
-  __webpack_public_path__ = AssetUrlPrefix.endsWith('/') ? AssetUrlPrefix : `${AssetUrlPrefix}/`;
-} else {
-  const url = new URL(document.currentScript.src);
-  __webpack_public_path__ = url.pathname.replace(/\/[^/]*?\/[^/]*?$/, '/');
-}
+__webpack_public_path__ = joinPaths(AssetUrlPrefix, '/');

web_src/js/utils.js

@@ -9,6 +9,16 @@ export function extname(path = '') {
   return ext || '';
 }
 
+// join a list of path segments with slashes, ensuring no double slashes
+export function joinPaths(...parts) {
+  let str = '';
+  for (const part of parts) {
+    if (!part) continue;
+    str = !str ? part : `${str.replace(/\/$/, '')}/${part.replace(/^\//, '')}`;
+  }
+  return str;
+}
+
 // test whether a variable is an object
 export function isObject(obj) {
   return Object.prototype.toString.call(obj) === '[object Object]';

web_src/js/utils.test.js

@@ -1,5 +1,5 @@
 import {
-  basename, extname, isObject, uniq, stripTags,
+  basename, extname, isObject, uniq, stripTags, joinPaths,
 } from './utils.js';
 
 test('basename', () => {
@@ -15,6 +15,45 @@ test('extname', () => {
   expect(extname('file.js')).toEqual('.js');
 });
 
+test('joinPaths', () => {
+  expect(joinPaths('', '')).toEqual('');
+  expect(joinPaths('', 'b')).toEqual('b');
+  expect(joinPaths('', '/b')).toEqual('/b');
+  expect(joinPaths('', '/b/')).toEqual('/b/');
+  expect(joinPaths('a', '')).toEqual('a');
+  expect(joinPaths('/a', '')).toEqual('/a');
+  expect(joinPaths('/a/', '')).toEqual('/a/');
+  expect(joinPaths('a', 'b')).toEqual('a/b');
+  expect(joinPaths('a', '/b')).toEqual('a/b');
+  expect(joinPaths('/a', '/b')).toEqual('/a/b');
+  expect(joinPaths('/a', '/b')).toEqual('/a/b');
+  expect(joinPaths('/a/', '/b')).toEqual('/a/b');
+  expect(joinPaths('/a', '/b/')).toEqual('/a/b/');
+  expect(joinPaths('/a/', '/b/')).toEqual('/a/b/');
+
+  expect(joinPaths('', '', '')).toEqual('');
+  expect(joinPaths('', 'b', '')).toEqual('b');
+  expect(joinPaths('', 'b', 'c')).toEqual('b/c');
+  expect(joinPaths('', '', 'c')).toEqual('c');
+  expect(joinPaths('', '/b', '/c')).toEqual('/b/c');
+  expect(joinPaths('/a', '', '/c')).toEqual('/a/c');
+  expect(joinPaths('/a', '/b', '')).toEqual('/a/b');
+
+  expect(joinPaths('', '/')).toEqual('/');
+  expect(joinPaths('a', '/')).toEqual('a/');
+  expect(joinPaths('', '/', '/')).toEqual('/');
+  expect(joinPaths('/', '/')).toEqual('/');
+  expect(joinPaths('/', '')).toEqual('/');
+  expect(joinPaths('/', 'b')).toEqual('/b');
+  expect(joinPaths('/', 'b/')).toEqual('/b/');
+  expect(joinPaths('/', '', '/')).toEqual('/');
+  expect(joinPaths('/', 'b', '/')).toEqual('/b/');
+  expect(joinPaths('/', 'b/', '/')).toEqual('/b/');
+  expect(joinPaths('a', '/', '/')).toEqual('a/');
+  expect(joinPaths('/', '/', 'c')).toEqual('/c');
+  expect(joinPaths('/', '/', 'c/')).toEqual('/c/');
+});
+
 test('isObject', () => {
   expect(isObject({})).toBeTrue();
   expect(isObject([])).toBeFalse();