diff --git a/web/src/lib/__tests__/staleBuild.test.ts b/web/src/lib/__tests__/staleBuild.test.ts index 8a62635..2064f82 100644 --- a/web/src/lib/__tests__/staleBuild.test.ts +++ b/web/src/lib/__tests__/staleBuild.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { reloadIfServerRebuilt, holdReloadWhile, makeConnectionWatcher, startBuildWatch } from "@/lib/staleBuild"; +import { reloadIfServerRebuilt, makeConnectionWatcher, startBuildWatch } from "@/lib/staleBuild"; import { APP_VERSION } from "@/lib/version"; function healthReplies(body: unknown, ok = true) { @@ -66,27 +66,6 @@ describe("reloadIfServerRebuilt", () => { }); }); -describe("unsaved work holds the page", () => { - it("does not reload while something says it has unsaved work", async () => { - const release = holdReloadWhile(() => true); - vi.stubGlobal("fetch", healthReplies({ ok: true, version: "9.9.9" })); - expect(await reloadIfServerRebuilt()).toBe(false); - expect(reload).not.toHaveBeenCalled(); - release(); - expect(await reloadIfServerRebuilt()).toBe(true); - expect(reload).toHaveBeenCalledOnce(); - }); - - it("treats a predicate that throws as a reason to wait", async () => { - const release = holdReloadWhile(() => { - throw new Error("broken"); - }); - vi.stubGlobal("fetch", healthReplies({ ok: true, version: "9.9.9" })); - expect(await reloadIfServerRebuilt()).toBe(false); - release(); - }); -}); - describe("noticing without being asked", () => { it("checks when the push stream drops, but not before it has connected", async () => { const fetchMock = healthReplies({ ok: true, version: APP_VERSION }); diff --git a/web/src/lib/staleBuild.ts b/web/src/lib/staleBuild.ts index 9c69ae3..568ed8e 100644 --- a/web/src/lib/staleBuild.ts +++ b/web/src/lib/staleBuild.ts @@ -14,10 +14,16 @@ import { push, type PushState } from "@/jmap/push"; * * `index.html` is served `no-cache` and the assets under it are content-hashed * and immutable, so a reload is all it takes; the only missing part was - * something to ask for one. Checking on a 401 rather than on a timer keeps it - * to the moment it matters and costs one small request, and comparing versions - * rather than reloading on every 401 means an ordinary session expiry still - * lands on the sign-in form with the page intact. + * something to ask for one. Comparing versions rather than reloading on every + * 401 means an ordinary session expiry still lands on the sign-in form with the + * page intact -- only a build that actually moved costs the page. + * + * The reload is unconditional once the versions differ. A compose window can + * be holding text that never reached the server, and after a deploy it cannot + * be saved either, since the session went with the container -- so this will + * sometimes take an unsent draft with it. That is a deliberate trade: a tab + * running code the server no longer speaks is the worse failure, and one that + * stays behind because someone left a draft open is not automatic at all. */ const TRIED_KEY = "ihasmail:reloaded-for"; @@ -46,35 +52,6 @@ function forget(): void { } } -/** - * Reasons to leave a stale page alone for now. - * - * A reload throws away everything the tab has not sent anywhere, and on an - * immutable instance the session is gone by the time we get here, so a compose - * window holding text that never reached the server cannot save it either. - * Reloading would be the difference between the author signing in again and - * pressing send, and losing what they wrote. Whoever owns such state says so - * here; see the registration at the bottom of `store/compose.ts`. - */ -const holds = new Set<() => boolean>(); - -export function holdReloadWhile(fn: () => boolean): () => void { - holds.add(fn); - return () => holds.delete(fn); -} - -function held(): boolean { - for (const fn of holds) { - try { - if (fn()) return true; - } catch { - /* a broken predicate is not a reason to reload over someone's work */ - return true; - } - } - return false; -} - let inFlight: Promise | null = null; /** @@ -114,7 +91,6 @@ async function check(): Promise { // still reports the old version -- a stale proxy cache, a half-finished // deploy -- this stops the two of them reloading each other in a loop. if (tried() === serverVersion) return false; - if (held()) return false; remember(serverVersion); window.location.reload(); return true; diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index b73ae2d..8330874 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -10,7 +10,6 @@ import { useMail, FULL_PROPS, BODY_PROPS } from "./mail"; import { ensureScheduledMailbox, useScheduled } from "./scheduled"; import { formatScheduleTime, holdUntil } from "@/lib/schedule"; import { settings } from "./settings"; -import { holdReloadWhile } from "@/lib/staleBuild"; export interface ComposeAttachment { id: string; @@ -808,8 +807,3 @@ export function draftFromMailto(url: string): Partial { ...(body ? { html: body, text: m.body } : {}), }; } - -// A deploy can reload this tab out from under whoever is writing. Text that has -// not been autosaved lives only here, and once the session is gone it cannot be -// saved at all -- so say so, and let them sign in and send it instead. -holdReloadWhile(() => useCompose.getState().drafts.some((d) => d.dirty));