diff --git a/scripts/screenshot.ts b/scripts/screenshot.ts index 5fe5b36..d4d3260 100644 --- a/scripts/screenshot.ts +++ b/scripts/screenshot.ts @@ -5,6 +5,7 @@ * bun scripts/screenshot.ts [--w=1280] [--h=900] * [--wait=1500] [--full] [--click=] * [--eval=|--eval-file=] [--dark] [--reload] + * [--profile=] [--eval-after-file=] * * Why this exists: ARCHITECTURE.md and TODO.md both say nothing here can run a * browser, so appearance has never been verified. Chrome IS available, but its @@ -40,6 +41,10 @@ const chrome = Bun.spawn( "--hide-scrollbars", "--force-device-scale-factor=1", "--remote-debugging-port=0", + // A fresh profile each run means empty IndexedDB, empty caches and NO + // registered service worker — which makes anything stateful across loads + // (an update prompt, a migration, an install) impossible to reproduce. + ...(flag("profile") ? [`--user-data-dir=${flag("profile")}`] : []), `--window-size=${width},${height}`, ...(has("dark") ? ["--force-dark-mode", "--enable-features=WebContentsForceDark"] : []), "about:blank", @@ -171,6 +176,21 @@ if (clickSelector) { await Bun.sleep(700); } +// Runs after the reload, so a flow that ends in a navigation can be observed +// on the other side of it — an update prompt applying itself, a migration. +const evalAfterFile = flag("eval-after-file"); +if (evalAfterFile) { + const source = await Bun.file(evalAfterFile).text(); + const result = await send("Runtime.evaluate", { + expression: source, + awaitPromise: true, + }); + console.log( + "eval-after:", + JSON.stringify(result?.result?.value ?? result?.result?.description ?? null), + ); +} + const shot = await send("Page.captureScreenshot", { format: "png", captureBeyondViewport: has("full"), diff --git a/src/index.ts b/src/index.ts index dde826c..8a0a687 100644 --- a/src/index.ts +++ b/src/index.ts @@ -37,9 +37,24 @@ const server = serve({ // clients and handles no fetches keeps registration honest in dev without // caching anything, which would fight HMR. [`${basePath}sw.js`]: () => - new Response("/* dev: intentionally does nothing */\n", { - headers: { "content-type": "text/javascript", "cache-control": "no-cache" }, - }), + new Response( + // Caches nothing — that would fight HMR — but it DOES honour the + // lifecycle, so the update prompt's Reload button behaves in dev the + // way it behaves in production instead of only ever hitting the + // timeout backstop. + [ + "/* dev worker: no caching, real lifecycle */", + 'self.addEventListener("install", () => self.skipWaiting());', + 'self.addEventListener("activate", (e) => e.waitUntil(self.clients.claim()));', + 'self.addEventListener("message", (e) => {', + ' if (e.data && e.data.type === "SKIP_WAITING") self.skipWaiting();', + "});", + "", + ].join("\n"), + { + headers: { "content-type": "text/javascript", "cache-control": "no-cache" }, + }, + ), // The @font-face rules are injected by frontend.tsx; these serve the // files those rules point at. ...Object.fromEntries( diff --git a/src/sw.ts b/src/sw.ts index 89bc08c..a6c4f4f 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -1,6 +1,8 @@ /// declare const self: ServiceWorkerGlobalScope; +import { precacheUrls } from "./swPrecache.ts"; + // Both are replaced at build time by Bun.build's `define`. const PRECACHE: string[] = JSON.parse(process.env.PRECACHE_MANIFEST ?? "[]"); const VERSION = process.env.SW_VERSION ?? "dev"; @@ -10,8 +12,10 @@ const CACHE = `budget2-${VERSION}`; const SHELL = `${BASE}index.html`; self.addEventListener("install", (event) => { + // Deduped: addAll() rejects the whole install on a repeated request, and the + // build's own outputs already contain the shell. See src/swPrecache.ts. event.waitUntil( - caches.open(CACHE).then((cache) => cache.addAll([...PRECACHE, SHELL])), + caches.open(CACHE).then((cache) => cache.addAll(precacheUrls(PRECACHE, SHELL))), ); }); diff --git a/src/swPrecache.test.ts b/src/swPrecache.test.ts new file mode 100644 index 0000000..89f8e54 --- /dev/null +++ b/src/swPrecache.test.ts @@ -0,0 +1,53 @@ +import { test, expect, describe } from "bun:test"; +import { precacheUrls } from "./swPrecache.ts"; + +describe("the service worker's precache list", () => { + /** + * The bug this exists for: `cache.addAll()` rejects the WHOLE install with + * InvalidStateError if the list contains the same request twice. The build's + * precache list is derived from its own outputs, which already include + * index.html, and the worker then appended the shell — index.html again. So + * install failed every time, the worker went redundant, and no service + * worker ever activated. Nothing reported it: registration itself succeeds, + * and the failure is inside the install event. + */ + test("the shell is not repeated when the build already emitted it", () => { + expect(precacheUrls(["/index.html", "/index-abc.js"], "/index.html")).toEqual([ + "/index.html", + "/index-abc.js", + ]); + }); + + test("the shell is added when the build did not emit it", () => { + expect(precacheUrls(["/index-abc.js"], "/index.html")).toEqual([ + "/index-abc.js", + "/index.html", + ]); + }); + + test("any duplicate is removed, not just the shell", () => { + expect(precacheUrls(["/a.js", "/b.css", "/a.js"], "/index.html")).toEqual([ + "/a.js", + "/b.css", + "/index.html", + ]); + }); + + test("order is preserved, so the shell is not fetched before the assets", () => { + expect(precacheUrls(["/z.js", "/a.css"], "/index.html")).toEqual([ + "/z.js", + "/a.css", + "/index.html", + ]); + }); + + test("a subpath deploy is not a special case", () => { + expect( + precacheUrls(["/budget2.0/index.html", "/budget2.0/x.js"], "/budget2.0/index.html"), + ).toEqual(["/budget2.0/index.html", "/budget2.0/x.js"]); + }); + + test("an empty build still precaches the shell", () => { + expect(precacheUrls([], "/index.html")).toEqual(["/index.html"]); + }); +}); diff --git a/src/swPrecache.ts b/src/swPrecache.ts new file mode 100644 index 0000000..ecad6b1 --- /dev/null +++ b/src/swPrecache.ts @@ -0,0 +1,20 @@ +/** + * The exact list the service worker precaches on install. + * + * Separate from src/sw.ts so it can be tested: importing sw.ts outside a + * worker would run its `self.addEventListener` calls. + * + * Deduplication is the whole point. `cache.addAll()` rejects the ENTIRE + * install with InvalidStateError if the same request appears twice, and the + * build's precache list is derived from its own outputs — which already + * include index.html — while the worker also needs the shell by name. Those + * two facts collided silently: install failed on every load, the worker went + * redundant, and no service worker ever activated. Registration itself + * succeeds, so nothing surfaced it. + * + * Deduping here rather than in build.ts means no future addition to either + * list can reintroduce the collision. + */ +export function precacheUrls(precache: readonly string[], shell: string): string[] { + return [...new Set([...precache, shell])]; +} diff --git a/src/ui/registerSw.ts b/src/ui/registerSw.ts index fb89983..185aaaa 100644 --- a/src/ui/registerSw.ts +++ b/src/ui/registerSw.ts @@ -1,4 +1,5 @@ import { BASE_PATH, withBase } from "./basePath.ts"; +import { requestUpdate } from "./swUpdate.ts"; let waiting: ServiceWorker | null = null; @@ -32,8 +33,32 @@ export function registerServiceWorker(onUpdateReady: () => void): void { }); } +/** + * Reloads only once the new worker is actually in control. Posting + * SKIP_WAITING and reloading on the next line raced activation, and when the + * reload won, the reloaded page still found a waiting worker and showed the + * update prompt again — forever. See src/ui/swUpdate.ts for the measurement. + */ export function applyUpdate(): void { - waiting?.postMessage({ type: "SKIP_WAITING" }); + const target = waiting; waiting = null; - window.location.reload(); + + if (!target) { + window.location.reload(); + return; + } + + requestUpdate({ + postSkipWaiting: () => target.postMessage({ type: "SKIP_WAITING" }), + onControllerChange: (listener) => { + navigator.serviceWorker.addEventListener("controllerchange", listener); + return () => + navigator.serviceWorker.removeEventListener("controllerchange", listener); + }, + setTimer: (fn, ms) => { + const id = setTimeout(fn, ms); + return () => clearTimeout(id); + }, + reload: () => window.location.reload(), + }); } diff --git a/src/ui/swUpdate.test.ts b/src/ui/swUpdate.test.ts new file mode 100644 index 0000000..57da326 --- /dev/null +++ b/src/ui/swUpdate.test.ts @@ -0,0 +1,100 @@ +import { test, expect, describe } from "bun:test"; +import { requestUpdate, type UpdateHandle } from "./swUpdate.ts"; + +function fakeHandle() { + const events: string[] = []; + let controllerListener: (() => void) | null = null; + let timer: { fn: () => void; ms: number } | null = null; + let unsubscribed = false; + + const handle: UpdateHandle = { + postSkipWaiting() { + events.push("postSkipWaiting"); + }, + onControllerChange(listener) { + events.push("listen"); + controllerListener = listener; + return () => { + unsubscribed = true; + }; + }, + setTimer(fn, ms) { + timer = { fn, ms }; + return () => {}; + }, + reload() { + events.push("reload"); + }, + }; + + return { + handle, + events, + get unsubscribed() { + return unsubscribed; + }, + fireControllerChange: () => controllerListener?.(), + fireTimeout: () => timer?.fn(), + get timerMs() { + return timer?.ms; + }, + }; +} + +describe("applying a waiting service worker", () => { + /** + * The bug: applyUpdate posted SKIP_WAITING and called reload() on the very + * next line. Measured in Chrome, the controller had NOT changed at that + * instant — it changed 2 ms later — so the reload raced activation. When the + * reload won, the fresh page still saw registration.waiting and showed + * "A new version is ready" again, and clicking Reload re-entered the same + * race. The prompt never went away. + */ + test("does not reload until the new worker actually takes control", () => { + const f = fakeHandle(); + requestUpdate(f.handle); + expect(f.events).toEqual(["listen", "postSkipWaiting"]); + expect(f.events).not.toContain("reload"); + }); + + test("listens before posting, or a fast activation is missed entirely", () => { + const f = fakeHandle(); + requestUpdate(f.handle); + expect(f.events.indexOf("listen")).toBeLessThan(f.events.indexOf("postSkipWaiting")); + }); + + test("reloads once the controller changes", () => { + const f = fakeHandle(); + requestUpdate(f.handle); + f.fireControllerChange(); + expect(f.events).toEqual(["listen", "postSkipWaiting", "reload"]); + }); + + /** + * A worker that ignores SKIP_WAITING — the deliberately inert one the dev + * server serves, or a genuinely broken build — must not leave the Reload + * button dead. Reload anyway; the user asked for it. + */ + test("reloads anyway if the worker never takes control", () => { + const f = fakeHandle(); + requestUpdate(f.handle, 1500); + expect(f.timerMs).toBe(1500); + f.fireTimeout(); + expect(f.events).toEqual(["listen", "postSkipWaiting", "reload"]); + }); + + test("reloads exactly once when both the change and the timeout fire", () => { + const f = fakeHandle(); + requestUpdate(f.handle); + f.fireControllerChange(); + f.fireTimeout(); + expect(f.events.filter((e) => e === "reload")).toHaveLength(1); + }); + + test("stops listening once it has reloaded", () => { + const f = fakeHandle(); + requestUpdate(f.handle); + f.fireControllerChange(); + expect(f.unsubscribed).toBe(true); + }); +}); diff --git a/src/ui/swUpdate.ts b/src/ui/swUpdate.ts new file mode 100644 index 0000000..44da19e --- /dev/null +++ b/src/ui/swUpdate.ts @@ -0,0 +1,52 @@ +/** + * Sequencing for "apply the waiting service worker, then reload". + * + * Pure, with every browser touchpoint injected, because the bug it fixes is + * entirely about ORDER and order is what a test can pin. The old code was: + * + * waiting.postMessage({ type: "SKIP_WAITING" }); + * window.location.reload(); + * + * Measured in Chrome: at the moment of that reload the new worker was still + * `installed` and the controller had not changed — it changed 2 ms later. So + * the reload raced activation. When the reload won, the fresh page load still + * found `registration.waiting` and showed "A new version is ready" again, and + * pressing Reload re-entered the same race. The prompt never went away. + * + * The window is wider than 2 ms in practice: the worker's `activate` handler + * deletes every stale cache before calling `clients.claim()`. + */ +export interface UpdateHandle { + /** Tells the waiting worker to activate now. */ + postSkipWaiting(): void; + /** Subscribes to the page's controller changing. Returns an unsubscribe. */ + onControllerChange(listener: () => void): () => void; + setTimer(fn: () => void, ms: number): () => void; + reload(): void; +} + +/** + * `timeoutMs` is a backstop, not a delay: a worker that ignores SKIP_WAITING — + * the deliberately inert one the dev server serves, or a broken build — must + * not leave the Reload button doing nothing at all. It reloads anyway, because + * that is what the user asked for. + */ +export function requestUpdate(handle: UpdateHandle, timeoutMs = 3000): void { + let reloaded = false; + + const finish = () => { + if (reloaded) return; + reloaded = true; + stopListening(); + cancelTimer(); + handle.reload(); + }; + + // Subscribe BEFORE posting. Activation can complete in a couple of + // milliseconds, and a listener attached afterwards would miss it and fall + // through to the timeout every time. + const stopListening = handle.onControllerChange(finish); + const cancelTimer = handle.setTimer(finish, timeoutMs); + + handle.postSkipWaiting(); +}