commit fe307e8197383af1434b2e8aed38d9fc715b13ef
parent c661ab224d6fe5d7596ff72042b58caf358afc2b
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Tue, 22 Sep 2026 16:19:14 -0400
contract: guard shipsPwa's env read, and say what the guard is for
The comment already named the fix and left it undone: "if this ever becomes
reachable from a client module it needs a `typeof process !== \"undefined\"`
guard". The guard costs one clause and turns the failure mode from
`ReferenceError: process is not defined` at import time — which takes the whole
page down — into `false`, no PWA, the right answer for every site but a hub.
It is NOT hub detection working client-side, and the rewritten comment says so
twice: INSTANCE_MODE is neither NEXT_PUBLIC_ nor in a next.config `env:` block,
so a browser reads undefined whatever this line does. The test asserts the
source, because `typeof process` is true in every runtime the suite runs in and
the guard is therefore invisible in a return value — the same technique the
service-worker checks in this file already use.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/common/lib/archive/contract.test.ts b/common/lib/archive/contract.test.ts
@@ -261,3 +261,24 @@ test("the search index worker's private pageFileName matches CONTRACT.pagePad",
assert.ok(src.includes("`/transcripts/${slug}/${pageFileName(p)}`"));
assert.equal(pageUrl("transcripts", "alpha", 12), "/transcripts/alpha/page-0012.json");
});
+
+// shipsPwa is the one function in this module that reads the ambient
+// environment, and the guard in front of that read is not observable from its
+// return value — `typeof process` is true in every runtime the test suite has.
+// So the assertion is on the SOURCE, the same way the service-worker checks
+// above are: what is being pinned is that the read cannot throw at import time
+// in a browser, not what it answers.
+test("shipsPwa guards its process.env read", () => {
+ const src = readSource("common/lib/archive/contract.ts");
+ const fn = /export function shipsPwa\([\s\S]*?\n}/.exec(src);
+ assert.ok(fn, "shipsPwa not found — did it move or get renamed?");
+ assert.match(fn[0], /typeof process !== "undefined"/);
+ // The guard must come BEFORE the dereference, which is the only arrangement
+ // that stops `ReferenceError: process is not defined`.
+ assert.ok(
+ fn[0].indexOf('typeof process !== "undefined"') <
+ fn[0].indexOf("process.env.INSTANCE_MODE"),
+ "the guard must precede the read",
+ );
+ // What it ANSWERS is unchanged, and is pinned by the behaviour test above.
+});
diff --git a/common/lib/archive/contract.ts b/common/lib/archive/contract.ts
@@ -180,18 +180,30 @@ export function pageUrl(
// Was two copies with "keep in sync" comments on each (compose-site.ts and
// export/app/lib/mode.ts); S2c deleted both, and this is the one.
//
-// `process.env.INSTANCE_MODE` is read bare, exactly as both copies read it, and
-// the reason that is safe is NOT that Next inlines it — it does not:
-// INSTANCE_MODE is neither `NEXT_PUBLIC_` nor listed in a next.config `env:`
-// block, so a client bundle would read `undefined` here. (An earlier draft of
-// this comment claimed the opposite; the S1 review checked.) It is safe because
-// no browser evaluates it: the only caller in the export app is
-// export/app/lib/mode.ts, whose only caller is export/app/layout.tsx, a SERVER
-// component, and the other caller is compose-site.ts, a build script. If this
-// ever becomes reachable from a client module it needs a
-// `typeof process !== "undefined"` guard AND an env var the client can see.
+// THE `typeof process` GUARD STOPS A CRASH. IT DOES NOT MAKE HUB DETECTION
+// WORK CLIENT-SIDE, and that distinction is the whole comment.
+//
+// `INSTANCE_MODE` is neither `NEXT_PUBLIC_` nor listed in a next.config `env:`
+// block, so Next does not inline it and a client bundle reads `undefined` here.
+// (An earlier draft of this comment claimed the opposite; the S1 review
+// checked.) Hub detection is therefore SERVER-ONLY, guard or no guard, until
+// somebody ships the value to the client deliberately — which would need an env
+// var the client can actually see, not a change to this line.
+//
+// Today that costs nothing: the export app reaches this only through
+// export/app/lib/mode.ts ← export/app/layout.tsx, a SERVER component, and the
+// other caller is compose-site.ts, a build script. What the guard buys is the
+// failure mode when that stops being true. An unguarded `process.env` in a
+// module some client component pulls in throws `ReferenceError: process is not
+// defined` at import time and takes the page down with it; guarded, the same
+// import yields `false` — no PWA, which is the right answer for every site but
+// a hub and a recoverable one for a hub. A wrong answer nobody notices beats a
+// white screen.
export function shipsPwa(site: { pwa?: boolean }): boolean {
- return site.pwa === true || process.env.INSTANCE_MODE === "hub";
+ return (
+ site.pwa === true ||
+ (typeof process !== "undefined" && process.env.INSTANCE_MODE === "hub")
+ );
}
// ─── The hub's member entry, once ───
