Reload even when there is an unsent draft
Holding the reload back while a compose window had unsaved text protected the text, but it meant a tab could sit on a build the server no longer runs for as long as someone left a draft open -- which is not automatic, and automatic is the point. So the reload is unconditional once the versions differ, and this will sometimes take an unsent draft with it. The trade is deliberate: a tab talking to a server it does not match is the worse failure, and it fails quietly.
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
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";
|
import { APP_VERSION } from "@/lib/version";
|
||||||
|
|
||||||
function healthReplies(body: unknown, ok = true) {
|
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", () => {
|
describe("noticing without being asked", () => {
|
||||||
it("checks when the push stream drops, but not before it has connected", async () => {
|
it("checks when the push stream drops, but not before it has connected", async () => {
|
||||||
const fetchMock = healthReplies({ ok: true, version: APP_VERSION });
|
const fetchMock = healthReplies({ ok: true, version: APP_VERSION });
|
||||||
|
|||||||
+10
-34
@@ -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
|
* `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
|
* 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
|
* something to ask for one. Comparing versions rather than reloading on every
|
||||||
* to the moment it matters and costs one small request, and comparing versions
|
* 401 means an ordinary session expiry still lands on the sign-in form with the
|
||||||
* rather than reloading on every 401 means an ordinary session expiry still
|
* page intact -- only a build that actually moved costs the page.
|
||||||
* lands on the sign-in form with the page intact.
|
*
|
||||||
|
* 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";
|
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<boolean> | null = null;
|
let inFlight: Promise<boolean> | null = null;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -114,7 +91,6 @@ async function check(): Promise<boolean> {
|
|||||||
// still reports the old version -- a stale proxy cache, a half-finished
|
// 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.
|
// deploy -- this stops the two of them reloading each other in a loop.
|
||||||
if (tried() === serverVersion) return false;
|
if (tried() === serverVersion) return false;
|
||||||
if (held()) return false;
|
|
||||||
remember(serverVersion);
|
remember(serverVersion);
|
||||||
window.location.reload();
|
window.location.reload();
|
||||||
return true;
|
return true;
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import { useMail, FULL_PROPS, BODY_PROPS } from "./mail";
|
|||||||
import { ensureScheduledMailbox, useScheduled } from "./scheduled";
|
import { ensureScheduledMailbox, useScheduled } from "./scheduled";
|
||||||
import { formatScheduleTime, holdUntil } from "@/lib/schedule";
|
import { formatScheduleTime, holdUntil } from "@/lib/schedule";
|
||||||
import { settings } from "./settings";
|
import { settings } from "./settings";
|
||||||
import { holdReloadWhile } from "@/lib/staleBuild";
|
|
||||||
|
|
||||||
export interface ComposeAttachment {
|
export interface ComposeAttachment {
|
||||||
id: string;
|
id: string;
|
||||||
@@ -808,8 +807,3 @@ export function draftFromMailto(url: string): Partial<Draft> {
|
|||||||
...(body ? { html: body, text: m.body } : {}),
|
...(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));
|
|
||||||
|
|||||||
Reference in New Issue
Block a user