From 9d9e9c4536ed98e3fe319e15c7c5cd8b3dbe1cf3 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 14:41:06 -0400 Subject: [PATCH 1/3] docs: correct the fabro-web bundler reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apps/fabro-web is bundled by a custom Bun script (scripts/build.ts), not Vite. The stale reference sends agents toward Vite-specific APIs — most notably `vite:preloadError`, which does not exist in this codebase — when reasoning about the SPA build. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index bdc9ab53f..59712966b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,7 +123,7 @@ Fabro is an AI-powered workflow orchestration platform. Workflows are defined as - **fabro-util** — Shared utilities (redaction, terminal formatting) ### TypeScript (`apps/` and `lib/packages/`) -- **apps/fabro-web** — React 19 + React Router + Vite + Tailwind CSS frontend +- **apps/fabro-web** — React 19 + React Router + Tailwind CSS frontend, bundled by a custom Bun script (`apps/fabro-web/scripts/build.ts`), not Vite - **lib/packages/fabro-api-client** — Auto-generated TypeScript Axios client from OpenAPI spec ### Key design patterns From 695a981f42c7456e4aa926debf25bb788cbd8fd8 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 14:45:26 -0400 Subject: [PATCH 2/3] feat(web): tell open tabs when a new build ships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tab left open across a deploy keeps running the previous build's JavaScript indefinitely. index.html is fetched only on a full page load, all later navigation is client-side, and hashed bundles are served `immutable`, so nothing reveals that the code is stale. This produced a false-positive bug report where two correctly-deployed fixes appeared to be missing. Publishes a build id and offers a reload when the running document falls behind. The toast never reloads on its own; the only automatic reload is recovery from a chunk that no longer exists. Build id derivation ------------------- The obvious approach — hash the emitted asset filenames, which already embed content hashes — does not work: Bun's minified identifier naming is not deterministic. Building an unchanged tree twice produces byte-different output roughly one run in three (same length, ~100k differing bytes, all of it mangled names). Output hashes therefore move with no source change, which would fire the toast on redeploys of identical code and train people to ignore it. The id is instead derived from the bundle's source inputs, so it changes if and only if something we control changed. Verified stable across eight consecutive builds while the entry hash flipped between both variants. This non-determinism also means two builds of the same commit embed different bytes into the server binary, which is worth addressing separately for reproducible builds. Detection --------- SWR with `refreshInterval` + `revalidateOnFocus`, per the repo's React effects policy. SWR does not poll while the document is hidden, so background tabs stay quiet without extra gating. Unknown state on either side — missing meta tag, failed fetch, 503 during a dev rebuild — never produces a prompt. Stylesheet hashing ------------------ Tailwind's output was stable-named and therefore served `no-cache`, letting a tab revalidate into new CSS while running old JS. Tailwind purges unused classes per build, so classes the old bundle still emits could silently lose their styles. It is now content-hashed and moves with the build. Co-Authored-By: Claude Opus 5 (1M context) --- .../playground/canvas/use-canvas-render.ts | 4 +- apps/fabro-web/app/components/toast.tsx | 15 ++ .../app/hooks/use-build-version-guard.ts | 49 ++++++ .../app/hooks/use-rendered-viz-diagram.ts | 4 +- .../app/hooks/use-terminal-session.ts | 5 +- apps/fabro-web/app/layouts/app-shell.tsx | 4 + apps/fabro-web/app/lib/build-version.test.ts | 59 +++++++ apps/fabro-web/app/lib/build-version.ts | 68 ++++++++ apps/fabro-web/app/lib/import-chunk.test.ts | 132 +++++++++++++++ apps/fabro-web/app/lib/import-chunk.ts | 54 ++++++ apps/fabro-web/app/routes/run-files.tsx | 9 +- apps/fabro-web/index.template.html | 1 + apps/fabro-web/scripts/build.test.ts | 60 ++++++- apps/fabro-web/scripts/build.ts | 157 ++++++++++++++++-- lib/apps/fabro-server/src/static_files.rs | 7 + 15 files changed, 608 insertions(+), 20 deletions(-) create mode 100644 apps/fabro-web/app/hooks/use-build-version-guard.ts create mode 100644 apps/fabro-web/app/lib/build-version.test.ts create mode 100644 apps/fabro-web/app/lib/build-version.ts create mode 100644 apps/fabro-web/app/lib/import-chunk.test.ts create mode 100644 apps/fabro-web/app/lib/import-chunk.ts diff --git a/apps/fabro-web/app/components/playground/canvas/use-canvas-render.ts b/apps/fabro-web/app/components/playground/canvas/use-canvas-render.ts index 688f27d8b..d11b7bb67 100644 --- a/apps/fabro-web/app/components/playground/canvas/use-canvas-render.ts +++ b/apps/fabro-web/app/components/playground/canvas/use-canvas-render.ts @@ -1,5 +1,7 @@ import { useEffect, useRef, useState, type RefObject } from "react"; +import { importChunk } from "../../../lib/import-chunk"; + /** * Synchronizes a DOM container with a Graphviz-rendered SVG. Pipes the * supplied DOT string through `@viz-js/viz` (the same layout engine Fabro @@ -28,7 +30,7 @@ export function useCanvasRender( let cancelled = false; (async () => { try { - const { instance } = await import("@viz-js/viz"); + const { instance } = await importChunk(() => import("@viz-js/viz")); const viz = await instance(); if (cancelled) return; const svg = viz.renderSVGElement(dot); diff --git a/apps/fabro-web/app/components/toast.tsx b/apps/fabro-web/app/components/toast.tsx index afa57fa90..a6c3646e0 100644 --- a/apps/fabro-web/app/components/toast.tsx +++ b/apps/fabro-web/app/components/toast.tsx @@ -9,10 +9,17 @@ import { Toaster as SonnerToaster, toast as sonnerToast, useSonner } from "sonne export type ToastTone = "info" | "error"; +export interface ToastAction { + label: string; + onClick: () => void; +} + export interface ToastInput { message: string; tone?: ToastTone; + /** Pass `Infinity` for a toast that stays until dismissed or acted on. */ autoDismissMs?: number; + action?: ToastAction; } interface ToastContextValue { @@ -27,6 +34,14 @@ function push(toast: ToastInput): string { const id = `toast-${nextToastId++}`; const options = { id, + ...(toast.action + ? { + action: { + label: toast.action.label, + onClick: toast.action.onClick, + }, + } + : {}), ...(toast.tone === "error" ? { duration: Infinity } : toast.autoDismissMs != null diff --git a/apps/fabro-web/app/hooks/use-build-version-guard.ts b/apps/fabro-web/app/hooks/use-build-version-guard.ts new file mode 100644 index 000000000..8c35d9a92 --- /dev/null +++ b/apps/fabro-web/app/hooks/use-build-version-guard.ts @@ -0,0 +1,49 @@ +import { useEffect, useRef, useState } from "react"; + +import { useToast } from "../components/toast"; +import { + documentBuildId, + isStaleBuild, + useLatestBuildId, +} from "../lib/build-version"; + +const NEW_VERSION_MESSAGE = "A new version of Fabro is available."; + +/** + * Synchronizes a reload prompt with the build id the server is publishing. + * + * Client-side routing never re-fetches `index.html`, and hashed bundles are + * served `immutable`, so a tab left open across a deploy keeps running the + * previous build's JavaScript indefinitely with nothing to reveal it. This + * offers a reload when that happens; it never reloads on its own. + */ +export function useBuildVersionGuard(): void { + const { push } = useToast(); + // Read once. It describes the document this tab loaded, which cannot change + // without a full page load — and that remounts the hook anyway. + const [loadedBuildId] = useState(documentBuildId); + const latestBuildId = useLatestBuildId(); + const promptedForRef = useRef(null); + + const stale = isStaleBuild(loadedBuildId, latestBuildId); + + useEffect(() => { + if (!stale || !latestBuildId) return; + // One prompt per distinct build. Repeated polls of the same new build must + // not re-nag, but dismissing this one must not suppress a later, genuinely + // different build. + if (promptedForRef.current === latestBuildId) return; + promptedForRef.current = latestBuildId; + + push({ + message: NEW_VERSION_MESSAGE, + // Persistent by design: a prompt that vanishes after a few seconds is one + // the user will miss, which is the whole failure this exists to fix. + autoDismissMs: Infinity, + action: { + label: "Reload", + onClick: () => window.location.reload(), + }, + }); + }, [latestBuildId, push, stale]); +} diff --git a/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts b/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts index 9733f335e..f647b54c4 100644 --- a/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts +++ b/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts @@ -1,5 +1,7 @@ import { useEffect, useState } from "react"; +import { importChunk } from "../lib/import-chunk"; + /** * Synchronizes a DOT source with the imperative @viz-js SVG renderer and a DOM * container. Async renders are ignored after identity changes or unmount. @@ -27,7 +29,7 @@ export function useRenderedVizDiagram({ async function render() { setError(null); onRenderStart?.(); - const { instance } = await import("@viz-js/viz"); + const { instance } = await importChunk(() => import("@viz-js/viz")); const viz = await instance(); if (cancelled) return; diff --git a/apps/fabro-web/app/hooks/use-terminal-session.ts b/apps/fabro-web/app/hooks/use-terminal-session.ts index c8476f4e8..d8c7c751c 100644 --- a/apps/fabro-web/app/hooks/use-terminal-session.ts +++ b/apps/fabro-web/app/hooks/use-terminal-session.ts @@ -6,6 +6,7 @@ import { buildTerminalWebSocketUrl, parseTerminalServerMessage, } from "../components/terminal-view-helpers"; +import { importChunk } from "../lib/import-chunk"; export type ConnectionStatus = "connecting" | "ready" | "closed" | "error"; @@ -93,8 +94,8 @@ export function useTerminalSession({ setError(null); const [{ Terminal }, { FitAddon }] = await Promise.all([ - import("@xterm/xterm"), - import("@xterm/addon-fit"), + importChunk(() => import("@xterm/xterm")), + importChunk(() => import("@xterm/addon-fit")), ]); if (disposed || !terminalEl.current) return; diff --git a/apps/fabro-web/app/layouts/app-shell.tsx b/apps/fabro-web/app/layouts/app-shell.tsx index c8a758018..24c9c84c5 100644 --- a/apps/fabro-web/app/layouts/app-shell.tsx +++ b/apps/fabro-web/app/layouts/app-shell.tsx @@ -15,6 +15,7 @@ import { Link, Outlet, useLocation, useMatches } from "react-router"; import { FabroToaster } from "../components/toast"; import { ErrorState } from "../components/state"; import { TooltipProvider } from "../components/ui"; +import { useBuildVersionGuard } from "../hooks/use-build-version-guard"; import { DemoModeProvider } from "../lib/demo-mode"; import { useAuthMe } from "../lib/queries"; import { navigation } from "./navigation"; @@ -31,6 +32,9 @@ export default function AppShell() { const { data: auth, error, isLoading } = useAuthMe(); const { pathname } = useLocation(); const matches = useMatches(); + // Before the early returns below, so the check keeps running while the shell + // is in its loading or error state. + useBuildVersionGuard(); if (isLoading && !auth) { return
; diff --git a/apps/fabro-web/app/lib/build-version.test.ts b/apps/fabro-web/app/lib/build-version.test.ts new file mode 100644 index 000000000..5d2022d92 --- /dev/null +++ b/apps/fabro-web/app/lib/build-version.test.ts @@ -0,0 +1,59 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { fetchBuildId, isStaleBuild } from "./build-version"; + +describe("isStaleBuild", () => { + test("reports stale only when both ids are known and differ", () => { + expect(isStaleBuild("abc", "def")).toBe(true); + expect(isStaleBuild("abc", "abc")).toBe(false); + }); + + // A false "new version" claim is worse than a missed one: it trains people to + // ignore the toast. Anything unknown must stay silent. + test("stays silent when either side is unknown", () => { + expect(isStaleBuild(null, "def")).toBe(false); + expect(isStaleBuild("abc", null)).toBe(false); + expect(isStaleBuild(null, null)).toBe(false); + expect(isStaleBuild("", "def")).toBe(false); + }); +}); + +describe("fetchBuildId", () => { + const realFetch = globalThis.fetch; + afterEach(() => { + globalThis.fetch = realFetch; + }); + + function stubFetch(response: { ok: boolean; body?: unknown }) { + globalThis.fetch = (async () => ({ + ok: response.ok, + json: async () => response.body, + })) as unknown as typeof fetch; + } + + test("returns the published build id", async () => { + stubFetch({ ok: true, body: { buildId: "8f2yqj8q" } }); + expect(await fetchBuildId("/build-id.json")).toBe("8f2yqj8q"); + }); + + test("returns null for a non-ok response", async () => { + stubFetch({ ok: false }); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + }); + + // A server that returns something unexpected must not be read as "a new + // build shipped" — that would fire the toast on every poll. + test("returns null for a malformed body", async () => { + stubFetch({ ok: true, body: { buildId: 42 } }); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + + stubFetch({ ok: true, body: {} }); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + + stubFetch({ ok: true, body: null }); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + + stubFetch({ ok: true, body: { buildId: "" } }); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + }); +}); diff --git a/apps/fabro-web/app/lib/build-version.ts b/apps/fabro-web/app/lib/build-version.ts new file mode 100644 index 000000000..29efa4792 --- /dev/null +++ b/apps/fabro-web/app/lib/build-version.ts @@ -0,0 +1,68 @@ +import useSWR from "swr"; + +/** Where `scripts/build.ts` publishes the id of the build being served. */ +const BUILD_ID_URL = "/build-id.json"; + +/** + * How often a visible tab re-checks. SWR does not poll while the document is + * hidden (`refreshWhenHidden` defaults to false), so background tabs stay + * silent without any extra gating, and a hidden tab revalidates on focus. + */ +const POLL_INTERVAL_MS = 60_000; + +export const BUILD_ID_META_NAME = "fabro-build-id"; + +/** + * The build this document loaded, from the meta tag `scripts/build.ts` writes + * into `index.html`. + * + * The meta tag is the honest source for "what is this tab running": client-side + * routing never re-fetches `index.html`, so it stays pinned to the build the + * tab actually started with, however long the tab lives. + */ +export function documentBuildId(): string | null { + if (typeof document === "undefined") return null; + const content = document + .querySelector(`meta[name="${BUILD_ID_META_NAME}"]`) + ?.getAttribute("content") + ?.trim(); + return content ? content : null; +} + +export async function fetchBuildId(url: string): Promise { + // Served `no-cache` with an ETag, so the browser revalidates and normally + // gets a 304 rather than a fresh body. + const response = await fetch(url); + if (!response.ok) return null; + const body: unknown = await response.json(); + const buildId = (body as { buildId?: unknown } | null)?.buildId; + return typeof buildId === "string" && buildId ? buildId : null; +} + +/** + * True only when the running document is provably behind what the server is + * serving now. + * + * Unknown on either side means "claim nothing". A missing meta tag (a build + * predating this feature, or a non-DOM test environment) or a failed fetch must + * never produce a reload prompt — a false "new version" claim is worse than a + * missed one, because it teaches people to ignore the real ones. + */ +export function isStaleBuild( + loaded: string | null, + latest: string | null, +): boolean { + if (!loaded || !latest) return false; + return loaded !== latest; +} + +/** Synchronizes React with the build id the server is currently publishing. */ +export function useLatestBuildId(): string | null { + const { data } = useSWR(BUILD_ID_URL, fetchBuildId, { + refreshInterval: POLL_INTERVAL_MS, + revalidateOnFocus: true, + // A failed check is not worth retry storms; the next poll covers it. + shouldRetryOnError: false, + }); + return data ?? null; +} diff --git a/apps/fabro-web/app/lib/import-chunk.test.ts b/apps/fabro-web/app/lib/import-chunk.test.ts new file mode 100644 index 000000000..87939d224 --- /dev/null +++ b/apps/fabro-web/app/lib/import-chunk.test.ts @@ -0,0 +1,132 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { importChunk } from "./import-chunk"; + +interface WindowStub { + reloads: number; + store: Map; + restore: () => void; +} + +/** + * bun:test runs without a DOM, so install a `window` carrying just the surface + * `importChunk` touches. Follows the descriptor save/restore pattern used by + * stage-insights-sidebar.test.tsx so other test files can install their own. + */ +function installWindow({ throwOnStorage = false } = {}): WindowStub { + const store = new Map(); + const stub: WindowStub = { + reloads: 0, + store, + restore: () => undefined, + }; + + const windowStub = { + sessionStorage: { + getItem: (key: string) => { + if (throwOnStorage) throw new Error("storage disabled"); + return store.get(key) ?? null; + }, + setItem: (key: string, value: string) => { + if (throwOnStorage) throw new Error("storage disabled"); + store.set(key, value); + }, + }, + location: { + reload: () => { + stub.reloads += 1; + }, + }, + }; + + const had = "window" in globalThis; + const prev = (globalThis as { window?: unknown }).window; + Object.defineProperty(globalThis, "window", { + value: windowStub, + writable: true, + configurable: true, + }); + stub.restore = () => { + if (had) { + Object.defineProperty(globalThis, "window", { + value: prev, + writable: true, + configurable: true, + }); + } else { + delete (globalThis as { window?: unknown }).window; + } + }; + return stub; +} + +let installed: WindowStub | null = null; +afterEach(() => { + installed?.restore(); + installed = null; +}); + +describe("importChunk", () => { + test("passes a successful import through untouched", async () => { + installed = installWindow(); + await expect(importChunk(async () => "loaded")).resolves.toBe("loaded"); + expect(installed.reloads).toBe(0); + }); + + test("reloads once and rethrows when a chunk fails to load", async () => { + installed = installWindow(); + const failure = new Error("Failed to fetch dynamically imported module"); + + await expect( + importChunk(async () => { + throw failure; + }), + ).rejects.toThrow(failure); + + expect(installed.reloads).toBe(1); + }); + + // Without this, a chunk that fails for a reason a reload cannot fix would + // reload forever. + test("does not reload again for the same build", async () => { + installed = installWindow(); + const load = async () => { + throw new Error("Failed to fetch dynamically imported module"); + }; + + await expect(importChunk(load)).rejects.toThrow(); + await expect(importChunk(load)).rejects.toThrow(); + await expect(importChunk(load)).rejects.toThrow(); + + expect(installed.reloads).toBe(1); + }); + + // The marker is keyed by build id, so a tab that recovers from one deploy + // still has a reload available for the next. + test("keys the once-only marker by build id", async () => { + installed = installWindow(); + await expect( + importChunk(async () => { + throw new Error("boom"); + }), + ).rejects.toThrow(); + + expect([...installed.store.keys()]).toEqual([ + "fabro:chunk-reload:unknown", + ]); + }); + + // No durable marker means no way to promise "only once", and a reload loop is + // far worse than a surfaced error. + test("does not reload when session storage is unavailable", async () => { + installed = installWindow({ throwOnStorage: true }); + + await expect( + importChunk(async () => { + throw new Error("boom"); + }), + ).rejects.toThrow(); + + expect(installed.reloads).toBe(0); + }); +}); diff --git a/apps/fabro-web/app/lib/import-chunk.ts b/apps/fabro-web/app/lib/import-chunk.ts new file mode 100644 index 000000000..fe56eaaf9 --- /dev/null +++ b/apps/fabro-web/app/lib/import-chunk.ts @@ -0,0 +1,54 @@ +import { documentBuildId } from "./build-version"; + +const RELOAD_MARKER_PREFIX = "fabro:chunk-reload:"; + +/** + * Loads a lazily-imported chunk, reloading the page once if it cannot be + * fetched. + * + * Each deploy replaces the served assets and the previous build's hashed + * filenames stop existing, so a tab open across a deploy can request a chunk + * that now 404s. Static route imports mean most of the graph is already in + * memory, but the handful of genuinely lazy imports — the terminal, Graphviz + * rendering, the file tree — are loaded on demand and can land in that window. + * + * A failed chunk means the feature is already broken, so reloading is recovery + * rather than an interruption. This is the one place the app reloads without an + * explicit click; the build-version toast never does. + */ +export function importChunk(load: () => Promise): Promise { + return load().catch((error: unknown) => { + reloadOnceForStaleChunk(); + // Rethrow rather than returning a never-settling promise. The reload + // normally replaces the document before this surfaces; if it doesn't, an + // error boundary is a better outcome than a spinner that hangs forever. + throw error; + }); +} + +/** + * Reloads at most once per build. + * + * Keyed by build id rather than a bare flag so a tab that recovers from one + * deploy still has a reload available for the next one. Without the key, a + * single chunk failure would disarm the backstop for the rest of the session. + * + * A module that loads fine but throws while evaluating is indistinguishable + * here from a missing chunk, so it also spends the reload. The per-build key + * bounds the cost at one wasted reload, after which the real error surfaces. + */ +function reloadOnceForStaleChunk(): void { + if (typeof window === "undefined") return; + + const key = `${RELOAD_MARKER_PREFIX}${documentBuildId() ?? "unknown"}`; + try { + if (window.sessionStorage.getItem(key)) return; + window.sessionStorage.setItem(key, "1"); + } catch { + // Storage disabled or full. Without a durable marker we can't guarantee + // "only once", and a reload loop is far worse than a surfaced error. + return; + } + + window.location.reload(); +} diff --git a/apps/fabro-web/app/routes/run-files.tsx b/apps/fabro-web/app/routes/run-files.tsx index a2f3a1a25..aba185f1f 100644 --- a/apps/fabro-web/app/routes/run-files.tsx +++ b/apps/fabro-web/app/routes/run-files.tsx @@ -16,6 +16,7 @@ import { type FileContents, } from "@pierre/diffs/react"; import { useToast } from "../components/toast"; +import { importChunk } from "../lib/import-chunk"; import type { FileDiff as ApiFileDiff, PaginatedRunFileList, @@ -58,9 +59,11 @@ import { useTickingNow } from "../lib/time"; export { extractRequestId }; const FileTreeSidebar = lazy(() => - import("./run-files/file-tree-sidebar").then((module) => ({ - default: module.FileTreeSidebar, - })), + importChunk(() => + import("./run-files/file-tree-sidebar").then((module) => ({ + default: module.FileTreeSidebar, + })), + ), ); export const handle = { wide: true, fullHeight: true }; diff --git a/apps/fabro-web/index.template.html b/apps/fabro-web/index.template.html index 5ebdb65b9..eab794756 100644 --- a/apps/fabro-web/index.template.html +++ b/apps/fabro-web/index.template.html @@ -14,6 +14,7 @@ href="https://fonts.googleapis.com/css2?family=Geist:wght@100..900&family=JetBrains+Mono:wght@400;500;600&display=swap" /> {{styles}} + {{buildMeta}}
diff --git a/apps/fabro-web/scripts/build.test.ts b/apps/fabro-web/scripts/build.test.ts index e3e852a71..fafc79491 100644 --- a/apps/fabro-web/scripts/build.test.ts +++ b/apps/fabro-web/scripts/build.test.ts @@ -1,6 +1,6 @@ import { test, expect } from "bun:test"; import { existsSync } from "node:fs"; -import { lstat, readdir, readlink } from "node:fs/promises"; +import { lstat, readFile, readdir, readlink } from "node:fs/promises"; import { basename, join } from "node:path"; const root = Bun.fileURLToPath(new URL("..", import.meta.url)); @@ -64,6 +64,64 @@ test("dist is a symlink into .dist-builds and old builds are pruned", async () = expect(existsSync(join(distPath, "index.html"))).toBe(true); }, 60000); +test("publishes a build id that index.html and build-id.json agree on", async () => { + await runBuild(); + + const distPath = join(root, "dist"); + const published = JSON.parse( + await readFile(join(distPath, "build-id.json"), "utf8"), + ) as { buildId: string }; + + expect(published.buildId).toMatch(/^[a-z0-9]{8}$/); + + const html = await readFile(join(distPath, "index.html"), "utf8"); + expect(html).toContain( + ``, + ); +}, 60000); + +// The id is derived from source inputs rather than emitted filenames precisely +// so this holds: Bun's minified identifier naming is not deterministic, so the +// entry bundle's content hash changes between builds of an unchanged tree +// roughly one run in three. An id that moved with it would fire the client's +// "new version" toast on redeploys of identical code. +test("build id is stable across rebuilds of an unchanged tree", async () => { + const distPath = join(root, "dist"); + const readBuildId = async () => + ( + JSON.parse(await readFile(join(distPath, "build-id.json"), "utf8")) as { + buildId: string; + } + ).buildId; + + await runBuild(); + const first = await readBuildId(); + await runBuild(); + const second = await readBuildId(); + + expect(second).toBe(first); +}, 120000); + +// A stable-named stylesheet is served `no-cache`, letting a tab revalidate into +// new CSS while running old JS; Tailwind purges per build, so classes the old +// bundle still emits can vanish. The hash must match the `[a-z0-9]{8}` shape +// `is_content_hashed` in static_files.rs keys on. +test("stylesheet is content-hashed and referenced from index.html", async () => { + await runBuild(); + + const distPath = join(root, "dist"); + const assets = await readdir(join(distPath, "assets")); + const stylesheets = assets.filter((file) => /^app-.*\.css$/.test(file)); + + expect(stylesheets).toHaveLength(1); + expect(stylesheets[0]).toMatch(/^app-[a-z0-9]{8}\.css$/); + expect(assets).not.toContain("app.css"); + + const html = await readFile(join(distPath, "index.html"), "utf8"); + expect(html).toContain(`href="/assets/${stylesheets[0]}"`); + expect(html).not.toContain('href="/assets/app.css"'); +}, 60000); + test("watch mode keeps running until interrupted", async () => { const process = Bun.spawn([ "bun", diff --git a/apps/fabro-web/scripts/build.ts b/apps/fabro-web/scripts/build.ts index 78e520d29..1a9845128 100644 --- a/apps/fabro-web/scripts/build.ts +++ b/apps/fabro-web/scripts/build.ts @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { watch as fsWatch } from "node:fs"; import { cp, @@ -34,13 +35,103 @@ const tailwindCliBin = join( JSON.parse(await readFile(tailwindCliPackageJsonPath, "utf8")).bin.tailwindcss, ); -function newBuildId(): string { +// Names the `.dist-builds/` staging directory only. Time-ordered so builds sort +// chronologically on disk, and unique so concurrent builds never collide. This +// is deliberately NOT the id published to browsers: see `publishedBuildId`. +function newBuildDirName(): string { return `${Date.now()}-${Math.random().toString(36).slice(2, 10)}`; } +/** + * Lowercase-alphanumeric 8-char digest, matching the `[a-z0-9]{8}` shape the + * bundler uses for its own content hashes — and which the server's cache + * classifier (`is_content_hashed` in `static_files.rs`) keys on to decide + * between `immutable` and `no-cache`. + */ +function toShortId(hex: string): string { + return BigInt(`0x${hex.slice(0, 32)}`) + .toString(36) + .padStart(8, "0") + .slice(0, 8); +} + +function contentHash8(content: string | Uint8Array): string { + return toShortId(createHash("sha256").update(content).digest("hex")); +} + +// Everything that determines what the bundle contains. `bun.lock` lives at the +// workspace root, so a dependency bump changes the id even though no file under +// `app/` moved. +const BUILD_INPUT_DIRS = ["app", "public"]; +const BUILD_INPUT_FILES = [ + "index.template.html", + "package.json", + "scripts/build.ts", + "../../bun.lock", +]; + +/** + * The build id published to browsers, derived from the bundle's *source inputs*. + * + * The obvious implementation — hash the emitted asset filenames, which already + * embed content hashes — does not work, because **Bun's minified identifier + * naming is not deterministic**. Building this app twice from an unchanged tree + * produces byte-different output roughly one run in three: same length, ~100k + * differing bytes, all of it mangled names (`var Gr=C3((Pl5,qq)=>` in one run, + * `var yr=C3((Uc5,Oq)=>` in the next). Output hashes therefore change without + * any source change. + * + * That matters because the client shows a "new version" toast on mismatch. An id + * that flips at random would fire the toast on redeploys of identical code and + * train people to ignore it, which is worse than having no toast at all. Hashing + * the inputs makes the id change if and only if something we actually control + * changed. + * + * The tradeoff: when Bun emits a different permutation for the same source, the + * asset filenames change while the build id does not, so an open tab isn't told + * to reload. That is the correct call — the two builds are the same program — + * and `importChunk` covers the case where such a tab later needs a chunk whose + * name moved. + */ +async function publishedBuildId(): Promise { + const files: string[] = []; + for (const dir of BUILD_INPUT_DIRS) { + files.push(...(await collectFilesRecursively(join(rootPath, dir)))); + } + for (const file of BUILD_INPUT_FILES) { + files.push(join(rootPath, file)); + } + + const digest = createHash("sha256"); + // The bundler itself is an input: a Bun upgrade can change output semantics. + digest.update(`bun:${Bun.version}\n`); + for (const file of files.sort()) { + // Hash the repo-relative path, not the absolute one, so the id doesn't + // depend on where the repo is checked out. + digest.update(relative(rootPath, file)); + digest.update("\0"); + digest.update(createHash("sha256").update(await readFile(file)).digest()); + } + return toShortId(digest.digest("hex")); +} + +async function collectFilesRecursively(dir: string): Promise { + const collected: string[] = []; + const entries = await readdir(dir, { withFileTypes: true }); + for (const entry of entries) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + collected.push(...(await collectFilesRecursively(full))); + } else if (entry.isFile()) { + collected.push(full); + } + } + return collected; +} + async function buildOnce() { - const buildId = newBuildId(); - const buildDir = join(buildsRootDir, buildId); + const buildDirName = newBuildDirName(); + const buildDir = join(buildsRootDir, buildDirName); const buildAssetsDir = join(buildDir, "assets"); await mkdir(buildAssetsDir, { recursive: true }); @@ -75,18 +166,48 @@ async function buildOnce() { throw new Error("Tailwind build failed"); } + const stylesheetPath = await hashStylesheet(buildDir, buildAssetsDir); + await cp(publicDir, buildDir, { recursive: true }); await copyPierreWorkerAssets(join(buildAssetsDir, "pierre-diffs-worker")); - await writeIndexHtml( - buildDir, - result.outputs.map((output: any) => ({ - kind: output.kind, - path: relative(buildDir, output.path), - })), + + const outputs: IndexHtmlOutput[] = result.outputs.map((output: any) => ({ + kind: output.kind, + path: relative(buildDir, output.path), + })); + const buildId = await publishedBuildId(); + + await writeIndexHtml(buildDir, outputs, stylesheetPath, buildId); + // Served with `no-cache` + ETag (it doesn't match the server's content-hash + // pattern), so a polling client revalidates it as a cheap 304. + await writeFile( + join(buildDir, "build-id.json"), + `${JSON.stringify({ buildId }, null, 2)}\n`, + "utf8", ); await publishBuild(buildDir); - await pruneOldBuilds(buildId); + await pruneOldBuilds(buildDirName); +} + +/** + * Renames Tailwind's stable-named `app.css` to `app-.css`. + * + * A stable name forces `no-cache`, which lets a tab revalidate into the new + * stylesheet while still running the previous build's JavaScript. Tailwind + * purges unused classes per build, so classes the old JS still emits can vanish + * from the new CSS and elements silently render unstyled. Hashing pins the two + * together and lets the server cache the stylesheet immutably. + */ +async function hashStylesheet( + buildDir: string, + buildAssetsDir: string, +): Promise { + const source = join(buildAssetsDir, "app.css"); + const css = await readFile(source); + const hashedName = `app-${contentHash8(css)}.css`; + await rename(source, join(buildAssetsDir, hashedName)); + return relative(buildDir, join(buildAssetsDir, hashedName)); } async function copyPierreWorkerAssets(targetDir: string) { @@ -110,7 +231,12 @@ type IndexHtmlOutput = { path: string; }; -async function writeIndexHtml(buildDir: string, outputs: IndexHtmlOutput[]) { +async function writeIndexHtml( + buildDir: string, + outputs: IndexHtmlOutput[], + stylesheetPath: string, + buildId: string, +) { const template = await readFile(templatePath, "utf8"); // Only entry points get `) .join("\n "); const styles = [ - "/assets/app.css", + `/${stylesheetPath.replaceAll("\\\\", "/")}`, ...outputs .filter((output) => output.path.endsWith(".css")) .map((output) => `/${output.path.replaceAll("\\\\", "/")}`), @@ -132,8 +258,15 @@ async function writeIndexHtml(buildDir: string, outputs: IndexHtmlOutput[]) { .map((path) => ``) .join("\n "); + // Records which build this document loaded. A tab reads it back at runtime + // and compares against /build-id.json; the meta tag is the honest answer + // because client-side routing never re-fetches index.html, so it stays + // pinned to the build the tab actually started with. + const buildMeta = ``; + const html = template .replace("{{styles}}", styles) + .replace("{{buildMeta}}", buildMeta) .replace("{{scripts}}", scripts); await writeFile(join(buildDir, "index.html"), html, "utf8"); diff --git a/lib/apps/fabro-server/src/static_files.rs b/lib/apps/fabro-server/src/static_files.rs index b0a9e623e..67db55a4e 100644 --- a/lib/apps/fabro-server/src/static_files.rs +++ b/lib/apps/fabro-server/src/static_files.rs @@ -473,6 +473,10 @@ mod tests { "assets/entry-0sv53bs3.js", "assets/chunk-4tr91ktd.js", "assets/chunk-x912wb67.css", + // Tailwind's stylesheet is hashed by the build so it moves with the + // bundle; a stable name would let a tab revalidate into new CSS + // while still running the previous build's JavaScript. + "assets/app-381qtfxr.css", ] { assert!(is_content_hashed(path), "{path} should be content-hashed"); } @@ -485,6 +489,9 @@ mod tests { // pinned stale in browsers for a year if marked immutable. for path in [ "index.html", + // Clients poll this to learn whether their tab is running a stale + // build, so it must revalidate rather than be pinned for a year. + "build-id.json", "assets/app.css", "assets/pierre-diffs-worker/worker-portable.js", "images/apple-touch-icon.png", From 2902b8c77363f11385f71786fc4f2f7ec08b7467 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 25 Jul 2026 15:15:25 -0400 Subject: [PATCH 3/3] fix(web): harden build version detection --- apps/fabro-web/app/components/toast.tsx | 15 +- apps/fabro-web/app/entry.tsx | 15 +- .../app/hooks/dynamic-import-errors.test.tsx | 95 ++++++++ .../hooks/use-build-version-guard.test.tsx | 132 +++++++++++ .../app/hooks/use-build-version-guard.ts | 28 ++- .../app/hooks/use-rendered-viz-diagram.ts | 13 +- .../app/hooks/use-terminal-session.ts | 44 +++- apps/fabro-web/app/layouts/app-shell.tsx | 8 - .../app/lib/build-version-contract.ts | 22 ++ apps/fabro-web/app/lib/build-version.test.ts | 46 ++-- apps/fabro-web/app/lib/build-version.ts | 20 +- apps/fabro-web/app/lib/import-chunk.test.ts | 54 ++++- apps/fabro-web/app/routes/run-terminal.tsx | 3 +- apps/fabro-web/scripts/build.test.ts | 118 +++++----- apps/fabro-web/scripts/build.ts | 213 +++++++++++------- 15 files changed, 597 insertions(+), 229 deletions(-) create mode 100644 apps/fabro-web/app/hooks/dynamic-import-errors.test.tsx create mode 100644 apps/fabro-web/app/hooks/use-build-version-guard.test.tsx create mode 100644 apps/fabro-web/app/lib/build-version-contract.ts diff --git a/apps/fabro-web/app/components/toast.tsx b/apps/fabro-web/app/components/toast.tsx index a6c3646e0..02bfe9294 100644 --- a/apps/fabro-web/app/components/toast.tsx +++ b/apps/fabro-web/app/components/toast.tsx @@ -34,14 +34,7 @@ function push(toast: ToastInput): string { const id = `toast-${nextToastId++}`; const options = { id, - ...(toast.action - ? { - action: { - label: toast.action.label, - onClick: toast.action.onClick, - }, - } - : {}), + ...(toast.action ? { action: toast.action } : {}), ...(toast.tone === "error" ? { duration: Infinity } : toast.autoDismissMs != null @@ -70,9 +63,9 @@ const toastApi: ToastContextValue = { /** * No-op wrapper retained so existing test harnesses and the standalone terminal * route can keep their mount points. In a browser the real - * is mounted globally in AppShell; in non-DOM test environments we - * render an aria-live fallback that subscribes to the Sonner store so test - * assertions can read the toast text. + * is mounted globally at the entry point; in non-DOM test + * environments we render an aria-live fallback that subscribes to the Sonner + * store so test assertions can read the toast text. */ export function ToastProvider({ children }: { children: ReactNode }) { if (typeof document !== "undefined") { diff --git a/apps/fabro-web/app/entry.tsx b/apps/fabro-web/app/entry.tsx index ff9d681f9..a3488b680 100644 --- a/apps/fabro-web/app/entry.tsx +++ b/apps/fabro-web/app/entry.tsx @@ -2,6 +2,8 @@ import { StrictMode } from "react"; import { createRoot } from "react-dom/client"; import { createBrowserRouter, RouterProvider } from "react-router"; import { SWRConfig } from "swr"; +import { FabroToaster } from "./components/toast"; +import { useBuildVersionGuard } from "./hooks/use-build-version-guard"; import { installRoutes } from "./install-router"; import { resolveFabroMode } from "./mode"; import { routes } from "./router"; @@ -21,6 +23,17 @@ if (!rootElement) { throw new Error("Missing #root element"); } +function AppRuntime() { + useBuildVersionGuard(); + + return ( + <> + + + + ); +} + createRoot(rootElement).render( - + , ); diff --git a/apps/fabro-web/app/hooks/dynamic-import-errors.test.tsx b/apps/fabro-web/app/hooks/dynamic-import-errors.test.tsx new file mode 100644 index 000000000..1add46d1b --- /dev/null +++ b/apps/fabro-web/app/hooks/dynamic-import-errors.test.tsx @@ -0,0 +1,95 @@ +import { afterEach, beforeEach, expect, mock, test } from "bun:test"; +import { useState } from "react"; +import TestRenderer, { act } from "react-test-renderer"; + +import { setupReactTestEnv } from "../lib/test-utils"; +import type { + ConnectionStatus, + TerminalConnectionError, +} from "./use-terminal-session"; + +const importFailure = new Error("chunk unavailable"); + +mock.module("../lib/import-chunk", () => ({ + importChunk: async () => { + throw importFailure; + }, +})); + +const [{ useRenderedVizDiagram }, { useTerminalSession }] = await Promise.all([ + import("./use-rendered-viz-diagram"), + import("./use-terminal-session"), +]); + +let renderer: TestRenderer.ReactTestRenderer | null = null; +let restoreReactTestEnv = () => {}; + +beforeEach(() => { + restoreReactTestEnv = setupReactTestEnv(); +}); + +afterEach(() => { + act(() => { + renderer?.unmount(); + }); + renderer = null; + restoreReactTestEnv(); +}); + +async function renderAndFlush(element: React.ReactElement) { + await act(async () => { + renderer = TestRenderer.create(element); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +const diagramContainerRef = { current: null }; +const diagramSvgRef = { current: null }; +const buildDot = () => "digraph {}"; +let diagramError: string | null = null; + +function DiagramHost() { + diagramError = useRenderedVizDiagram({ + buildDot, + innerRef: diagramContainerRef, + identity: "diagram", + svgRef: diagramSvgRef, + }); + return null; +} + +test("diagram import failures populate the hook error instead of rejecting", async () => { + await renderAndFlush(); + expect(diagramError).toBe("chunk unavailable"); +}); + +const terminalElementRef = { + current: {} as HTMLDivElement, +}; +let terminalError: TerminalConnectionError | null = null; +let terminalStatus: ConnectionStatus = "closed"; + +function TerminalHost() { + const [error, setError] = useState(null); + const [status, setStatus] = useState("closed"); + terminalError = error; + terminalStatus = status; + useTerminalSession({ + connectionKey: 0, + runId: "run_1", + setError, + setStatus, + terminalEl: terminalElementRef, + }); + return null; +} + +test("terminal import failures leave a recoverable error state", async () => { + await renderAndFlush(); + expect(terminalStatus).toBe("error"); + expect(terminalError).toEqual({ + message: "Terminal initialization failed: chunk unavailable", + recoverable: true, + }); +}); diff --git a/apps/fabro-web/app/hooks/use-build-version-guard.test.tsx b/apps/fabro-web/app/hooks/use-build-version-guard.test.tsx new file mode 100644 index 000000000..bec6a7d36 --- /dev/null +++ b/apps/fabro-web/app/hooks/use-build-version-guard.test.tsx @@ -0,0 +1,132 @@ +import { afterEach, beforeEach, expect, mock, test } from "bun:test"; +import TestRenderer, { act } from "react-test-renderer"; +import { toast as sonnerToast } from "sonner"; + +import { setupReactTestEnv } from "../lib/test-utils"; + +const loadedBuildId = "aaaaaaaa"; +let latestBuildId: string | null = loadedBuildId; + +mock.module("../lib/build-version", () => ({ + documentBuildId: () => loadedBuildId, + isStaleBuild: (loaded: string | null, latest: string | null) => + loaded != null && latest != null && loaded !== latest, + useLatestBuildId: () => latestBuildId, +})); + +const { useBuildVersionGuard } = await import("./use-build-version-guard"); + +let renderer: TestRenderer.ReactTestRenderer | null = null; +let restoreReactTestEnv = () => {}; +let reloads = 0; +let previousWindow: unknown; +let hadWindow = false; +let previousRequestAnimationFrame: typeof requestAnimationFrame | undefined; + +function GuardHost({ revision }: { revision: number }) { + void revision; + useBuildVersionGuard(); + return null; +} + +function activeToasts() { + return sonnerToast.getToasts().filter((toast) => !toast.delete); +} + +function renderRevision(revision: number) { + act(() => { + if (renderer) { + renderer.update(); + } else { + renderer = TestRenderer.create(); + } + }); +} + +beforeEach(() => { + restoreReactTestEnv = setupReactTestEnv(); + latestBuildId = loadedBuildId; + reloads = 0; + hadWindow = "window" in globalThis; + previousWindow = (globalThis as { window?: unknown }).window; + previousRequestAnimationFrame = globalThis.requestAnimationFrame; + globalThis.requestAnimationFrame = (callback) => + setTimeout(callback, 0) as unknown as number; + Object.defineProperty(globalThis, "window", { + value: { + location: { + reload: () => { + reloads += 1; + }, + }, + }, + writable: true, + configurable: true, + }); +}); + +afterEach(() => { + act(() => { + renderer?.unmount(); + }); + renderer = null; + sonnerToast.dismiss(); + if (hadWindow) { + Object.defineProperty(globalThis, "window", { + value: previousWindow, + writable: true, + configurable: true, + }); + } else { + delete (globalThis as { window?: unknown }).window; + } + if (previousRequestAnimationFrame) { + globalThis.requestAnimationFrame = previousRequestAnimationFrame; + } else { + delete (globalThis as { requestAnimationFrame?: typeof requestAnimationFrame }) + .requestAnimationFrame; + } + restoreReactTestEnv(); +}); + +test("keeps one actionable prompt synchronized with the latest stale build", () => { + renderRevision(0); + expect(activeToasts()).toHaveLength(0); + + latestBuildId = "bbbbbbbb"; + renderRevision(1); + const firstPrompt = activeToasts(); + expect(firstPrompt).toHaveLength(1); + expect(firstPrompt[0]).toMatchObject({ + duration: Infinity, + title: "A new version of Fabro is available.", + }); + + const action = firstPrompt[0]?.action; + expect(action).toMatchObject({ label: "Reload" }); + if (action && typeof action === "object" && "onClick" in action) { + action.onClick({} as never); + } + expect(reloads).toBe(1); + + // Re-rendering the same poll result neither re-nags nor replaces the toast. + renderRevision(2); + expect(activeToasts()).toHaveLength(1); + expect(activeToasts()[0]?.id).toBe(firstPrompt[0]?.id); + + // Later deploys replace the active prompt instead of accumulating forever. + latestBuildId = "cccccccc"; + renderRevision(3); + expect(activeToasts()).toHaveLength(1); + expect(activeToasts()[0]?.id).not.toBe(firstPrompt[0]?.id); + + latestBuildId = "bbbbbbbb"; + renderRevision(4); + expect(activeToasts()).toHaveLength(1); + + // If the server rolls back to the document's build, the claim is no longer + // true and the prompt disappears. + latestBuildId = loadedBuildId; + renderRevision(5); + expect(activeToasts()).toHaveLength(0); +}); diff --git a/apps/fabro-web/app/hooks/use-build-version-guard.ts b/apps/fabro-web/app/hooks/use-build-version-guard.ts index 8c35d9a92..31367c164 100644 --- a/apps/fabro-web/app/hooks/use-build-version-guard.ts +++ b/apps/fabro-web/app/hooks/use-build-version-guard.ts @@ -18,24 +18,31 @@ const NEW_VERSION_MESSAGE = "A new version of Fabro is available."; * offers a reload when that happens; it never reloads on its own. */ export function useBuildVersionGuard(): void { - const { push } = useToast(); + const { dismiss, push } = useToast(); // Read once. It describes the document this tab loaded, which cannot change // without a full page load — and that remounts the hook anyway. const [loadedBuildId] = useState(documentBuildId); const latestBuildId = useLatestBuildId(); - const promptedForRef = useRef(null); + const promptRef = useRef<{ buildId: string; toastId: string } | null>(null); const stale = isStaleBuild(loadedBuildId, latestBuildId); useEffect(() => { - if (!stale || !latestBuildId) return; - // One prompt per distinct build. Repeated polls of the same new build must - // not re-nag, but dismissing this one must not suppress a later, genuinely - // different build. - if (promptedForRef.current === latestBuildId) return; - promptedForRef.current = latestBuildId; + if (!latestBuildId) return; + if (!stale) { + // A rollback to the document's own build makes an existing prompt false. + if (promptRef.current) { + dismiss(promptRef.current.toastId); + promptRef.current = null; + } + return; + } + if (promptRef.current?.buildId === latestBuildId) return; + if (promptRef.current) { + dismiss(promptRef.current.toastId); + } - push({ + const toastId = push({ message: NEW_VERSION_MESSAGE, // Persistent by design: a prompt that vanishes after a few seconds is one // the user will miss, which is the whole failure this exists to fix. @@ -45,5 +52,6 @@ export function useBuildVersionGuard(): void { onClick: () => window.location.reload(), }, }); - }, [latestBuildId, push, stale]); + promptRef.current = { buildId: latestBuildId, toastId }; + }, [dismiss, latestBuildId, push, stale]); } diff --git a/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts b/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts index f647b54c4..b9627b712 100644 --- a/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts +++ b/apps/fabro-web/app/hooks/use-rendered-viz-diagram.ts @@ -28,12 +28,13 @@ export function useRenderedVizDiagram({ async function render() { setError(null); - onRenderStart?.(); - const { instance } = await importChunk(() => import("@viz-js/viz")); - const viz = await instance(); - if (cancelled) return; try { + onRenderStart?.(); + const { instance } = await importChunk(() => import("@viz-js/viz")); + const viz = await instance(); + if (cancelled) return; + const svg = viz.renderSVGElement(buildDot(identity)); prepareSvg?.(svg); @@ -42,7 +43,9 @@ export function useRenderedVizDiagram({ innerRef.current.replaceChildren(svg); } } catch (e) { - setError(e instanceof Error ? e.message : "Failed to render diagram"); + if (!cancelled) { + setError(e instanceof Error ? e.message : "Failed to render diagram"); + } } } diff --git a/apps/fabro-web/app/hooks/use-terminal-session.ts b/apps/fabro-web/app/hooks/use-terminal-session.ts index d8c7c751c..963f00a0d 100644 --- a/apps/fabro-web/app/hooks/use-terminal-session.ts +++ b/apps/fabro-web/app/hooks/use-terminal-session.ts @@ -89,6 +89,25 @@ export function useTerminalSession({ const textEncoder = new TextEncoder(); const disposables: Array<{ dispose: () => void }> = []; + function disposeResources() { + resizeObserver?.disconnect(); + resizeObserver = null; + for (const disposable of disposables.splice(0)) disposable.dispose(); + + const socket = socketRef.current; + if (socket) { + if (socket.readyState === WebSocket.OPEN) { + socket.send(JSON.stringify({ type: "close" })); + } + socket.close(); + socketRef.current = null; + } + + terminalRef.current?.dispose(); + terminalRef.current = null; + fitRef.current = null; + } + async function connect() { setStatus("connecting"); setError(null); @@ -109,12 +128,12 @@ export function useTerminalSession({ theme: TERMINAL_THEME, }); const fitAddon = new FitAddon(); + terminalRef.current = terminal; + fitRef.current = fitAddon; terminal.loadAddon(fitAddon); terminal.open(terminalEl.current); fitAddon.fit(); terminal.focus(); - terminalRef.current = terminal; - fitRef.current = fitAddon; const socket = new WebSocket(buildTerminalWebSocketUrl(window.location, runId)); socket.binaryType = "arraybuffer"; @@ -191,18 +210,21 @@ export function useTerminalSession({ } } - void connect(); + void connect().catch((error: unknown) => { + if (disposed) return; + disposeResources(); + setStatus("error"); + setError({ + message: error instanceof Error + ? `Terminal initialization failed: ${error.message}` + : "Terminal initialization failed.", + recoverable: true, + }); + }); return () => { disposed = true; - resizeObserver?.disconnect(); - for (const disposable of disposables) disposable.dispose(); - socketRef.current?.send(JSON.stringify({ type: "close" })); - socketRef.current?.close(); - socketRef.current = null; - terminalRef.current?.dispose(); - terminalRef.current = null; - fitRef.current = null; + disposeResources(); }; }, [connectionKey, runId, setError, setStatus, terminalEl]); } diff --git a/apps/fabro-web/app/layouts/app-shell.tsx b/apps/fabro-web/app/layouts/app-shell.tsx index 24c9c84c5..6b610f941 100644 --- a/apps/fabro-web/app/layouts/app-shell.tsx +++ b/apps/fabro-web/app/layouts/app-shell.tsx @@ -12,10 +12,8 @@ import { XMarkIcon, } from "@heroicons/react/24/outline"; import { Link, Outlet, useLocation, useMatches } from "react-router"; -import { FabroToaster } from "../components/toast"; import { ErrorState } from "../components/state"; import { TooltipProvider } from "../components/ui"; -import { useBuildVersionGuard } from "../hooks/use-build-version-guard"; import { DemoModeProvider } from "../lib/demo-mode"; import { useAuthMe } from "../lib/queries"; import { navigation } from "./navigation"; @@ -32,9 +30,6 @@ export default function AppShell() { const { data: auth, error, isLoading } = useAuthMe(); const { pathname } = useLocation(); const matches = useMatches(); - // Before the early returns below, so the check keeps running while the shell - // is in its loading or error state. - useBuildVersionGuard(); if (isLoading && !auth) { return
; @@ -252,9 +247,6 @@ export default function AppShell() { )}
- {typeof document !== "undefined" && ( - - )} ); diff --git a/apps/fabro-web/app/lib/build-version-contract.ts b/apps/fabro-web/app/lib/build-version-contract.ts new file mode 100644 index 000000000..f9fb07059 --- /dev/null +++ b/apps/fabro-web/app/lib/build-version-contract.ts @@ -0,0 +1,22 @@ +import { getString } from "./unknown"; + +export const BUILD_ID_FILE_NAME = "build-id.json"; +export const BUILD_ID_URL = `/${BUILD_ID_FILE_NAME}`; +export const BUILD_ID_META_NAME = "fabro-build-id"; +export const BUILD_ID_FIELD = "buildId"; + +const BUILD_ID_PATTERN = /^[a-z0-9]{8}$/; + +export function parseBuildId(value: unknown): string | null { + if (typeof value !== "string") return null; + const normalized = value.trim(); + return BUILD_ID_PATTERN.test(normalized) ? normalized : null; +} + +export function parseBuildIdDocument(value: unknown): string | null { + return parseBuildId(getString(value, BUILD_ID_FIELD)); +} + +export function buildIdDocument(buildId: string): Record { + return { [BUILD_ID_FIELD]: buildId }; +} diff --git a/apps/fabro-web/app/lib/build-version.test.ts b/apps/fabro-web/app/lib/build-version.test.ts index 5d2022d92..59f10f9de 100644 --- a/apps/fabro-web/app/lib/build-version.test.ts +++ b/apps/fabro-web/app/lib/build-version.test.ts @@ -1,20 +1,21 @@ import { afterEach, describe, expect, test } from "bun:test"; import { fetchBuildId, isStaleBuild } from "./build-version"; +import { parseBuildId } from "./build-version-contract"; describe("isStaleBuild", () => { test("reports stale only when both ids are known and differ", () => { - expect(isStaleBuild("abc", "def")).toBe(true); - expect(isStaleBuild("abc", "abc")).toBe(false); + expect(isStaleBuild("aaaaaaaa", "bbbbbbbb")).toBe(true); + expect(isStaleBuild("aaaaaaaa", "aaaaaaaa")).toBe(false); }); // A false "new version" claim is worse than a missed one: it trains people to // ignore the toast. Anything unknown must stay silent. test("stays silent when either side is unknown", () => { - expect(isStaleBuild(null, "def")).toBe(false); - expect(isStaleBuild("abc", null)).toBe(false); + expect(isStaleBuild(null, "bbbbbbbb")).toBe(false); + expect(isStaleBuild("aaaaaaaa", null)).toBe(false); expect(isStaleBuild(null, null)).toBe(false); - expect(isStaleBuild("", "def")).toBe(false); + expect(isStaleBuild("", "bbbbbbbb")).toBe(false); }); }); @@ -24,36 +25,49 @@ describe("fetchBuildId", () => { globalThis.fetch = realFetch; }); - function stubFetch(response: { ok: boolean; body?: unknown }) { - globalThis.fetch = (async () => ({ - ok: response.ok, - json: async () => response.body, - })) as unknown as typeof fetch; + function stubFetch(response: Response) { + globalThis.fetch = async () => response; } test("returns the published build id", async () => { - stubFetch({ ok: true, body: { buildId: "8f2yqj8q" } }); + stubFetch(Response.json({ buildId: "8f2yqj8q" })); expect(await fetchBuildId("/build-id.json")).toBe("8f2yqj8q"); }); test("returns null for a non-ok response", async () => { - stubFetch({ ok: false }); + stubFetch(new Response(null, { status: 503 })); expect(await fetchBuildId("/build-id.json")).toBeNull(); }); // A server that returns something unexpected must not be read as "a new // build shipped" — that would fire the toast on every poll. test("returns null for a malformed body", async () => { - stubFetch({ ok: true, body: { buildId: 42 } }); + stubFetch(Response.json({ buildId: 42 })); expect(await fetchBuildId("/build-id.json")).toBeNull(); - stubFetch({ ok: true, body: {} }); + stubFetch(Response.json({})); expect(await fetchBuildId("/build-id.json")).toBeNull(); - stubFetch({ ok: true, body: null }); + stubFetch(Response.json(null)); expect(await fetchBuildId("/build-id.json")).toBeNull(); - stubFetch({ ok: true, body: { buildId: "" } }); + stubFetch(Response.json({ buildId: "" })); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + + stubFetch(Response.json({ buildId: "not-a-build-id" })); + expect(await fetchBuildId("/build-id.json")).toBeNull(); + + stubFetch(new Response("")); expect(await fetchBuildId("/build-id.json")).toBeNull(); }); }); + +describe("parseBuildId", () => { + test("normalizes valid ids and rejects values outside the wire format", () => { + expect(parseBuildId(" 8f2yqj8q ")).toBe("8f2yqj8q"); + expect(parseBuildId(" ")).toBeNull(); + expect(parseBuildId("8F2YQJ8Q")).toBeNull(); + expect(parseBuildId("abc123")).toBeNull(); + expect(parseBuildId(null)).toBeNull(); + }); +}); diff --git a/apps/fabro-web/app/lib/build-version.ts b/apps/fabro-web/app/lib/build-version.ts index 29efa4792..36cf54fc5 100644 --- a/apps/fabro-web/app/lib/build-version.ts +++ b/apps/fabro-web/app/lib/build-version.ts @@ -1,7 +1,11 @@ import useSWR from "swr"; -/** Where `scripts/build.ts` publishes the id of the build being served. */ -const BUILD_ID_URL = "/build-id.json"; +import { + BUILD_ID_META_NAME, + BUILD_ID_URL, + parseBuildId, + parseBuildIdDocument, +} from "./build-version-contract"; /** * How often a visible tab re-checks. SWR does not poll while the document is @@ -10,8 +14,6 @@ const BUILD_ID_URL = "/build-id.json"; */ const POLL_INTERVAL_MS = 60_000; -export const BUILD_ID_META_NAME = "fabro-build-id"; - /** * The build this document loaded, from the meta tag `scripts/build.ts` writes * into `index.html`. @@ -24,9 +26,8 @@ export function documentBuildId(): string | null { if (typeof document === "undefined") return null; const content = document .querySelector(`meta[name="${BUILD_ID_META_NAME}"]`) - ?.getAttribute("content") - ?.trim(); - return content ? content : null; + ?.getAttribute("content"); + return parseBuildId(content); } export async function fetchBuildId(url: string): Promise { @@ -34,9 +35,8 @@ export async function fetchBuildId(url: string): Promise { // gets a 304 rather than a fresh body. const response = await fetch(url); if (!response.ok) return null; - const body: unknown = await response.json(); - const buildId = (body as { buildId?: unknown } | null)?.buildId; - return typeof buildId === "string" && buildId ? buildId : null; + const body: unknown = await response.json().catch(() => null); + return parseBuildIdDocument(body); } /** diff --git a/apps/fabro-web/app/lib/import-chunk.test.ts b/apps/fabro-web/app/lib/import-chunk.test.ts index 87939d224..06a24ee5e 100644 --- a/apps/fabro-web/app/lib/import-chunk.test.ts +++ b/apps/fabro-web/app/lib/import-chunk.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { importChunk } from "./import-chunk"; interface WindowStub { + buildId: string | null; reloads: number; store: Map; restore: () => void; @@ -13,9 +14,16 @@ interface WindowStub { * `importChunk` touches. Follows the descriptor save/restore pattern used by * stage-insights-sidebar.test.tsx so other test files can install their own. */ -function installWindow({ throwOnStorage = false } = {}): WindowStub { +function installWindow({ + buildId = "aaaaaaaa", + throwOnStorage = false, +}: { + buildId?: string | null; + throwOnStorage?: boolean; +} = {}): WindowStub { const store = new Map(); const stub: WindowStub = { + buildId, reloads: 0, store, restore: () => undefined, @@ -41,11 +49,24 @@ function installWindow({ throwOnStorage = false } = {}): WindowStub { const had = "window" in globalThis; const prev = (globalThis as { window?: unknown }).window; + const hadDocument = "document" in globalThis; + const previousDocument = (globalThis as { document?: unknown }).document; Object.defineProperty(globalThis, "window", { value: windowStub, writable: true, configurable: true, }); + Object.defineProperty(globalThis, "document", { + value: { + querySelector: () => stub.buildId == null + ? null + : { + getAttribute: () => stub.buildId, + }, + }, + writable: true, + configurable: true, + }); stub.restore = () => { if (had) { Object.defineProperty(globalThis, "window", { @@ -56,6 +77,15 @@ function installWindow({ throwOnStorage = false } = {}): WindowStub { } else { delete (globalThis as { window?: unknown }).window; } + if (hadDocument) { + Object.defineProperty(globalThis, "document", { + value: previousDocument, + writable: true, + configurable: true, + }); + } else { + delete (globalThis as { document?: unknown }).document; + } }; return stub; } @@ -103,16 +133,24 @@ describe("importChunk", () => { // The marker is keyed by build id, so a tab that recovers from one deploy // still has a reload available for the next. - test("keys the once-only marker by build id", async () => { + test("allows one recovery reload for each loaded build id", async () => { installed = installWindow(); - await expect( - importChunk(async () => { - throw new Error("boom"); - }), - ).rejects.toThrow(); + const load = async () => { + throw new Error("boom"); + }; + await expect(importChunk(load)).rejects.toThrow(); + await expect(importChunk(load)).rejects.toThrow(); + expect(installed.reloads).toBe(1); + + installed.buildId = "bbbbbbbb"; + await expect(importChunk(load)).rejects.toThrow(); + await expect(importChunk(load)).rejects.toThrow(); + + expect(installed.reloads).toBe(2); expect([...installed.store.keys()]).toEqual([ - "fabro:chunk-reload:unknown", + "fabro:chunk-reload:aaaaaaaa", + "fabro:chunk-reload:bbbbbbbb", ]); }); diff --git a/apps/fabro-web/app/routes/run-terminal.tsx b/apps/fabro-web/app/routes/run-terminal.tsx index 6e4bddd93..9c6327ddc 100644 --- a/apps/fabro-web/app/routes/run-terminal.tsx +++ b/apps/fabro-web/app/routes/run-terminal.tsx @@ -1,5 +1,5 @@ import TerminalView from "../components/terminal-view"; -import { FabroToaster, ToastProvider } from "../components/toast"; +import { ToastProvider } from "../components/toast"; import { useDocumentTitle } from "../hooks/effects"; export default function RunTerminal({ params }: { params: { id: string } }) { @@ -10,7 +10,6 @@ export default function RunTerminal({ params }: { params: { id: string } }) {
- {typeof document !== "undefined" && } ); } diff --git a/apps/fabro-web/scripts/build.test.ts b/apps/fabro-web/scripts/build.test.ts index fafc79491..1a2b9cd44 100644 --- a/apps/fabro-web/scripts/build.test.ts +++ b/apps/fabro-web/scripts/build.test.ts @@ -1,10 +1,30 @@ import { test, expect } from "bun:test"; import { existsSync } from "node:fs"; import { lstat, readFile, readdir, readlink } from "node:fs/promises"; -import { basename, join } from "node:path"; +import { basename, join, relative, sep } from "node:path"; + +import { + BUILD_ID_FILE_NAME, + BUILD_ID_META_NAME, + parseBuildIdDocument, +} from "../app/lib/build-version-contract"; +import { localBundlerInputPaths } from "./build"; const root = Bun.fileURLToPath(new URL("..", import.meta.url)); +test("resolved build inputs retain workspace sources and omit installed packages", () => { + const inputs = localBundlerInputPaths([ + "app/entry.tsx", + "../../lib/packages/fabro-api-client/src/index.ts", + "../../node_modules/example/index.js", + ]).map((path) => relative(root, path).split(sep).join("/")); + + expect(inputs).toEqual([ + "app/entry.tsx", + "../../lib/packages/fabro-api-client/src/index.ts", + ]); +}); + async function runBuild() { const process = Bun.spawn(["bun", "run", "scripts/build.ts"], { cwd: root, @@ -22,10 +42,15 @@ async function runBuild() { } } -test("production build copies Pierre worker assets", async () => { +test("production builds publish a stable asset set and prune the previous build", async () => { await runBuild(); - const workerDist = join(root, "dist", "assets", "pierre-diffs-worker"); + const distPath = join(root, "dist"); + const firstTarget = await readlink(distPath); + expect((await lstat(distPath)).isSymbolicLink()).toBe(true); + expect(firstTarget.startsWith(".dist-builds/")).toBe(true); + + const workerDist = join(distPath, "assets", "pierre-diffs-worker"); expect(existsSync(join(workerDist, "worker-portable.js"))).toBe(true); const upstreamWorkerDir = join( @@ -43,84 +68,43 @@ test("production build copies Pierre worker assets", async () => { for (const wasmFile of wasmFiles) { expect(existsSync(join(workerDist, wasmFile))).toBe(true); } -}, 60000); -test("dist is a symlink into .dist-builds and old builds are pruned", async () => { - await runBuild(); - await runBuild(); - - const distPath = join(root, "dist"); - const stat = await lstat(distPath); - expect(stat.isSymbolicLink()).toBe(true); - - const target = await readlink(distPath); - expect(target.startsWith(".dist-builds/")).toBe(true); - - const buildId = target.slice(".dist-builds/".length); - const buildsRoot = join(root, ".dist-builds"); - const remaining = await readdir(buildsRoot); - expect(remaining).toEqual([buildId]); - - expect(existsSync(join(distPath, "index.html"))).toBe(true); -}, 60000); - -test("publishes a build id that index.html and build-id.json agree on", async () => { - await runBuild(); - - const distPath = join(root, "dist"); const published = JSON.parse( - await readFile(join(distPath, "build-id.json"), "utf8"), - ) as { buildId: string }; - - expect(published.buildId).toMatch(/^[a-z0-9]{8}$/); + await readFile(join(distPath, BUILD_ID_FILE_NAME), "utf8"), + ); + const firstBuildId = parseBuildIdDocument(published); + expect(firstBuildId).not.toBeNull(); const html = await readFile(join(distPath, "index.html"), "utf8"); expect(html).toContain( - ``, + ``, ); -}, 60000); -// The id is derived from source inputs rather than emitted filenames precisely -// so this holds: Bun's minified identifier naming is not deterministic, so the -// entry bundle's content hash changes between builds of an unchanged tree -// roughly one run in three. An id that moved with it would fire the client's -// "new version" toast on redeploys of identical code. -test("build id is stable across rebuilds of an unchanged tree", async () => { - const distPath = join(root, "dist"); - const readBuildId = async () => - ( - JSON.parse(await readFile(join(distPath, "build-id.json"), "utf8")) as { - buildId: string; - } - ).buildId; - - await runBuild(); - const first = await readBuildId(); - await runBuild(); - const second = await readBuildId(); - - expect(second).toBe(first); -}, 120000); - -// A stable-named stylesheet is served `no-cache`, letting a tab revalidate into -// new CSS while running old JS; Tailwind purges per build, so classes the old -// bundle still emits can vanish. The hash must match the `[a-z0-9]{8}` shape -// `is_content_hashed` in static_files.rs keys on. -test("stylesheet is content-hashed and referenced from index.html", async () => { - await runBuild(); - - const distPath = join(root, "dist"); const assets = await readdir(join(distPath, "assets")); const stylesheets = assets.filter((file) => /^app-.*\.css$/.test(file)); - expect(stylesheets).toHaveLength(1); expect(stylesheets[0]).toMatch(/^app-[a-z0-9]{8}\.css$/); expect(assets).not.toContain("app.css"); - - const html = await readFile(join(distPath, "index.html"), "utf8"); expect(html).toContain(`href="/assets/${stylesheets[0]}"`); expect(html).not.toContain('href="/assets/app.css"'); -}, 60000); + + // The id is derived from source inputs rather than emitted filenames. Bun's + // minified identifiers are nondeterministic, so output hashes can move even + // when the source graph is unchanged. + await runBuild(); + + const secondTarget = await readlink(distPath); + expect(secondTarget.startsWith(".dist-builds/")).toBe(true); + expect(secondTarget).not.toBe(firstTarget); + const secondBuildId = parseBuildIdDocument( + JSON.parse(await readFile(join(distPath, BUILD_ID_FILE_NAME), "utf8")), + ); + expect(secondBuildId).toBe(firstBuildId); + + const currentDirName = secondTarget.slice(".dist-builds/".length); + expect(await readdir(join(root, ".dist-builds"))).toEqual([currentDirName]); + expect(existsSync(join(distPath, "index.html"))).toBe(true); +}, 120000); test("watch mode keeps running until interrupted", async () => { const process = Bun.spawn([ diff --git a/apps/fabro-web/scripts/build.ts b/apps/fabro-web/scripts/build.ts index 1a9845128..360726fae 100644 --- a/apps/fabro-web/scripts/build.ts +++ b/apps/fabro-web/scripts/build.ts @@ -11,7 +11,13 @@ import { symlink, writeFile, } from "node:fs/promises"; -import { dirname, join, relative } from "node:path"; +import { dirname, join, relative, resolve, sep } from "node:path"; + +import { + BUILD_ID_FILE_NAME, + BUILD_ID_META_NAME, + buildIdDocument, +} from "../app/lib/build-version-contract"; declare const Bun: any; @@ -59,16 +65,18 @@ function contentHash8(content: string | Uint8Array): string { return toShortId(createHash("sha256").update(content).digest("hex")); } -// Everything that determines what the bundle contains. `bun.lock` lives at the -// workspace root, so a dependency bump changes the id even though no file under -// `app/` moved. +// Inputs outside Bun's JavaScript module graph. Tailwind scans `app/` for class +// names, `public/` is copied verbatim, and the remaining files control template +// rendering, module resolution, dependency versions, or the build itself. const BUILD_INPUT_DIRS = ["app", "public"]; const BUILD_INPUT_FILES = [ "index.template.html", "package.json", "scripts/build.ts", + "tsconfig.json", "../../bun.lock", ]; +const FILE_HASH_BATCH_SIZE = 32; /** * The build id published to browsers, derived from the bundle's *source inputs*. @@ -93,40 +101,83 @@ const BUILD_INPUT_FILES = [ * and `importChunk` covers the case where such a tab later needs a chunk whose * name moved. */ -async function publishedBuildId(): Promise { - const files: string[] = []; +async function publishedBuildId(bundlerInputs: Iterable): Promise { + const files = new Set(); for (const dir of BUILD_INPUT_DIRS) { - files.push(...(await collectFilesRecursively(join(rootPath, dir)))); + for (const file of await collectFiles(join(rootPath, dir))) { + files.add(file); + } } for (const file of BUILD_INPUT_FILES) { - files.push(join(rootPath, file)); + files.add(resolve(rootPath, file)); + } + // Bun's metafile is the source of truth for resolved production modules. In + // particular, it captures workspace sources reached through tsconfig path + // aliases, which a hand-maintained app-local file list would miss. + for (const file of localBundlerInputPaths(bundlerInputs)) { + files.add(file); } const digest = createHash("sha256"); // The bundler itself is an input: a Bun upgrade can change output semantics. digest.update(`bun:${Bun.version}\n`); - for (const file of files.sort()) { - // Hash the repo-relative path, not the absolute one, so the id doesn't - // depend on where the repo is checked out. - digest.update(relative(rootPath, file)); - digest.update("\0"); - digest.update(createHash("sha256").update(await readFile(file)).digest()); + const inputs = [...files] + .map((file) => ({ + file, + name: toUrlPath(relative(rootPath, file)), + })) + .sort((left, right) => + left.name < right.name ? -1 : left.name > right.name ? 1 : 0 + ); + + // Bound parallel reads so hashing stays off the rebuild critical path without + // exhausting low per-process file-descriptor limits on macOS. + for (let start = 0; start < inputs.length; start += FILE_HASH_BATCH_SIZE) { + const batch = inputs.slice(start, start + FILE_HASH_BATCH_SIZE); + const hashes = await Promise.all( + batch.map(async ({ file, name }) => ({ + name, + hash: createHash("sha256").update(await readFile(file)).digest(), + })), + ); + for (const { name, hash } of hashes) { + // Hash the repo-relative path, not the absolute one, so the id doesn't + // depend on where the repo is checked out. + digest.update(name); + digest.update("\0"); + digest.update(hash); + } } return toShortId(digest.digest("hex")); } -async function collectFilesRecursively(dir: string): Promise { - const collected: string[] = []; - const entries = await readdir(dir, { withFileTypes: true }); - for (const entry of entries) { - const full = join(dir, entry.name); - if (entry.isDirectory()) { - collected.push(...(await collectFilesRecursively(full))); - } else if (entry.isFile()) { - collected.push(full); - } +async function collectFiles(dir: string): Promise { + const glob = new Bun.Glob("**/*"); + const files: string[] = []; + for await (const file of glob.scan({ cwd: dir, dot: true, onlyFiles: true })) { + files.push(join(dir, file)); } - return collected; + return files; +} + +export function localBundlerInputPaths(inputs: Iterable): string[] { + return [...inputs] + .map((input) => resolve(rootPath, input)) + .filter((path) => !isInstalledDependency(path)); +} + +function isInstalledDependency(path: string): boolean { + return toUrlPath(relative(rootPath, path)) + .split("/") + .includes("node_modules"); +} + +function toUrlPath(path: string): string { + return path.split(sep).join("/"); +} + +function assetHref(path: string): string { + return `/${toUrlPath(path)}`; } async function buildOnce() { @@ -135,54 +186,61 @@ async function buildOnce() { const buildAssetsDir = join(buildDir, "assets"); await mkdir(buildAssetsDir, { recursive: true }); - const result = await Bun.build({ - entrypoints: [join(rootPath, "app", "entry.tsx")], - outdir: buildAssetsDir, - naming: "[name]-[hash].[ext]", - minify: true, - splitting: true, - target: "browser", - }); - - if (!result.success) { - throw new Error(result.logs.map((log: any) => log.message).join("\n")); - } - - const cssResult = await Bun.spawn([ - process.execPath, - tailwindCliBin, - "-i", - "app/app.css", - "-o", - relative(rootPath, join(buildAssetsDir, "app.css")), - "--minify", - ], { - cwd: rootPath, - stdout: "inherit", - stderr: "inherit", - }).exited; + const [result, cssResult] = await Promise.all([ + Bun.build({ + entrypoints: [join(rootPath, "app", "entry.tsx")], + outdir: buildAssetsDir, + naming: "[name]-[hash].[ext]", + minify: true, + splitting: true, + target: "browser", + metafile: true, + root: rootPath, + }), + Bun.spawn([ + process.execPath, + tailwindCliBin, + "-i", + "app/app.css", + "-o", + relative(rootPath, join(buildAssetsDir, "app.css")), + "--minify", + ], { + cwd: rootPath, + stdout: "inherit", + stderr: "inherit", + }).exited, + ]); if (cssResult !== 0) { throw new Error("Tailwind build failed"); } - const stylesheetPath = await hashStylesheet(buildDir, buildAssetsDir); + if (!result.success) { + throw new Error(result.logs.map((log: any) => log.message).join("\n")); + } - await cp(publicDir, buildDir, { recursive: true }); - await copyPierreWorkerAssets(join(buildAssetsDir, "pierre-diffs-worker")); + const [stylesheetPath, buildId] = await Promise.all([ + hashStylesheet(buildAssetsDir), + publishedBuildId(Object.keys(result.metafile.inputs)), + cp(publicDir, buildDir, { recursive: true }), + copyPierreWorkerAssets(join(buildAssetsDir, "pierre-diffs-worker")), + ]); - const outputs: IndexHtmlOutput[] = result.outputs.map((output: any) => ({ - kind: output.kind, - path: relative(buildDir, output.path), - })); - const buildId = await publishedBuildId(); + const outputs: IndexHtmlOutput[] = [ + { kind: "asset", path: stylesheetPath }, + ...result.outputs.map((output: any) => ({ + kind: output.kind, + path: relative(buildDir, output.path), + })), + ]; - await writeIndexHtml(buildDir, outputs, stylesheetPath, buildId); + await writeIndexHtml(buildDir, outputs, buildId); // Served with `no-cache` + ETag (it doesn't match the server's content-hash // pattern), so a polling client revalidates it as a cheap 304. await writeFile( - join(buildDir, "build-id.json"), - `${JSON.stringify({ buildId }, null, 2)}\n`, + join(buildDir, BUILD_ID_FILE_NAME), + `${JSON.stringify(buildIdDocument(buildId), null, 2)}\n`, "utf8", ); @@ -199,15 +257,12 @@ async function buildOnce() { * from the new CSS and elements silently render unstyled. Hashing pins the two * together and lets the server cache the stylesheet immutably. */ -async function hashStylesheet( - buildDir: string, - buildAssetsDir: string, -): Promise { +async function hashStylesheet(buildAssetsDir: string): Promise { const source = join(buildAssetsDir, "app.css"); const css = await readFile(source); const hashedName = `app-${contentHash8(css)}.css`; await rename(source, join(buildAssetsDir, hashedName)); - return relative(buildDir, join(buildAssetsDir, hashedName)); + return join("assets", hashedName); } async function copyPierreWorkerAssets(targetDir: string) { @@ -234,7 +289,6 @@ type IndexHtmlOutput = { async function writeIndexHtml( buildDir: string, outputs: IndexHtmlOutput[], - stylesheetPath: string, buildId: string, ) { const template = await readFile(templatePath, "utf8"); @@ -246,14 +300,11 @@ async function writeIndexHtml( // import() chunks load on demand. const scripts = outputs .filter((output) => output.kind === "entry-point" && output.path.endsWith(".js")) - .map((output) => ``) + .map((output) => ``) .join("\n "); - const styles = [ - `/${stylesheetPath.replaceAll("\\\\", "/")}`, - ...outputs - .filter((output) => output.path.endsWith(".css")) - .map((output) => `/${output.path.replaceAll("\\\\", "/")}`), - ] + const styles = outputs + .filter((output) => output.path.endsWith(".css")) + .map((output) => assetHref(output.path)) .filter((value, index, array) => array.indexOf(value) === index) .map((path) => ``) .join("\n "); @@ -262,7 +313,7 @@ async function writeIndexHtml( // and compares against /build-id.json; the meta tag is the honest answer // because client-side routing never re-fetches index.html, so it stays // pinned to the build the tab actually started with. - const buildMeta = ``; + const buildMeta = ``; const html = template .replace("{{styles}}", styles) @@ -397,7 +448,9 @@ async function main() { }); } -main().catch((error) => { - console.error(error); - process.exit(1); -}); +if (import.meta.main) { + main().catch((error) => { + console.error(error); + process.exit(1); + }); +}