From fc2b905837344a1c4e4bae1ecc9b9a70af1a5c79 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sat, 11 Jul 2026 16:39:50 -0700 Subject: [PATCH] fix(cli): distinguish producer-absent from injector failure in font localization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #2264: the localization helper had one broad catch around both dynamic producer resolution and injector execution, so it couldn't tell a benign 'producer not in this environment' from a real injector/fetch failure, and emitted no diagnostic. Split into loadFontInjector() (returns null when the module is absent — silent fail-open) and localizeWithProducer() (warns ONCE per distinct message when the injector itself throws, then fails open). Per-family resolution failures remain the injector's own responsibility (producer's warnUnresolvedFonts). The localizer seam is injectable; tests now cover success, producer-unavailable, injector-throw, warn dedup, and call-site integration. --- .../utils/bundleWithLocalizedFonts.test.ts | 55 +++++++++++-- .../cli/src/utils/bundleWithLocalizedFonts.ts | 78 +++++++++++++++---- 2 files changed, 108 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/utils/bundleWithLocalizedFonts.test.ts b/packages/cli/src/utils/bundleWithLocalizedFonts.test.ts index 957f432c7..fb7e7a751 100644 --- a/packages/cli/src/utils/bundleWithLocalizedFonts.test.ts +++ b/packages/cli/src/utils/bundleWithLocalizedFonts.test.ts @@ -4,13 +4,18 @@ vi.mock("@hyperframes/core/compiler", () => ({ bundleToSingleHtml: vi.fn(async () => "bundled"), })); -import { bundleWithLocalizedFonts } from "./bundleWithLocalizedFonts.js"; +import { + __resetFontLocalizationWarningsForTests, + bundleWithLocalizedFonts, + localizeWithProducer, +} from "./bundleWithLocalizedFonts.js"; afterEach(() => { + __resetFontLocalizationWarningsForTests(); vi.clearAllMocks(); }); -describe("bundleWithLocalizedFonts", () => { +describe("bundleWithLocalizedFonts (call-site integration)", () => { it("runs the injected font localizer over the plain bundle", async () => { const localize = vi.fn(async (html: string) => html.replace("bundled", "bundled+fonts")); const html = await bundleWithLocalizedFonts("/project", localize); @@ -19,11 +24,45 @@ describe("bundleWithLocalizedFonts", () => { expect(html).toBe("bundled+fonts"); }); - it("returns the localizer's output verbatim (localization is the last step)", async () => { - const html = await bundleWithLocalizedFonts( - "/project", - async () => "embedded-face", - ); - expect(html).toBe("embedded-face"); + it("returns the localizer output verbatim (localization is the last step)", async () => { + const html = await bundleWithLocalizedFonts("/project", async () => "embedded"); + expect(html).toBe("embedded"); + }); +}); + +describe("localizeWithProducer", () => { + it("embeds fonts when the injector is available", async () => { + const inject = vi.fn(async (html: string) => `${html}`); + const warn = vi.fn(); + const out = await localizeWithProducer("", async () => inject, warn); + expect(out).toBe(""); + expect(warn).not.toHaveBeenCalled(); + }); + + it("fails open silently when producer is unavailable (module absent → null)", async () => { + const warn = vi.fn(); + const out = await localizeWithProducer("plain", async () => null, warn); + // Never worse than a plain bundle; benign absence is not a warning. + expect(out).toBe("plain"); + expect(warn).not.toHaveBeenCalled(); + }); + + it("fails open WITH a diagnostic when the injector itself throws", async () => { + const warn = vi.fn(); + const boom: () => Promise = () => Promise.reject(new Error("fetch layer down")); + const out = await localizeWithProducer("plain", async () => boom, warn); + expect(out).toBe("plain"); + expect(warn).toHaveBeenCalledOnce(); + expect(warn.mock.calls[0]?.[0]).toContain("fetch layer down"); + }); + + it("dedups repeated identical injector failures across re-bundles", async () => { + const warn = vi.fn(); + const boom: () => Promise = () => Promise.reject(new Error("same failure")); + for (let i = 0; i < 5; i++) { + await localizeWithProducer("", async () => boom, warn); + } + // snapshot/check re-bundle per grid point; the warning must fire once. + expect(warn).toHaveBeenCalledOnce(); }); }); diff --git a/packages/cli/src/utils/bundleWithLocalizedFonts.ts b/packages/cli/src/utils/bundleWithLocalizedFonts.ts index fa0bc339b..46dd6a8cf 100644 --- a/packages/cli/src/utils/bundleWithLocalizedFonts.ts +++ b/packages/cli/src/utils/bundleWithLocalizedFonts.ts @@ -1,3 +1,6 @@ +import { normalizeErrorMessage } from "./errorMessage.js"; +import { c } from "../ui/colors.js"; + /** * Bundle a project to a single HTML string AND localize its fonts — fetch and * embed `@font-face` rules for every requested family (including families @@ -11,15 +14,11 @@ * fall back to an un-styled system sans when the remote font loses the race * against the capture. Running the SAME localization the render path uses makes * snapshot/check captures font-faithful and deterministic — no network race. - * - * Fail-open: if a family can't be fetched (offline, unknown font), the - * underlying injector leaves the HTML unchanged, so this never makes a bundle - * worse than plain `bundleToSingleHtml`. */ export async function bundleWithLocalizedFonts( projectDir: string, // Injectable for tests. Production callers omit it and get the producer - // font-localization pass, resolved lazily at runtime (see localizeWithProducer). + // font-localization pass (see localizeWithProducer). localizeFonts: (html: string) => Promise = localizeWithProducer, ): Promise { const { bundleToSingleHtml } = await import("@hyperframes/core/compiler"); @@ -27,26 +26,71 @@ export async function bundleWithLocalizedFonts( return localizeFonts(html); } +type FontInjector = (html: string) => Promise; + /** - * Run the render pipeline's `injectDeterministicFontFaces` pass, resolving + * Load the render pipeline's `injectDeterministicFontFaces`, resolving * `@hyperframes/producer` at RUNTIME only. The specifier is kept out of the * bundler's/test-runner's static module graph (`@vite-ignore` + a variable - * specifier) on purpose: the CLI test job doesn't build producer, so a static - * `import("@hyperframes/producer")` would fail Vitest's transform-time - * resolution. At runtime — the built CLI, or an installed package — producer is - * a real dependency and resolves via node_modules. + * specifier) on purpose: the CLI test job builds with `--filter + * '!@hyperframes/producer'`, so a static `import("@hyperframes/producer")` + * would fail Vitest's transform-time resolution. At runtime — the built CLI or + * an installed package — producer is a real dependency and resolves via + * node_modules. * - * Fail-open: if producer can't be resolved or a fetch layer throws, return the - * HTML unchanged so a bundle is never worse than plain `bundleToSingleHtml`. + * Returns `null` (not a throw) when the module simply isn't available in this + * environment, so the caller can treat "producer absent" — a benign, expected + * condition — differently from "the injector itself failed". */ -async function localizeWithProducer(html: string): Promise { +async function loadFontInjector(): Promise { try { const producerSpecifier = "@hyperframes/producer"; - const { injectDeterministicFontFaces } = (await import( - /* @vite-ignore */ producerSpecifier - )) as typeof import("@hyperframes/producer"); - return await injectDeterministicFontFaces(html); + const mod = (await import(/* @vite-ignore */ producerSpecifier)) as { + injectDeterministicFontFaces?: FontInjector; + }; + return mod.injectDeterministicFontFaces ?? null; } catch { + return null; + } +} + +const warnedFontLocalizationFailures = new Set(); + +/** Reset the dedup latch — tests only. */ +export function __resetFontLocalizationWarningsForTests(): void { + warnedFontLocalizationFailures.clear(); +} + +/** + * Localize fonts via the producer injector, distinguishing two failure modes: + * + * - **Module unavailable** (`loadInjector` yields `null`): benign — producer + * isn't in this environment. Return the HTML unchanged, silently; fonts + * declared via a remote `` still load at capture time as before. + * - **Injector threw** (a fetch layer failed, a family errored past the + * injector's own per-family handling): unexpected — surface it ONCE per + * distinct message (snapshot/check re-bundle per grid point, so an + * un-deduped warning would spam), then fail open to the plain bundle. + * + * Per-family resolution failures inside a successful pass are already reported + * by the injector itself (producer's `warnUnresolvedFonts`); this layer only + * owns the module-vs-execution distinction and the dedup. + */ +export async function localizeWithProducer( + html: string, + loadInjector: () => Promise = loadFontInjector, + warn: (message: string) => void = (m) => console.warn(` ${c.warn("⚠")} ${m}`), +): Promise { + const inject = await loadInjector(); + if (!inject) return html; + try { + return await inject(html); + } catch (err) { + const message = `Font localization failed; capturing with remote/fallback fonts instead: ${normalizeErrorMessage(err)}`; + if (!warnedFontLocalizationFailures.has(message)) { + warnedFontLocalizationFailures.add(message); + warn(message); + } return html; } }